* [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem @ 2019-02-26 14:28 Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Przemyslaw Gaj @ 2019-02-26 14:28 UTC (permalink / raw) To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares I3C MIPI spec does not clearly say that I2C 10bit devices are supported. Additionally, it isn't possible to pass 10 bit device properly through DEFSLVS command and reserve address for such device on secondary master side. Przemyslaw Gaj (3): i3c: Drop support for I2C 10 bit addresing i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master dt-bindings: i3c: Document dropped support for I2C 10 bit devices Documentation/devicetree/bindings/i3c/i3c.txt | 4 +++- drivers/i3c/master.c | 9 +++++++++ drivers/i3c/master/i3c-master-cdns.c | 4 +--- 3 files changed, 13 insertions(+), 4 deletions(-) -- 2.8.3 _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing 2019-02-26 14:28 [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem Przemyslaw Gaj @ 2019-02-26 14:28 ` Przemyslaw Gaj 2019-02-27 12:05 ` vitor 2019-02-26 14:28 ` [PATCH v2 2/3] i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 3/3] dt-bindings: i3c: Document dropped support for I2C 10 bit devices Przemyslaw Gaj 2 siblings, 1 reply; 8+ messages in thread From: Przemyslaw Gaj @ 2019-02-26 14:28 UTC (permalink / raw) To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares This patch dropps support for I2C devices with 10 bit addressing. When I2C device with 10 bit address is defined in DT, I3C master registration fails. Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> --- Main changes between v1 and v2 are: - Add error message when registering I2C device with 10 bit address. --- drivers/i3c/master.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index 2dc628d..8c1e365 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, if (ret) return ret; + /* The I3C Specification does not clearly say I2C devices with 10-bit + * address are supported. These devices can't be passed properly through + * DEFSLVS command. + */ + if (boardinfo->base.flags & I2C_CLIENT_TEN) { + dev_err(&master->dev, "I2C device with 10 bit address not supported."); + return -ENOTSUPP; + } + /* LVR is encoded in reg[2]. */ boardinfo->lvr = reg[2]; -- 2.8.3 _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing 2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj @ 2019-02-27 12:05 ` vitor 2019-02-27 12:10 ` Boris Brezillon 0 siblings, 1 reply; 8+ messages in thread From: vitor @ 2019-02-27 12:05 UTC (permalink / raw) To: Przemyslaw Gaj, bbrezillon; +Cc: linux-i3c, rafalc, vitor.soares On 26/02/19 14:28, Przemyslaw Gaj wrote: > This patch dropps support for I2C devices with 10 bit addressing. When I2C > device with 10 bit address is defined in DT, I3C master registration fails. > > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > > --- > > Main changes between v1 and v2 are: > - Add error message when registering I2C device with 10 bit address. > > --- > drivers/i3c/master.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index 2dc628d..8c1e365 100644 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, > if (ret) > return ret; > > + /* The I3C Specification does not clearly say I2C devices with 10-bit > + * address are supported. These devices can't be passed properly through > + * DEFSLVS command. > + */ IMO we just need to say 10-bit address not used or not supported in i3c. > + if (boardinfo->base.flags & I2C_CLIENT_TEN) { > + dev_err(&master->dev, "I2C device with 10 bit address not supported."); > + return -ENOTSUPP; > + } > + > /* LVR is encoded in reg[2]. */ > boardinfo->lvr = reg[2]; > Also need to change: #define I2C_MAX_ADDR GENMASK(9, 0) to #define I2C_MAX_ADDR GENMASK(6, 0) in master.h file Not sure about: unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG]; @Boris can you check this part? Best regards, Vitor Soares _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing 2019-02-27 12:05 ` vitor @ 2019-02-27 12:10 ` Boris Brezillon 2019-02-27 12:52 ` vitor 0 siblings, 1 reply; 8+ messages in thread From: Boris Brezillon @ 2019-02-27 12:10 UTC (permalink / raw) To: vitor; +Cc: Przemyslaw Gaj, linux-i3c, rafalc, bbrezillon Hi Vitor, On Wed, 27 Feb 2019 12:05:30 +0000 vitor <vitor.soares@synopsys.com> wrote: > On 26/02/19 14:28, Przemyslaw Gaj wrote: > > This patch dropps support for I2C devices with 10 bit addressing. When I2C > > device with 10 bit address is defined in DT, I3C master registration fails. > > > > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > > > > --- > > > > Main changes between v1 and v2 are: > > - Add error message when registering I2C device with 10 bit address. > > > > --- > > drivers/i3c/master.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > index 2dc628d..8c1e365 100644 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, > > if (ret) > > return ret; > > > > + /* The I3C Specification does not clearly say I2C devices with 10-bit > > + * address are supported. These devices can't be passed properly through > > + * DEFSLVS command. > > + */ > > IMO we just need to say 10-bit address not used or not supported in i3c. I'd like to keep a clear comment on why it cannot supported right now even though the spec is unclear about that. If the spec is updated to state that I2C devs using extended addresses are forbidden, then we'll update this comment accordingly. > > > + if (boardinfo->base.flags & I2C_CLIENT_TEN) { > > + dev_err(&master->dev, "I2C device with 10 bit address not supported."); > > + return -ENOTSUPP; > > + } > > + > > /* LVR is encoded in reg[2]. */ > > boardinfo->lvr = reg[2]; > > > > Also need to change: > > #define I2C_MAX_ADDR GENMASK(9, 0) > to > #define I2C_MAX_ADDR GENMASK(6, 0) > in master.h file Yep, you can reduce the address space. > > Not sure about: > unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG]; > @Boris can you check this part? This part is still valid, no need to update it as you already updated I2C_MAX_ADDR. Thanks for the review. Boris _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing 2019-02-27 12:10 ` Boris Brezillon @ 2019-02-27 12:52 ` vitor 2019-02-27 13:09 ` Boris Brezillon 0 siblings, 1 reply; 8+ messages in thread From: vitor @ 2019-02-27 12:52 UTC (permalink / raw) To: Boris Brezillon, vitor; +Cc: Przemyslaw Gaj, linux-i3c, rafalc, bbrezillon Hi, On 27/02/19 12:10, Boris Brezillon wrote: > Hi Vitor, > > On Wed, 27 Feb 2019 12:05:30 +0000 > vitor <vitor.soares@synopsys.com> wrote: > >> On 26/02/19 14:28, Przemyslaw Gaj wrote: >>> This patch dropps support for I2C devices with 10 bit addressing. When I2C >>> device with 10 bit address is defined in DT, I3C master registration fails. >>> >>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> >>> >>> --- >>> >>> Main changes between v1 and v2 are: >>> - Add error message when registering I2C device with 10 bit address. >>> >>> --- >>> drivers/i3c/master.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c >>> index 2dc628d..8c1e365 100644 >>> --- a/drivers/i3c/master.c >>> +++ b/drivers/i3c/master.c >>> @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, >>> if (ret) >>> return ret; >>> >>> + /* The I3C Specification does not clearly say I2C devices with 10-bit >>> + * address are supported. These devices can't be passed properly through >>> + * DEFSLVS command. >>> + */ >> IMO we just need to say 10-bit address not used or not supported in i3c. > I'd like to keep a clear comment on why it cannot supported right now > even though the spec is unclear about that. If the spec is updated to > state that I2C devs using extended addresses are forbidden, then we'll > update this comment accordingly. I'm not sure if the terms aren't the same, but let's keep the comment. > >>> + if (boardinfo->base.flags & I2C_CLIENT_TEN) { >>> + dev_err(&master->dev, "I2C device with 10 bit address not supported."); >>> + return -ENOTSUPP; >>> + } >>> + >>> /* LVR is encoded in reg[2]. */ >>> boardinfo->lvr = reg[2]; >>> >> Also need to change: >> >> #define I2C_MAX_ADDR GENMASK(9, 0) >> to >> #define I2C_MAX_ADDR GENMASK(6, 0) >> in master.h file > Yep, you can reduce the address space. > >> Not sure about: >> unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG]; >> @Boris can you check this part? > This part is still valid, no need to update it as you already updated > I2C_MAX_ADDR. > > Thanks for the review. > > Boris Also we can make i2c_algorithm.functionality = I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C, and in this way we won't need ->i2c_func() and avoid the I2C_FUNC_10BIT_ADDR flag on host controller side. Best regards, Vitor Soares _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing 2019-02-27 12:52 ` vitor @ 2019-02-27 13:09 ` Boris Brezillon 0 siblings, 0 replies; 8+ messages in thread From: Boris Brezillon @ 2019-02-27 13:09 UTC (permalink / raw) To: vitor; +Cc: Przemyslaw Gaj, linux-i3c, rafalc, bbrezillon On Wed, 27 Feb 2019 12:52:42 +0000 vitor <vitor.soares@synopsys.com> wrote: > Hi, > > On 27/02/19 12:10, Boris Brezillon wrote: > > Hi Vitor, > > > > On Wed, 27 Feb 2019 12:05:30 +0000 > > vitor <vitor.soares@synopsys.com> wrote: > > > >> On 26/02/19 14:28, Przemyslaw Gaj wrote: > >>> This patch dropps support for I2C devices with 10 bit addressing. When I2C > >>> device with 10 bit address is defined in DT, I3C master registration fails. > >>> > >>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >>> > >>> --- > >>> > >>> Main changes between v1 and v2 are: > >>> - Add error message when registering I2C device with 10 bit address. > >>> > >>> --- > >>> drivers/i3c/master.c | 9 +++++++++ > >>> 1 file changed, 9 insertions(+) > >>> > >>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > >>> index 2dc628d..8c1e365 100644 > >>> --- a/drivers/i3c/master.c > >>> +++ b/drivers/i3c/master.c > >>> @@ -1962,6 +1962,15 @@ of_i3c_master_add_i2c_boardinfo(struct i3c_master_controller *master, > >>> if (ret) > >>> return ret; > >>> > >>> + /* The I3C Specification does not clearly say I2C devices with 10-bit Nitpick: please do not use net-style comments in the I3C subsystem. Use /* * blablabla */ instead. > >>> + * address are supported. These devices can't be passed properly through > >>> + * DEFSLVS command. > >>> + */ > >> IMO we just need to say 10-bit address not used or not supported in i3c. > > I'd like to keep a clear comment on why it cannot supported right now > > even though the spec is unclear about that. If the spec is updated to > > state that I2C devs using extended addresses are forbidden, then we'll > > update this comment accordingly. > > I'm not sure if the terms aren't the same, Don't understand what you mean, sorry. >but let's keep the comment. > > > > >>> + if (boardinfo->base.flags & I2C_CLIENT_TEN) { > >>> + dev_err(&master->dev, "I2C device with 10 bit address not supported."); > >>> + return -ENOTSUPP; > >>> + } > >>> + > >>> /* LVR is encoded in reg[2]. */ > >>> boardinfo->lvr = reg[2]; > >>> > >> Also need to change: > >> > >> #define I2C_MAX_ADDR GENMASK(9, 0) > >> to > >> #define I2C_MAX_ADDR GENMASK(6, 0) > >> in master.h file > > Yep, you can reduce the address space. > > > >> Not sure about: > >> unsigned long addrslots[((I2C_MAX_ADDR + 1) * 2) / BITS_PER_LONG]; > >> @Boris can you check this part? > > This part is still valid, no need to update it as you already updated > > I2C_MAX_ADDR. > > > > Thanks for the review. > > > > Boris > > Also we can make i2c_algorithm.functionality = I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C, and in this way we won't need ->i2c_func() and avoid the I2C_FUNC_10BIT_ADDR flag on host controller side. Ack. Let's rip ->i2c_func() out and return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C for everyone. We'll add it back if some drivers want to support SMBUS natively or if 10bit addressing appears to be needed at some point. _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master 2019-02-26 14:28 [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj @ 2019-02-26 14:28 ` Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 3/3] dt-bindings: i3c: Document dropped support for I2C 10 bit devices Przemyslaw Gaj 2 siblings, 0 replies; 8+ messages in thread From: Przemyslaw Gaj @ 2019-02-26 14:28 UTC (permalink / raw) To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares Because this patch series dropped support for 10 bit I2C devices, support is also dropped in Cadence I3C master driver. Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> --- drivers/i3c/master/i3c-master-cdns.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c index 8889a4f..1797fbf 100644 --- a/drivers/i3c/master/i3c-master-cdns.c +++ b/drivers/i3c/master/i3c-master-cdns.c @@ -1010,9 +1010,7 @@ static int cdns_i3c_master_attach_i2c_dev(struct i2c_dev_desc *dev) master->free_rr_slots &= ~BIT(slot); i2c_dev_set_master_data(dev, data); - writel(prepare_rr0_dev_address(dev->boardinfo->base.addr) | - (dev->boardinfo->base.flags & I2C_CLIENT_TEN ? - DEV_ID_RR0_LVR_EXT_ADDR : 0), + writel(prepare_rr0_dev_address(dev->boardinfo->base.addr), master->regs + DEV_ID_RR0(data->id)); writel(dev->boardinfo->lvr, master->regs + DEV_ID_RR2(data->id)); writel(readl(master->regs + DEVS_CTRL) | -- 2.8.3 _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] dt-bindings: i3c: Document dropped support for I2C 10 bit devices 2019-02-26 14:28 [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 2/3] i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master Przemyslaw Gaj @ 2019-02-26 14:28 ` Przemyslaw Gaj 2 siblings, 0 replies; 8+ messages in thread From: Przemyslaw Gaj @ 2019-02-26 14:28 UTC (permalink / raw) To: bbrezillon; +Cc: linux-i3c, Przemyslaw Gaj, rafalc, vitor.soares Because this patch series dropped support for 10 bit I2C devices, I'm documenting this. Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> --- Documentation/devicetree/bindings/i3c/i3c.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt b/Documentation/devicetree/bindings/i3c/i3c.txt index ab729a0..4ffe059 100644 --- a/Documentation/devicetree/bindings/i3c/i3c.txt +++ b/Documentation/devicetree/bindings/i3c/i3c.txt @@ -39,7 +39,9 @@ valid here, but several new properties have been added. New constraint on existing properties: -------------------------------------- - reg: contains 3 cells - + first cell : still encoding the I2C address + + first cell : still encoding the I2C address. 10 bit addressing is not + supported. Devices with 10 bit address can't be properly passed through + DEFSLVS command. + second cell: shall be 0 -- 2.8.3 _______________________________________________ linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-27 13:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-26 14:28 [PATCH v2 0/3] Drop support for I2C 10 bit devices from I3C subsystem Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 1/3] i3c: Drop support for I2C 10 bit addresing Przemyslaw Gaj 2019-02-27 12:05 ` vitor 2019-02-27 12:10 ` Boris Brezillon 2019-02-27 12:52 ` vitor 2019-02-27 13:09 ` Boris Brezillon 2019-02-26 14:28 ` [PATCH v2 2/3] i3c: master: cdns: Drop support for I2C 10 bit addresing in Cadence I3C master Przemyslaw Gaj 2019-02-26 14:28 ` [PATCH v2 3/3] dt-bindings: i3c: Document dropped support for I2C 10 bit devices Przemyslaw Gaj
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).