| Message ID | 20240807052359.290046-3-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce initial AMD I3C HCI driver support | expand |
Hi On 8/7/24 8:23 AM, Shyam Sundar S K wrote: > The HC_CONTROL_PIO_MODE bit was introduced in the HC_CONTROL register > starting from version 1.1. Therefore, checking the HC_CONTROL_PIO_MODE bit > on hardware that adheres to older specification revisions (i.e., versions > earlier than 1.1) is incorrect. To address this, add an additional check > to read the HCI version before attempting to read the HC_CONTROL_PIO_MODE > status. > > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/i3c/master/mipi-i3c-hci/core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c > index 24dd4603d6c6..a16da70bdfe1 100644 > --- a/drivers/i3c/master/mipi-i3c-hci/core.c > +++ b/drivers/i3c/master/mipi-i3c-hci/core.c > @@ -33,6 +33,7 @@ > #define reg_clear(r, v) reg_write(r, reg_read(r) & ~(v)) > > #define HCI_VERSION 0x00 /* HCI Version (in BCD) */ > +#define HCI_VERSION_V1 0x100 /* MIPI HCI Version number V1.0 */ > > #define HC_CONTROL 0x04 > #define HC_CONTROL_BUS_ENABLE BIT(31) > @@ -756,7 +757,7 @@ static int i3c_hci_init(struct i3c_hci *hci) > /* Try activating DMA operations first */ > if (hci->RHS_regs) { > reg_clear(HC_CONTROL, HC_CONTROL_PIO_MODE); > - if (reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE) { > + if (regval > HCI_VERSION_V1 && !(reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE)) { > dev_err(&hci->master.dev, "PIO mode is stuck\n"); > ret = -EIO; > } else { Here's typo and logic is reversed.
On 8/9/2024 18:51, Jarkko Nikula wrote: > Hi > > On 8/7/24 8:23 AM, Shyam Sundar S K wrote: >> The HC_CONTROL_PIO_MODE bit was introduced in the HC_CONTROL register >> starting from version 1.1. Therefore, checking the >> HC_CONTROL_PIO_MODE bit >> on hardware that adheres to older specification revisions (i.e., >> versions >> earlier than 1.1) is incorrect. To address this, add an additional >> check >> to read the HCI version before attempting to read the >> HC_CONTROL_PIO_MODE >> status. >> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/i3c/master/mipi-i3c-hci/core.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c >> b/drivers/i3c/master/mipi-i3c-hci/core.c >> index 24dd4603d6c6..a16da70bdfe1 100644 >> --- a/drivers/i3c/master/mipi-i3c-hci/core.c >> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c >> @@ -33,6 +33,7 @@ >> #define reg_clear(r, v) reg_write(r, reg_read(r) & ~(v)) >> #define HCI_VERSION 0x00 /* HCI Version (in BCD) */ >> +#define HCI_VERSION_V1 0x100 /* MIPI HCI Version >> number V1.0 */ >> #define HC_CONTROL 0x04 >> #define HC_CONTROL_BUS_ENABLE BIT(31) >> @@ -756,7 +757,7 @@ static int i3c_hci_init(struct i3c_hci *hci) >> /* Try activating DMA operations first */ >> if (hci->RHS_regs) { >> reg_clear(HC_CONTROL, HC_CONTROL_PIO_MODE); >> - if (reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE) { >> + if (regval > HCI_VERSION_V1 && !(reg_read(HC_CONTROL) & >> HC_CONTROL_PIO_MODE)) { >> dev_err(&hci->master.dev, "PIO mode is stuck\n"); >> ret = -EIO; >> } else { > > Here's typo and logic is reversed. ah! good catch! thanks, will change this in the next version. Thanks, Shyam
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index 24dd4603d6c6..a16da70bdfe1 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -33,6 +33,7 @@ #define reg_clear(r, v) reg_write(r, reg_read(r) & ~(v)) #define HCI_VERSION 0x00 /* HCI Version (in BCD) */ +#define HCI_VERSION_V1 0x100 /* MIPI HCI Version number V1.0 */ #define HC_CONTROL 0x04 #define HC_CONTROL_BUS_ENABLE BIT(31) @@ -756,7 +757,7 @@ static int i3c_hci_init(struct i3c_hci *hci) /* Try activating DMA operations first */ if (hci->RHS_regs) { reg_clear(HC_CONTROL, HC_CONTROL_PIO_MODE); - if (reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE) { + if (regval > HCI_VERSION_V1 && !(reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE)) { dev_err(&hci->master.dev, "PIO mode is stuck\n"); ret = -EIO; } else { @@ -768,7 +769,7 @@ static int i3c_hci_init(struct i3c_hci *hci) /* If no DMA, try PIO */ if (!hci->io && hci->PIO_regs) { reg_set(HC_CONTROL, HC_CONTROL_PIO_MODE); - if (!(reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE)) { + if (regval > HCI_VERSION_V1 && !(reg_read(HC_CONTROL) & HC_CONTROL_PIO_MODE)) { dev_err(&hci->master.dev, "DMA mode is stuck\n"); ret = -EIO; } else {
The HC_CONTROL_PIO_MODE bit was introduced in the HC_CONTROL register starting from version 1.1. Therefore, checking the HC_CONTROL_PIO_MODE bit on hardware that adheres to older specification revisions (i.e., versions earlier than 1.1) is incorrect. To address this, add an additional check to read the HCI version before attempting to read the HC_CONTROL_PIO_MODE status. Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> --- drivers/i3c/master/mipi-i3c-hci/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)