Re: [PATCH 2/4] usb: typec: ucsi: ccg: add firmware flashing support
Hi Ajay,
On Thu, Apr 11, 2019 at 12:31:45AM +0800, kbuild test robot wrote:
> Hi Heikki,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v5.1-rc4 next-20190410]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Heikki-Krogerus/usb-typec-ucsi-Remaining-changes-for-v5-2/20190410-221455
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> usb-testing
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
>
>
> sparse warnings: (new ones prefixed by >>)
>
>drivers/usb/typec/ucsi/ucsi_ccg.c:212:24: sparse: expression using
> sizeof(void)
>drivers/usb/typec/ucsi/ucsi_ccg.c:212:24: sparse: expression using
> sizeof(void)
> >> drivers/usb/typec/ucsi/ucsi_ccg.c:690:16: sparse: restricted __le16
> >> degrades to integer
>drivers/usb/typec/ucsi/ucsi_ccg.c:698:24: sparse: restricted __le16
> degrades to integer
>drivers/usb/typec/ucsi/ucsi_ccg.c:735:26: sparse: restricted __le16
> degrades to integer
>drivers/usb/typec/ucsi/ucsi_ccg.c:737:33: sparse: restricted __le16
> degrades to integer
>drivers/usb/typec/ucsi/ucsi_ccg.c:777:37: sparse: restricted __le16
> degrades to integer
>
> vim +690 drivers/usb/typec/ucsi/ucsi_ccg.c
>
>680
>681static bool ccg_check_vendor_version(struct ucsi_ccg *uc,
>682 struct version_format *app,
>683 struct fw_config_table
> *fw_cfg)
>684{
>685struct device *dev = uc->dev;
>686
>687/* Check if the fw build is for supported vendors.
>688 * Add all supported vendors here.
>689 */
> > 690if (app->build != (('n' << 8) | 'v')) {
How about a macro for these?
#define CCG_VERSION_BUILD (__le16)(...)
>691dev_info(dev, "current fw is not from supported
> vendor\n");
>692return false;
>693}
>694
>695/* Check if the new fw build is for supported vendors
>696 * Add all supported vendors here.
>697 */
>698if (fw_cfg->app.build != (('n' << 8) | 'v')) {
>699dev_info(dev, "new fw is not from supported
> vendor\n");
>700return false;
>701}
>702return true;
>703}
>704
thanks,
--
heikki
Re: Why there is a refcnt check when we change configfs property
Hi Peter, W dniu 08.04.2019 o 09:00, Peter Chen pisze: Andrzej, would you tell us why you introduce refcnt for driver's configfs opts? And why it is needed to judge for "store" operation? The reason is that a function can be linked to more than one configuration. Imagine this sequence: you create a function, set its parameters and then create a configuration. Then you declare "I want this function in my configuration". You can decide that you want that particular function instance because you already know how it is configured and are fine with that state. Think of it as a contract between the function and the configuration. Now, suppose some other configuration is created and in that configuration a similar function is needed. The creator of that other configuration might be tempted to modify the existing and already used function (linked to a configuration), thus breaking the contract between the function and the first configuration. Thanks, Andrzej. But we designed "store" entry for function drivers, we can't use it now. Is it possible let the parameters per function device instead of function driver? Or like module parameters, let the user change one time at least. Can you please describe your use case? Andrzej
RE: [EXT] Re: Why there is a refcnt check when we change configfs property
> > > >>> > >>> Andrzej, would you tell us why you introduce refcnt for driver's > >>> configfs opts? And why it is needed to judge for "store" operation? > >>> > >> > >> The reason is that a function can be linked to more than one > >> configuration. Imagine this sequence: you create a function, set its > >> parameters and then create a configuration. Then you declare "I want > >> this function in my configuration". You can decide that you want that > >> particular function instance because you already know how it is > >> configured and are fine with that state. Think of it as a contract between > >> the > function and the configuration. > >> > >> Now, suppose some other configuration is created and in that > >> configuration a similar function is needed. The creator of that other > >> configuration might be tempted to modify the existing and already > >> used function (linked to a configuration), thus breaking the contract > >> between the > function and the first configuration. > > > > Thanks, Andrzej. > > > > But we designed "store" entry for function drivers, we can't use it now. > > Is it possible let the parameters per function device instead of function > > driver? > > Or like module parameters, let the user change one time at least. > > > > Can you please describe your use case? > > Andrzej I would like to change parameters for function driver through configfs, below is the example for ncm and f_sourcesink cases, thanks. if [ "$FUNC" == "ncm" ]; then mkdir functions/$FUNC".0" ln -s functions/$FUNC".0" configs/c.1 echo 10 > functions/ncm.0/qmult echo "12:a5:cf:42:92:fd" > functions/ncm.0/host_addr echo "5e:bc:ca:27:92:b1" > functions/ncm.0/dev_addr fi if [ "$FUNC" == "ss" ]; then cd /sys/kernel/config/usb_gadget cd g1 echo "0x0525" > idVendor echo "0xa4a0" > idProduct export FUNC="SourceSink" mkdir functions/$FUNC".0" ln -s functions/$FUNC".0" configs/c.1 cd functions/$FUNC".0" echo 1 > pattern echo 1 > isoc_interval echo 1024 > isoc_maxpacket echo 2 > isoc_mult #isoc_maxburst echo 1024 > bulk_buflen echo 16 > bulk_qlen echo 16 > iso_qlen fi Peter
Hi dear,
Hi dear, I am still waiting for your Email response, you did receive my first email to you Respectfully Yours, Mrs Katie Huggins
[PATCH v6 1/2] usb: typec: ucsi: ccg: add get_fw_info function
Function is to get the details of ccg firmware and device version.
It will be useful in debugging and also during firmware update.
Signed-off-by: Ajay Gupta
Signed-off-by: Heikki Krogerus
---
Changes from v5 to v6
- None
drivers/usb/typec/ucsi/ucsi_ccg.c | 66 ++-
1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index de8a43bdff68..3884fb41c72e 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -17,15 +17,54 @@
#include
#include "ucsi.h"
+enum enum_fw_mode {
+ BOOT, /* bootloader */
+ FW1,/* FW partition-1 (contains secondary fw) */
+ FW2,/* FW partition-2 (contains primary fw) */
+ FW_INVALID,
+};
+
+struct ccg_dev_info {
+#define CCG_DEVINFO_FWMODE_SHIFT (0)
+#define CCG_DEVINFO_FWMODE_MASK (0x3 << CCG_DEVINFO_FWMODE_SHIFT)
+#define CCG_DEVINFO_PDPORTS_SHIFT (2)
+#define CCG_DEVINFO_PDPORTS_MASK (0x3 << CCG_DEVINFO_PDPORTS_SHIFT)
+ u8 mode;
+ u8 bl_mode;
+ __le16 silicon_id;
+ __le16 bl_last_row;
+} __packed;
+
+struct version_format {
+ __le16 build;
+ u8 patch;
+ u8 ver;
+#define CCG_VERSION_MIN_SHIFT (0)
+#define CCG_VERSION_MIN_MASK (0xf << CCG_VERSION_MIN_SHIFT)
+#define CCG_VERSION_MAJ_SHIFT (4)
+#define CCG_VERSION_MAJ_MASK (0xf << CCG_VERSION_MAJ_SHIFT)
+} __packed;
+
+struct version_info {
+ struct version_format base;
+ struct version_format app;
+};
+
struct ucsi_ccg {
struct device *dev;
struct ucsi *ucsi;
struct ucsi_ppm ppm;
struct i2c_client *client;
+ struct ccg_dev_info info;
+ /* version info for boot, primary and secondary */
+ struct version_info version[FW2 + 1];
};
-#define CCGX_RAB_INTR_REG 0x06
-#define CCGX_RAB_UCSI_CONTROL 0x39
+#define CCGX_RAB_DEVICE_MODE 0x
+#define CCGX_RAB_INTR_REG 0x0006
+#define CCGX_RAB_READ_ALL_VER 0x0010
+#define CCGX_RAB_READ_FW2_VER 0x0020
+#define CCGX_RAB_UCSI_CONTROL 0x0039
#define CCGX_RAB_UCSI_CONTROL_STARTBIT(0)
#define CCGX_RAB_UCSI_CONTROL_STOP BIT(1)
#define CCGX_RAB_UCSI_DATA_BLOCK(offset) (0xf000 | ((offset) & 0xff))
@@ -220,6 +259,23 @@ static irqreturn_t ccg_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static int get_fw_info(struct ucsi_ccg *uc)
+{
+ int err;
+
+ err = ccg_read(uc, CCGX_RAB_READ_ALL_VER, (u8 *)(&uc->version),
+ sizeof(uc->version));
+ if (err < 0)
+ return err;
+
+ err = ccg_read(uc, CCGX_RAB_DEVICE_MODE, (u8 *)(&uc->info),
+ sizeof(uc->info));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
static int ucsi_ccg_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -248,6 +304,12 @@ static int ucsi_ccg_probe(struct i2c_client *client,
return status;
}
+ status = get_fw_info(uc);
+ if (status < 0) {
+ dev_err(uc->dev, "get_fw_info failed - %d\n", status);
+ return status;
+ }
+
status = devm_request_threaded_irq(dev, client->irq, NULL,
ccg_irq_handler,
IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
--
2.17.1
[PATCH v6 0/2] Add support for firmware update on Cypres CCGx
Hi Heikki These changes adds support for updating firmware on Cypress CCGx controller. New version (v6) fixes sparse warning as reported at [1]. I have tested them on NVIDIA GPU card. Firmware binary is already merged. Details are at [2]. Please help review this set. Thanks Ajay [1] https://marc.info/?l=linux-usb&m=155491396220133&w=2 [2] https://marc.info/?l=linux-usb&m=155006182508289&w=2 Ajay Gupta (2): usb: typec: ucsi: ccg: add get_fw_info function usb: typec: ucsi: ccg: add firmware flashing support drivers/usb/typec/ucsi/ucsi_ccg.c | 887 +- 1 file changed, 877 insertions(+), 10 deletions(-) -- 2.17.1
[PATCH v6 2/2] usb: typec: ucsi: ccg: add firmware flashing support
CCGx has two copies of the firmware in addition to the bootloader.
If the device is running FW1, FW2 can be updated with the new version.
Dual firmware mode allows the CCG device to stay in a PD contract and
support USB PD and Type-C functionality while a firmware update is in
progress.
First we read the currently flashed firmware version of both
primary and secondary firmware and then compare it with
version of firmware file to determine if flashing is required.
Command framework is added to support sending commands to CCGx
controller. We wait for response after sending the command and then
read the response from RAB_RESPONSE register.
Below commands are supported,
- ENTER_FLASHING
- RESET
- PDPORT_ENABLE
- JUMP_TO_BOOT
- FLASH_ROW_RW
- VALIDATE_FW
Command specific mutex lock is also added to sync between driver
and user threads.
PD port number information is added which is required while sending
PD_PORT_ENABLE command
Signed-off-by: Ajay Gupta
---
Changes from v5 to v6
- Fixed below sparse warnings
"restricted __le16 degrades to integer"
drivers/usb/typec/ucsi/ucsi_ccg.c | 831 +-
1 file changed, 818 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 3884fb41c72e..a899d4c29125 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -9,6 +9,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -24,6 +25,73 @@ enum enum_fw_mode {
FW_INVALID,
};
+#define CCGX_RAB_DEVICE_MODE 0x
+#define CCGX_RAB_INTR_REG 0x0006
+#define DEV_INT BIT(0)
+#define PORT0_INT BIT(1)
+#define PORT1_INT BIT(2)
+#define UCSI_READ_INT BIT(7)
+#define CCGX_RAB_JUMP_TO_BOOT 0x0007
+#define TO_BOOT 'J'
+#define TO_ALT_FW 'A'
+#define CCGX_RAB_RESET_REQ 0x0008
+#define RESET_SIG 'R'
+#define CMD_RESET_I2C 0x0
+#define CMD_RESET_DEV 0x1
+#define CCGX_RAB_ENTER_FLASHING0x000A
+#define FLASH_ENTER_SIG 'P'
+#define CCGX_RAB_VALIDATE_FW 0x000B
+#define CCGX_RAB_FLASH_ROW_RW 0x000C
+#define FLASH_SIG 'F'
+#define FLASH_RD_CMD 0x0
+#define FLASH_WR_CMD 0x1
+#define FLASH_FWCT1_WR_CMD0x2
+#define FLASH_FWCT2_WR_CMD0x3
+#define FLASH_FWCT_SIG_WR_CMD 0x4
+#define CCGX_RAB_READ_ALL_VER 0x0010
+#define CCGX_RAB_READ_FW2_VER 0x0020
+#define CCGX_RAB_UCSI_CONTROL 0x0039
+#define CCGX_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_RAB_UCSI_DATA_BLOCK(offset) (0xf000 | ((offset) & 0xff))
+#define REG_FLASH_RW_MEM0x0200
+#define DEV_REG_IDXCCGX_RAB_DEVICE_MODE
+#define CCGX_RAB_PDPORT_ENABLE 0x002C
+#define PDPORT_1 BIT(0)
+#define PDPORT_2 BIT(1)
+#define CCGX_RAB_RESPONSE 0x007E
+#define ASYNC_EVENT BIT(7)
+
+/* CCGx events & async msg codes */
+#define RESET_COMPLETE 0x80
+#define EVENT_INDEXRESET_COMPLETE
+#define PORT_CONNECT_DET 0x84
+#define PORT_DISCONNECT_DET0x85
+#define ROLE_SWAP_COMPELETE0x87
+
+/* ccg firmware */
+#define CYACD_LINE_SIZE 527
+#define CCG4_ROW_SIZE 256
+#define FW1_METADATA_ROW0x1FF
+#define FW2_METADATA_ROW0x1FE
+#define FW_CFG_TABLE_SIG_SIZE 256
+
+static int secondary_fw_min_ver = 41;
+
+enum enum_flash_mode {
+ SECONDARY_BL, /* update secondary using bootloader */
+ PRIMARY,/* update primary using secondary */
+ SECONDARY, /* update secondary using primary */
+ FLASH_NOT_NEEDED, /* update not required */
+ FLASH_INVALID,
+};
+
+static const char * const ccg_fw_names[] = {
+ "ccg_boot.cyacd",
+ "ccg_primary.cyacd",
+ "ccg_secondary.cyacd"
+};
+
struct ccg_dev_info {
#define CCG_DEVINFO_FWMODE_SHIFT (0)
#define CCG_DEVINFO_FWMODE_MASK (0x3 << CCG_DEVINFO_FWMODE_SHIFT)
@@ -50,6 +118,50 @@ struct version_info {
struct version_format app;
};
+struct fw_config_table {
+ u32 identity;
+ u16 table_size;
+ u8 fwct_version;
+ u8 is_key_change;
+ u8 guid[16];
+ struct version_format base;
+ struct version_format app;
+ u8 primary_fw_digest[32];
+ u32 key_exp_length;
+ u8 key_modulus[256];
+ u8
RE: [PATCH 2/4] usb: typec: ucsi: ccg: add firmware flashing support
Hi Heikki,
> -Original Message-
> From: Heikki Krogerus
> Sent: Thursday, April 11, 2019 12:40 AM
> To: Ajay Gupta
> Cc: kbuild test robot ; kbuild-...@01.org; Greg Kroah-Hartman
> ; linux-usb@vger.kernel.org
> Subject: Re: [PATCH 2/4] usb: typec: ucsi: ccg: add firmware flashing support
>
> Hi Ajay,
>
> On Thu, Apr 11, 2019 at 12:31:45AM +0800, kbuild test robot wrote:
> > Hi Heikki,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on usb/usb-testing] [also build test WARNING
> > on v5.1-rc4 next-20190410] [if your patch is applied to the wrong git
> > tree, please drop us a note to help improve the system]
> >
> > url:https://github.com/0day-ci/linux/commits/Heikki-Krogerus/usb-typec-
> ucsi-Remaining-changes-for-v5-2/20190410-221455
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-
> testing
> > reproduce:
> > # apt-get install sparse
> > make ARCH=x86_64 allmodconfig
> > make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >
> >drivers/usb/typec/ucsi/ucsi_ccg.c:212:24: sparse: expression using
> sizeof(void)
> >drivers/usb/typec/ucsi/ucsi_ccg.c:212:24: sparse: expression using
> > sizeof(void)
> > >> drivers/usb/typec/ucsi/ucsi_ccg.c:690:16: sparse: restricted __le16
> > >> degrades to integer
> >drivers/usb/typec/ucsi/ucsi_ccg.c:698:24: sparse: restricted __le16
> > degrades
> to integer
> >drivers/usb/typec/ucsi/ucsi_ccg.c:735:26: sparse: restricted __le16
> > degrades
> to integer
> >drivers/usb/typec/ucsi/ucsi_ccg.c:737:33: sparse: restricted __le16
> > degrades
> to integer
> >drivers/usb/typec/ucsi/ucsi_ccg.c:777:37: sparse: restricted __le16
> > degrades to integer
> >
> > vim +690 drivers/usb/typec/ucsi/ucsi_ccg.c
> >
> >680
> >681 static bool ccg_check_vendor_version(struct ucsi_ccg *uc,
> >682 struct version_format *app,
> >683 struct fw_config_table
> > *fw_cfg)
> >684 {
> >685 struct device *dev = uc->dev;
> >686
> >687 /* Check if the fw build is for supported vendors.
> >688 * Add all supported vendors here.
> >689 */
> > > 690 if (app->build != (('n' << 8) | 'v')) {
>
> How about a macro for these?
>
> #define CCG_VERSION_BUILD (__le16)(...)
Sure, I have fixed them and posted version-6 of patches at
https://marc.info/?l=linux-usb&m=155501300619846&w=2
Please help review.
Thanks
> nvpublic
>
> >691 dev_info(dev, "current fw is not from supported
> vendor\n");
> >692 return false;
> >693 }
> >694
> >695 /* Check if the new fw build is for supported vendors
> >696 * Add all supported vendors here.
> >697 */
> >698 if (fw_cfg->app.build != (('n' << 8) | 'v')) {
> >699 dev_info(dev, "new fw is not from supported
> vendor\n");
> >700 return false;
> >701 }
> >702 return true;
> >703 }
> >704
>
> thanks,
>
> --
> heikki
Zdravstvujte Vas interesuyut klientskie bazy dannyh?
Zdravstvujte Vas interesuyut klientskie bazy dannyh?
Re: [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
Hi Johan,
Do you have time to check the contents of my reply?
If you have any questions about the content of my reply,
please feel free to ask. Thank you!
My boss hopes to pass this patch as soon as possible.
There are some companies (HTC, HUAWEI, ASUS),
their software engineers have modified the patch with me 3 to 4 months ago,
and can use the new PL2303G (Type_HXN) or
the old PL2303 (TYPE_HX). ) on the Linux OS.
But these companies want to be able to update the patch from the Linux kernel
instead of using the patch from me.
Charles Yeh 於 2019年4月9日 週二 下午5:52寫道：
>
> Hi Johan,
> 1. The HXN register layout is entirely different from HX and
>earlier devices.
>
> 2. HXN use the same CDC class requests (line encoding, etc) as
>earlier revisions.
>
> Can you send me documentation for the HXN protocol? That would help a
> lot in finding the right abstraction level for this.
>
> Ans：
> Yes,The HXN register layout is entirely different from HX and earlier devices,
> Yes, HXN use the same CDC class requests (line encoding, etc) as
> earlier revisions
>
> I don't have a PL2303G (HXN) Word file, I only have Excel's table file,
> please refer to PL2303_Linux\Vendor_Requesut.png
> Or you can install window driver use PL2303G demo board.
> (The Fedex tracking no. is 8117 8594 0521)
> and then USB Bus Hound to analysis the vendor request.
>
> The original Prolific company is hoping:
> TYPE_HX and TYPE_01 used old driver(pl2303.c & pl2303.h)
> The new PL2303G (TYPE_HXN) uses new driver(plser.c & plser.h),
>
> You can go to the website of Prolific, in the website,
>
> TYPE_HX and TYPE_01 are called
> PL2303 USB to Serial Bridge Controllers (All Chip Versions)
>
> TYPE_HXN is called
> PL2303G USB to Serial Bridge Controllers (All Chip Versions).
>
> We provide two different file paths on the website, the name.file..
> is to distinguish between TYPE_HX/01 and TYPE_HXN
> please refer to PL2303_Linux\Window_support_hx_hxn.jpg
>
> TYPE_HX and TYPE_01:the windows file name is ser2pl.sys & ser2pl64.sys.
>
> TYPE_HXN:the windows file name is plser.sys & plser64.sys.
>
> please refer to PL2303_Linux\window_plser_ser2pl.bmp
>
> But Greg disagreed with the Linux OS about new File name(plser.c & plser.h)
> So we did that we merged with the existing pl2303.c(h)
> into a new chip (PL2303G, TYPE_HXN)
>
> > +#define TYPE_HXN_FLOWCONTROL_REG 0x0A
>
> > +#define TYPE_HXN_HARDWAREFLOW_DATA 0xFA
> > +#define TYPE_HXN_SOFTWAREFLOW_DATA 0xEE
> > +#define TYPE_HXN_NOFLOW_DATA 0xFF
>
> What exactly does bits 0x15 (bits 4, 2 and 0) do?
> Ans:please refer to PL2303_Linux\PL2303G_TYPE_HXN_UART_Flow.jpg,
>
> Is register 0x0a really only used for flow control?
> Ans: Yes, please refer to PL2303_Linux\PL2303G_TYPE_HXN_UART_Flow.jpg,
> 0x0A(Bit 2, 3, 4)is UART flow control settting,
> it only supports TYPE_HXN, no supports TYPE_HX/01.
>
>
> In an earlier version of your patch, you also checked bcdUSB here. Why
> did you remove it?
>
> > + res = usb_control_msg(serial->dev,
> > + usb_rcvctrlpipe(serial->dev, 0),
> > + VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
> > + TYPE_HX_READ_STATUS_REG, 0, buf, 1, 100);
>
> Please name the registers after what they do, not how you use them. What
> is register 0 that you read here? Does it have a name?
>
> Ans: Because the "kbuild test robot " reply me a error.
> Please refer to Email: "Tuesday, December 25, 2018, at 11:39 pm"
> or refer to PL2303_Linux\remoe_checked_bcdUSB.txt
>
>
> Why TYPE_01 sets bit 0x20 of register 2 instead of 0x40 as the HX does?
> Is that even correct?
> Ans: Yes, it is correct,
> The Output(TX/DTR/RTS) Tri-state H/W design is different.
>
> TYPE_01: RS-232 Output Tri-state:
> 0: RS-232 output in output mode;
> 1: RS-232 output tri-state.
>
> TYPE_HX_Serial Port (TXD, RTS, and DTR) Output Enable:
> 00 – Disable output signals (Tri-State) at all time;
> 01 – Disable output signals (Tri-State) during suspend;
> 10, 11 – Set output signals to HIGH during suspend;
>
>
>
> > if (C_CRTSCTS(tty)) {
> > - if (spriv->quirks & PL2303_QUIRK_LEGACY)
> > - pl2303_vendor_write(serial, 0x0, 0x41);
>
> Why do the TYPE_01 not set bit 0x20 here? Do the legacy device support
> both auto-rts and auto-cts?
>
> Ans: Yes, it is correct,
> The Hardware UART flow control settting design is different
> between TYPT_01 and TYPT_HX.
>
> Although I have been in Prolific Company for almost 16 years..
> But TYPE_01 (PL2303H) has been discontinued(EOL) for 13 years...
> Now can't find this IC(TYPT_01: PL2303H) on the market..
>
> The line"if (spriv->quirks & PL2303_QUIRK_LEGACY)"
> you are asking now is not what I wrote...
> I don't know why to define it as PL2303_QUIRK_LEGACY.
>
> I am mainly responsible for the PL2303 driver
> Windows, WinCE, Mac & Android in Prolific.
> On the Linux driver side, only when the customer has a problem,
> or want to add n
Re: [PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)
Hi Johan,
A Patch:
[PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
B Patch:
[PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to
support PL2303HXD (TYPE_HX)
To save time, let you have more time to process A patch,
B patch's reply will wait until A patch passes,
then I will repeat the problem of B patch.
Charles.
Johan Hovold 於 2019年4月2日 週二 下午3:34寫道：
>
> On Tue, Feb 12, 2019 at 08:50:49PM +0800, Charles Yeh wrote:
> > Pull-Up mode is disabled (default) in PL2303HXD.
> > When the Pull-Up mode is activated, its TX/DTR/RTS external resistor will
> > start the output function.
> >
> > How to enable the Pull-Up mode of PL2303HXD
> > 1.TX/DTR/RTS external resistor is required on the circuit diagram (PCB)
> > 2.PL2303HXD OTP needs to be programmed to have a Pull-Up mode through 6.5V
> > (USB_VCC,5V->6.5V)
> >
> > The patch driver will read whether the PL2303HXD has a Pull-Up mode,and if
> > so,
> > it will be set to enable Pull-Up mode function.
> >
> > Signed-off-by: Charles Yeh
> > ---
> > drivers/usb/serial/pl2303.c | 24
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> > index bb3f9aa4a909..e5d00e4a495d 100644
> > --- a/drivers/usb/serial/pl2303.c
> > +++ b/drivers/usb/serial/pl2303.c
> > @@ -145,6 +145,16 @@ MODULE_DEVICE_TABLE(usb, id_table);
> > #define UART_OVERRUN_ERROR 0x40
> > #define UART_CTS 0x80
> >
> > +#define TYPE_HX_READ_PUM_STATUS_REG 0x8484
> > +#define TYPE_HX_READ_PUM_ADD0x0404
> > +#define TYPE_HX_READ_PUM_DATA_REG 0x8383
>
> Again, the defines need to reflect what the registers are for, not what
> use happen to use them for in on specific code path.
>
> These are used to access the EEPROM/OTP, right?
>
> > +#define TYPE_HX_PULLUP_MODE_DATA0x08
>
> What is bit 0x08?
>
> > +#define TYPE_HX_PULLUP_MODE_REG 0x09
>
> And this register isn't just used for pull-up mode as far as I can tell.
>
> > +#define TYPE_HX_PUM_ADD00x00
> > +#define TYPE_HX_PUM_DATA0 0x31
> > +#define TYPE_HX_PUM_ADD10x01
> > +#define TYPE_HX_PUM_DATA1 0x08
>
> Same for these, we don't want multiple defines for register 0 for
> example. What are bits 0x31 of register 0?
>
> > +
> > static void pl2303_set_break(struct usb_serial_port *port, bool enable);
> >
> > enum pl2303_type {
> > @@ -688,6 +698,20 @@ static void pl2303_set_termios(struct tty_struct *tty,
> > pl2303_vendor_write(serial, 0x0, 0x0);
> > }
> >
> > + if (spriv->type == &pl2303_type_data[TYPE_HX]) {
> > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf);
> > + pl2303_vendor_write(serial, TYPE_HX_READ_PUM_ADD,
> > + TYPE_HX_PULLUP_MODE_REG);
> > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf);
> > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_DATA_REG, buf);
>
> Why do you need to access the OTP on every set_termios() call? I thought
> those settings where copied to the corresponding control registers
> during boot?
>
> > + if (*buf == TYPE_HX_PULLUP_MODE_DATA) {
>
> Don't you want to just check bit 0x80 here?
>
> > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD0,
> > + TYPE_HX_PUM_DATA0);
>
> So this looks broken since you're overwriting the flow control settings
> that were just set above. And again, what are bits 0x31 of register 0?
> Doesn't 0x30 control auto-rts?
>
> > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD1,
> > + TYPE_HX_PUM_DATA1);
>
> And why do you need to (over-)write register 2?
>
> Either way, configuring pull-up mode should be done at probe and not on
> every set_termios() call.
>
> > + }
> > + }
> > +
> > kfree(buf);
> > }
>
> Thanks,
> Johan
