| Message ID | 20190310135843.21154-2-pgaj@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add the I3C mastership request | expand |
On Sun, 10 Mar 2019 13:58:38 +0000 Przemyslaw Gaj <pgaj@cadence.com> wrote: > I need to store address and lvr value for I2C devices without static definition > in DT. This allows secondary master to transmit DEFSLVS command properly. > > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > --- > include/linux/i3c/master.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > index eca8337..3c27d9f 100644 > --- a/include/linux/i3c/master.h > +++ b/include/linux/i3c/master.h > @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > * @common: common part of the I2C device descriptor > * @boardinfo: pointer to the boardinfo attached to this I2C device > * @dev: I2C device object registered to the I2C framework > + * @addr: I2C device address > + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > + * the I2C device limitations > * > * Each I2C device connected on the bus will have an i2c_dev_desc. > * This object is created by the core and later attached to the controller > @@ -84,6 +87,8 @@ struct i2c_dev_desc { > struct i3c_i2c_dev_desc common; > const struct i2c_dev_boardinfo *boardinfo; > struct i2c_client *dev; > + u16 addr; > + u8 lvr; You also need to remove lvr from i2c_dev_boardinfo and adjust the code to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in patch 3. > }; > > /**
Hi, On 30/03/19 14:36, Boris Brezillon wrote: > On Sun, 10 Mar 2019 13:58:38 +0000 > Przemyslaw Gaj <pgaj@cadence.com> wrote: > >> I need to store address and lvr value for I2C devices without static definition >> in DT. This allows secondary master to transmit DEFSLVS command properly. >> >> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> >> --- >> include/linux/i3c/master.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h >> index eca8337..3c27d9f 100644 >> --- a/include/linux/i3c/master.h >> +++ b/include/linux/i3c/master.h >> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { >> * @common: common part of the I2C device descriptor >> * @boardinfo: pointer to the boardinfo attached to this I2C device >> * @dev: I2C device object registered to the I2C framework >> + * @addr: I2C device address >> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about >> + * the I2C device limitations >> * >> * Each I2C device connected on the bus will have an i2c_dev_desc. >> * This object is created by the core and later attached to the controller >> @@ -84,6 +87,8 @@ struct i2c_dev_desc { >> struct i3c_i2c_dev_desc common; >> const struct i2c_dev_boardinfo *boardinfo; >> struct i2c_client *dev; >> + u16 addr; >> + u8 lvr; > You also need to remove lvr from i2c_dev_boardinfo and adjust the code > to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > patch 3. > Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? >> }; >> >> /**
On Mon, 1 Apr 2019 19:17:03 +0100 vitor <vitor.soares@synopsys.com> wrote: > Hi, > > On 30/03/19 14:36, Boris Brezillon wrote: > > On Sun, 10 Mar 2019 13:58:38 +0000 > > Przemyslaw Gaj <pgaj@cadence.com> wrote: > > > >> I need to store address and lvr value for I2C devices without static definition > >> in DT. This allows secondary master to transmit DEFSLVS command properly. > >> > >> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >> --- > >> include/linux/i3c/master.h | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > >> index eca8337..3c27d9f 100644 > >> --- a/include/linux/i3c/master.h > >> +++ b/include/linux/i3c/master.h > >> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > >> * @common: common part of the I2C device descriptor > >> * @boardinfo: pointer to the boardinfo attached to this I2C device > >> * @dev: I2C device object registered to the I2C framework > >> + * @addr: I2C device address > >> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > >> + * the I2C device limitations > >> * > >> * Each I2C device connected on the bus will have an i2c_dev_desc. > >> * This object is created by the core and later attached to the controller > >> @@ -84,6 +87,8 @@ struct i2c_dev_desc { > >> struct i3c_i2c_dev_desc common; > >> const struct i2c_dev_boardinfo *boardinfo; > >> struct i2c_client *dev; > >> + u16 addr; > >> + u8 lvr; > > You also need to remove lvr from i2c_dev_boardinfo and adjust the code > > to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > > patch 3. > > > > Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? Because i2c_dev_boardinfo is extracted from the DT and the secondary slaves does not necessarily have this description. The idea is to keep reserving the address slot for the I2C device even if we don't expose it to the upper layers.
Hi, On 01/04/19 19:31, Boris Brezillon wrote: > On Mon, 1 Apr 2019 19:17:03 +0100 > vitor <vitor.soares@synopsys.com> wrote: > >> Hi, >> >> On 30/03/19 14:36, Boris Brezillon wrote: >>> On Sun, 10 Mar 2019 13:58:38 +0000 >>> Przemyslaw Gaj <pgaj@cadence.com> wrote: >>> >>>> I need to store address and lvr value for I2C devices without static definition >>>> in DT. This allows secondary master to transmit DEFSLVS command properly. >>>> >>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> >>>> --- >>>> include/linux/i3c/master.h | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h >>>> index eca8337..3c27d9f 100644 >>>> --- a/include/linux/i3c/master.h >>>> +++ b/include/linux/i3c/master.h >>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { >>>> * @common: common part of the I2C device descriptor >>>> * @boardinfo: pointer to the boardinfo attached to this I2C device >>>> * @dev: I2C device object registered to the I2C framework >>>> + * @addr: I2C device address >>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about >>>> + * the I2C device limitations >>>> * >>>> * Each I2C device connected on the bus will have an i2c_dev_desc. >>>> * This object is created by the core and later attached to the controller >>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { >>>> struct i3c_i2c_dev_desc common; >>>> const struct i2c_dev_boardinfo *boardinfo; >>>> struct i2c_client *dev; >>>> + u16 addr; >>>> + u8 lvr; >>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code >>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in >>> patch 3. >>> >> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? > Because i2c_dev_boardinfo is extracted from the DT and the secondary > slaves does not necessarily have this description. The idea is to keep > reserving the address slot for the I2C device even if we don't expose > it to the upper layers. So you are using i2c_dev_boardinfo just for DT devices? Because at end we need to register the new i2c devs.
Hi Vitor, The 04/01/2019 19:48, vitor wrote: > > Hi, > > On 01/04/19 19:31, Boris Brezillon wrote: > > On Mon, 1 Apr 2019 19:17:03 +0100 > > vitor <vitor.soares@synopsys.com> wrote: > > > >> Hi, > >> > >> On 30/03/19 14:36, Boris Brezillon wrote: > >>> On Sun, 10 Mar 2019 13:58:38 +0000 > >>> Przemyslaw Gaj <pgaj@cadence.com> wrote: > >>> > >>>> I need to store address and lvr value for I2C devices without static definition > >>>> in DT. This allows secondary master to transmit DEFSLVS command properly. > >>>> > >>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >>>> --- > >>>> include/linux/i3c/master.h | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > >>>> index eca8337..3c27d9f 100644 > >>>> --- a/include/linux/i3c/master.h > >>>> +++ b/include/linux/i3c/master.h > >>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > >>>> * @common: common part of the I2C device descriptor > >>>> * @boardinfo: pointer to the boardinfo attached to this I2C device > >>>> * @dev: I2C device object registered to the I2C framework > >>>> + * @addr: I2C device address > >>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > >>>> + * the I2C device limitations > >>>> * > >>>> * Each I2C device connected on the bus will have an i2c_dev_desc. > >>>> * This object is created by the core and later attached to the controller > >>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { > >>>> struct i3c_i2c_dev_desc common; > >>>> const struct i2c_dev_boardinfo *boardinfo; > >>>> struct i2c_client *dev; > >>>> + u16 addr; > >>>> + u8 lvr; > >>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code > >>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > >>> patch 3. > >>> > >> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? > > Because i2c_dev_boardinfo is extracted from the DT and the secondary > > slaves does not necessarily have this description. The idea is to keep > > reserving the address slot for the I2C device even if we don't expose > > it to the upper layers. > > So you are using i2c_dev_boardinfo just for DT devices? Because at end we need to register the new i2c devs. Yes, exactly. On secondary master side we are registering only I2C devices with DT definition.
On Mon, 1 Apr 2019 19:48:45 +0100 vitor <vitor.soares@synopsys.com> wrote: > Hi, > > On 01/04/19 19:31, Boris Brezillon wrote: > > On Mon, 1 Apr 2019 19:17:03 +0100 > > vitor <vitor.soares@synopsys.com> wrote: > > > >> Hi, > >> > >> On 30/03/19 14:36, Boris Brezillon wrote: > >>> On Sun, 10 Mar 2019 13:58:38 +0000 > >>> Przemyslaw Gaj <pgaj@cadence.com> wrote: > >>> > >>>> I need to store address and lvr value for I2C devices without static definition > >>>> in DT. This allows secondary master to transmit DEFSLVS command properly. > >>>> > >>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >>>> --- > >>>> include/linux/i3c/master.h | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > >>>> index eca8337..3c27d9f 100644 > >>>> --- a/include/linux/i3c/master.h > >>>> +++ b/include/linux/i3c/master.h > >>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > >>>> * @common: common part of the I2C device descriptor > >>>> * @boardinfo: pointer to the boardinfo attached to this I2C device > >>>> * @dev: I2C device object registered to the I2C framework > >>>> + * @addr: I2C device address > >>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > >>>> + * the I2C device limitations > >>>> * > >>>> * Each I2C device connected on the bus will have an i2c_dev_desc. > >>>> * This object is created by the core and later attached to the controller > >>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { > >>>> struct i3c_i2c_dev_desc common; > >>>> const struct i2c_dev_boardinfo *boardinfo; > >>>> struct i2c_client *dev; > >>>> + u16 addr; > >>>> + u8 lvr; > >>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code > >>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > >>> patch 3. > >>> > >> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? > > Because i2c_dev_boardinfo is extracted from the DT and the secondary > > slaves does not necessarily have this description. The idea is to keep > > reserving the address slot for the I2C device even if we don't expose > > it to the upper layers. > > So you are using i2c_dev_boardinfo just for DT devices? All devices that are described in some way, can be through DT, ACPI or board-file desc (only DT is supported right now, but we can easily add support for other formats). > Because at end we need to register the new i2c devs. No, only those that have a valid description, because there's nothing you can expose if you only have the address and LVR values. You at least need to know which driver this dev should be attached to (compatible string in your DT) and most drivers also need extra information (basically all the DT props that are described a the DT binding).
Hi, On 01/04/19 20:24, Boris Brezillon wrote: > On Mon, 1 Apr 2019 19:48:45 +0100 > vitor <vitor.soares@synopsys.com> wrote: > >> Hi, >> >> On 01/04/19 19:31, Boris Brezillon wrote: >>> On Mon, 1 Apr 2019 19:17:03 +0100 >>> vitor <vitor.soares@synopsys.com> wrote: >>> >>>> Hi, >>>> >>>> On 30/03/19 14:36, Boris Brezillon wrote: >>>>> On Sun, 10 Mar 2019 13:58:38 +0000 >>>>> Przemyslaw Gaj <pgaj@cadence.com> wrote: >>>>> >>>>>> I need to store address and lvr value for I2C devices without static definition >>>>>> in DT. This allows secondary master to transmit DEFSLVS command properly. >>>>>> >>>>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> >>>>>> --- >>>>>> include/linux/i3c/master.h | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h >>>>>> index eca8337..3c27d9f 100644 >>>>>> --- a/include/linux/i3c/master.h >>>>>> +++ b/include/linux/i3c/master.h >>>>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { >>>>>> * @common: common part of the I2C device descriptor >>>>>> * @boardinfo: pointer to the boardinfo attached to this I2C device >>>>>> * @dev: I2C device object registered to the I2C framework >>>>>> + * @addr: I2C device address >>>>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about >>>>>> + * the I2C device limitations >>>>>> * >>>>>> * Each I2C device connected on the bus will have an i2c_dev_desc. >>>>>> * This object is created by the core and later attached to the controller >>>>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { >>>>>> struct i3c_i2c_dev_desc common; >>>>>> const struct i2c_dev_boardinfo *boardinfo; >>>>>> struct i2c_client *dev; >>>>>> + u16 addr; >>>>>> + u8 lvr; >>>>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code >>>>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in >>>>> patch 3. >>>>> >>>> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? >>> Because i2c_dev_boardinfo is extracted from the DT and the secondary >>> slaves does not necessarily have this description. The idea is to keep >>> reserving the address slot for the I2C device even if we don't expose >>> it to the upper layers. >> So you are using i2c_dev_boardinfo just for DT devices? > All devices that are described in some way, can be through DT, ACPI or > board-file desc (only DT is supported right now, but we can easily add > support for other formats). We can drop i2c_dev_boardinfo. > >> Because at end we need to register the new i2c devs. > No, only those that have a valid description, because there's nothing > you can expose if you only have the address and LVR values. You at > least need to know which driver this dev should be attached to > (compatible string in your DT) and most drivers also need extra > information (basically all the DT props that are described a the DT > binding). In any case the HC need to know the bus mode, how is this done?
Hi, The 04/02/2019 12:10, vitor wrote: > > Hi, > > > On 01/04/19 20:24, Boris Brezillon wrote: > > On Mon, 1 Apr 2019 19:48:45 +0100 > > vitor <vitor.soares@synopsys.com> wrote: > > > >> Hi, > >> > >> On 01/04/19 19:31, Boris Brezillon wrote: > >>> On Mon, 1 Apr 2019 19:17:03 +0100 > >>> vitor <vitor.soares@synopsys.com> wrote: > >>> > >>>> Hi, > >>>> > >>>> On 30/03/19 14:36, Boris Brezillon wrote: > >>>>> On Sun, 10 Mar 2019 13:58:38 +0000 > >>>>> Przemyslaw Gaj <pgaj@cadence.com> wrote: > >>>>> > >>>>>> I need to store address and lvr value for I2C devices without static definition > >>>>>> in DT. This allows secondary master to transmit DEFSLVS command properly. > >>>>>> > >>>>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >>>>>> --- > >>>>>> include/linux/i3c/master.h | 5 +++++ > >>>>>> 1 file changed, 5 insertions(+) > >>>>>> > >>>>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > >>>>>> index eca8337..3c27d9f 100644 > >>>>>> --- a/include/linux/i3c/master.h > >>>>>> +++ b/include/linux/i3c/master.h > >>>>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > >>>>>> * @common: common part of the I2C device descriptor > >>>>>> * @boardinfo: pointer to the boardinfo attached to this I2C device > >>>>>> * @dev: I2C device object registered to the I2C framework > >>>>>> + * @addr: I2C device address > >>>>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > >>>>>> + * the I2C device limitations > >>>>>> * > >>>>>> * Each I2C device connected on the bus will have an i2c_dev_desc. > >>>>>> * This object is created by the core and later attached to the controller > >>>>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { > >>>>>> struct i3c_i2c_dev_desc common; > >>>>>> const struct i2c_dev_boardinfo *boardinfo; > >>>>>> struct i2c_client *dev; > >>>>>> + u16 addr; > >>>>>> + u8 lvr; > >>>>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code > >>>>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > >>>>> patch 3. > >>>>> > >>>> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? > >>> Because i2c_dev_boardinfo is extracted from the DT and the secondary > >>> slaves does not necessarily have this description. The idea is to keep > >>> reserving the address slot for the I2C device even if we don't expose > >>> it to the upper layers. > >> So you are using i2c_dev_boardinfo just for DT devices? > > All devices that are described in some way, can be through DT, ACPI or > > board-file desc (only DT is supported right now, but we can easily add > > support for other formats). > > We can drop i2c_dev_boardinfo. > Why you think we can drop it? We are still using it for DT devices. > > > >> Because at end we need to register the new i2c devs. > > No, only those that have a valid description, because there's nothing > > you can expose if you only have the address and LVR values. You at > > least need to know which driver this dev should be attached to > > (compatible string in your DT) and most drivers also need extra > > information (basically all the DT props that are described a the DT > > binding). > > In any case the HC need to know the bus mode, how is this done? LVR is passed through DEFSLVS command.
On 02/04/19 12:22, Przemyslaw Gaj wrote: > Hi, > > The 04/02/2019 12:10, vitor wrote: >> Hi, >> >> >> On 01/04/19 20:24, Boris Brezillon wrote: >>> On Mon, 1 Apr 2019 19:48:45 +0100 >>> vitor <vitor.soares@synopsys.com> wrote: >>> >>>> Hi, >>>> >>>> On 01/04/19 19:31, Boris Brezillon wrote: >>>>> On Mon, 1 Apr 2019 19:17:03 +0100 >>>>> vitor <vitor.soares@synopsys.com> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> On 30/03/19 14:36, Boris Brezillon wrote: >>>>>>> On Sun, 10 Mar 2019 13:58:38 +0000 >>>>>>> Przemyslaw Gaj <pgaj@cadence.com> wrote: >>>>>>> >>>>>>>> I need to store address and lvr value for I2C devices without static definition >>>>>>>> in DT. This allows secondary master to transmit DEFSLVS command properly. >>>>>>>> >>>>>>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> >>>>>>>> --- >>>>>>>> include/linux/i3c/master.h | 5 +++++ >>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>> >>>>>>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h >>>>>>>> index eca8337..3c27d9f 100644 >>>>>>>> --- a/include/linux/i3c/master.h >>>>>>>> +++ b/include/linux/i3c/master.h >>>>>>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { >>>>>>>> * @common: common part of the I2C device descriptor >>>>>>>> * @boardinfo: pointer to the boardinfo attached to this I2C device >>>>>>>> * @dev: I2C device object registered to the I2C framework >>>>>>>> + * @addr: I2C device address >>>>>>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about >>>>>>>> + * the I2C device limitations >>>>>>>> * >>>>>>>> * Each I2C device connected on the bus will have an i2c_dev_desc. >>>>>>>> * This object is created by the core and later attached to the controller >>>>>>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { >>>>>>>> struct i3c_i2c_dev_desc common; >>>>>>>> const struct i2c_dev_boardinfo *boardinfo; >>>>>>>> struct i2c_client *dev; >>>>>>>> + u16 addr; >>>>>>>> + u8 lvr; >>>>>>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code >>>>>>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in >>>>>>> patch 3. >>>>>>> >>>>>> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? >>>>> Because i2c_dev_boardinfo is extracted from the DT and the secondary >>>>> slaves does not necessarily have this description. The idea is to keep >>>>> reserving the address slot for the I2C device even if we don't expose >>>>> it to the upper layers. >>>> So you are using i2c_dev_boardinfo just for DT devices? >>> All devices that are described in some way, can be through DT, ACPI or >>> board-file desc (only DT is supported right now, but we can easily add >>> support for other formats). >> We can drop i2c_dev_boardinfo. >> > Why you think we can drop it? We are still using it for DT devices. Until now you need it to hold LVR, if you pass it to i2c_dev_desc, the i2c_board_info can be used for DT. > >>>> Because at end we need to register the new i2c devs. >>> No, only those that have a valid description, because there's nothing >>> you can expose if you only have the address and LVR values. You at >>> least need to know which driver this dev should be attached to >>> (compatible string in your DT) and most drivers also need extra >>> information (basically all the DT props that are described a the DT >>> binding). >> In any case the HC need to know the bus mode, how is this done? > LVR is passed through DEFSLVS command. Is HC driver who set the bus mode? Shouldn't be the subsystem when you register all devices?
On Sat, 30 Mar 2019 15:36:18 +0100 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Sun, 10 Mar 2019 13:58:38 +0000 > Przemyslaw Gaj <pgaj@cadence.com> wrote: > > > I need to store address and lvr value for I2C devices without static definition > > in DT. This allows secondary master to transmit DEFSLVS command properly. > > > > Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > > --- > > include/linux/i3c/master.h | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > > index eca8337..3c27d9f 100644 > > --- a/include/linux/i3c/master.h > > +++ b/include/linux/i3c/master.h > > @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > > * @common: common part of the I2C device descriptor > > * @boardinfo: pointer to the boardinfo attached to this I2C device > > * @dev: I2C device object registered to the I2C framework > > + * @addr: I2C device address > > + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > > + * the I2C device limitations > > * > > * Each I2C device connected on the bus will have an i2c_dev_desc. > > * This object is created by the core and later attached to the controller > > @@ -84,6 +87,8 @@ struct i2c_dev_desc { > > struct i3c_i2c_dev_desc common; > > const struct i2c_dev_boardinfo *boardinfo; > > struct i2c_client *dev; > > + u16 addr; > > + u8 lvr; > > You also need to remove lvr from i2c_dev_boardinfo and adjust the code > to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > patch 3. Hm, looks like I was wrong, you must keep i2c_dev_boardinfo->lvr as i2c_dev_desc instances and i2c_dev_boardinfo objects are created separately.
On Tue, 2 Apr 2019 12:32:41 +0100 vitor <vitor.soares@synopsys.com> wrote: > On 02/04/19 12:22, Przemyslaw Gaj wrote: > > Hi, > > > > The 04/02/2019 12:10, vitor wrote: > >> Hi, > >> > >> > >> On 01/04/19 20:24, Boris Brezillon wrote: > >>> On Mon, 1 Apr 2019 19:48:45 +0100 > >>> vitor <vitor.soares@synopsys.com> wrote: > >>> > >>>> Hi, > >>>> > >>>> On 01/04/19 19:31, Boris Brezillon wrote: > >>>>> On Mon, 1 Apr 2019 19:17:03 +0100 > >>>>> vitor <vitor.soares@synopsys.com> wrote: > >>>>> > >>>>>> Hi, > >>>>>> > >>>>>> On 30/03/19 14:36, Boris Brezillon wrote: > >>>>>>> On Sun, 10 Mar 2019 13:58:38 +0000 > >>>>>>> Przemyslaw Gaj <pgaj@cadence.com> wrote: > >>>>>>> > >>>>>>>> I need to store address and lvr value for I2C devices without static definition > >>>>>>>> in DT. This allows secondary master to transmit DEFSLVS command properly. > >>>>>>>> > >>>>>>>> Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> > >>>>>>>> --- > >>>>>>>> include/linux/i3c/master.h | 5 +++++ > >>>>>>>> 1 file changed, 5 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h > >>>>>>>> index eca8337..3c27d9f 100644 > >>>>>>>> --- a/include/linux/i3c/master.h > >>>>>>>> +++ b/include/linux/i3c/master.h > >>>>>>>> @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { > >>>>>>>> * @common: common part of the I2C device descriptor > >>>>>>>> * @boardinfo: pointer to the boardinfo attached to this I2C device > >>>>>>>> * @dev: I2C device object registered to the I2C framework > >>>>>>>> + * @addr: I2C device address > >>>>>>>> + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about > >>>>>>>> + * the I2C device limitations > >>>>>>>> * > >>>>>>>> * Each I2C device connected on the bus will have an i2c_dev_desc. > >>>>>>>> * This object is created by the core and later attached to the controller > >>>>>>>> @@ -84,6 +87,8 @@ struct i2c_dev_desc { > >>>>>>>> struct i3c_i2c_dev_desc common; > >>>>>>>> const struct i2c_dev_boardinfo *boardinfo; > >>>>>>>> struct i2c_client *dev; > >>>>>>>> + u16 addr; > >>>>>>>> + u8 lvr; > >>>>>>> You also need to remove lvr from i2c_dev_boardinfo and adjust the code > >>>>>>> to use i2c_dev_desc->addr and i2c_dev_desc->lvr in this patch, not in > >>>>>>> patch 3. > >>>>>>> > >>>>>> Why can't we keep the lvr and addr in i2c_dev_boardinfo and need this information on i2c_dev_desc? > >>>>> Because i2c_dev_boardinfo is extracted from the DT and the secondary > >>>>> slaves does not necessarily have this description. The idea is to keep > >>>>> reserving the address slot for the I2C device even if we don't expose > >>>>> it to the upper layers. > >>>> So you are using i2c_dev_boardinfo just for DT devices? > >>> All devices that are described in some way, can be through DT, ACPI or > >>> board-file desc (only DT is supported right now, but we can easily add > >>> support for other formats). > >> We can drop i2c_dev_boardinfo. > >> > > Why you think we can drop it? We are still using it for DT devices. > > Until now you need it to hold LVR, if you pass it to i2c_dev_desc, the i2c_board_info can be used for DT. It also contains a list node which we need to attach the object to the i2c boardinfo list. Anyway, looks like I was wrong and i2c_board_info->lvr is still needed. > > > > >>>> Because at end we need to register the new i2c devs. > >>> No, only those that have a valid description, because there's nothing > >>> you can expose if you only have the address and LVR values. You at > >>> least need to know which driver this dev should be attached to > >>> (compatible string in your DT) and most drivers also need extra > >>> information (basically all the DT props that are described a the DT > >>> binding). > >> In any case the HC need to know the bus mode, how is this done? > > LVR is passed through DEFSLVS command. > > Is HC driver who set the bus mode? Shouldn't be the subsystem when you register all devices? > The framework selects the mode based on the devices attached to the bus. The reception of a DEFSLVS frame should trigger the creation of I3C/I2C devices which will in turn update the bus mode. Of course, the HC driver must update the bus config accordingly.
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h index eca8337..3c27d9f 100644 --- a/include/linux/i3c/master.h +++ b/include/linux/i3c/master.h @@ -71,6 +71,9 @@ struct i2c_dev_boardinfo { * @common: common part of the I2C device descriptor * @boardinfo: pointer to the boardinfo attached to this I2C device * @dev: I2C device object registered to the I2C framework + * @addr: I2C device address + * @lvr: LVR (Legacy Virtual Register) needed by the I3C core to know about + * the I2C device limitations * * Each I2C device connected on the bus will have an i2c_dev_desc. * This object is created by the core and later attached to the controller @@ -84,6 +87,8 @@ struct i2c_dev_desc { struct i3c_i2c_dev_desc common; const struct i2c_dev_boardinfo *boardinfo; struct i2c_client *dev; + u16 addr; + u8 lvr; }; /**
I need to store address and lvr value for I2C devices without static definition in DT. This allows secondary master to transmit DEFSLVS command properly. Signed-off-by: Przemyslaw Gaj <pgaj@cadence.com> --- include/linux/i3c/master.h | 5 +++++ 1 file changed, 5 insertions(+)