[U-Boot] [PATCH v2 0/7] Align U-Boot I2C DM bus ID handling with Linux

Fri Feb 8 06:45:04 UTC 2019

Hi Simon,

On 08. 02. 19 5:11, Simon Glass wrote:
> Hi Michal,
> 
> On Tue, 5 Feb 2019 at 00:58, Michal Simek <michal.simek at xilinx.com> wrote:
>>
>> Hi Simon,
>>
>> On 02. 02. 19 15:10, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On Thu, 31 Jan 2019 at 08:31, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>
>>>> U-Boot with I2C_DM enabled is not capable to list i2c busses connected
>>>> to i2c mux. For getting this work there is a need to find out highest
>>>> alias ID and use this uniq number for new buses connected to I2C mux.
>>>> This series is making this happen.
>>>>
>>>> There is only one missing piece which is that also i2c controllers which
>>>> are not listed in DT are not using this feature.
>>>>
>>>> Removing setting up aliases from i2c mux code and unifying it in the
>>>> same code ensures that numbering schema is proper if no alias is
>>>> specified.
>>>>
>>>> ZynqMP> i2c bus
>>>> Bus 0:  i2c at ff020000
>>>>    20: gpio at 20, offset len 1, flags 0
>>>>    21: gpio at 21, offset len 1, flags 0
>>>>    75: i2c-mux at 75, offset len 1, flags 0
>>>> Bus 1:  i2c at ff020000->i2c-mux at 75->i2c at 0
>>>> Bus 2:  i2c at ff020000->i2c-mux at 75->i2c at 1
>>>> Bus 3:  i2c at ff020000->i2c-mux at 75->i2c at 2
>>>> Bus 4:  i2c at ff030000  (active 4)
>>>>    74: i2c-mux at 74, offset len 1, flags 0
>>>>    75: i2c-mux at 75, offset len 1, flags 0
>>>> Bus 5:  i2c at ff030000->i2c-mux at 74->i2c at 0  (active 5)
>>>>    54: eeprom at 54, offset len 1, flags 0
>>>> Bus 6:  i2c at ff030000->i2c-mux at 74->i2c at 1
>>>> Bus 7:  i2c at ff030000->i2c-mux at 74->i2c at 2
>>>> Bus 8:  i2c at ff030000->i2c-mux at 74->i2c at 3
>>>> Bus 9:  i2c at ff030000->i2c-mux at 74->i2c at 4
>>>> Bus 10: i2c at ff030000->i2c-mux at 75->i2c at 0
>>>> Bus 11: i2c at ff030000->i2c-mux at 75->i2c at 1
>>>> Bus 12: i2c at ff030000->i2c-mux at 75->i2c at 2
>>>> Bus 13: i2c at ff030000->i2c-mux at 75->i2c at 3
>>>> Bus 14: i2c at ff030000->i2c-mux at 75->i2c at 4
>>>> Bus 15: i2c at ff030000->i2c-mux at 75->i2c at 5
>>>> Bus 16: i2c at ff030000->i2c-mux at 75->i2c at 6
>>>> Bus 17: i2c at ff030000->i2c-mux at 75->i2c at 7
>>>>
>>>> Thanks,
>>>> Michal
>>>>
>>>> Changes in v2:
>>>> - Update kernel-doc binding
>>>> - Return -1 in case of error. -1 means that the next free alias is 0.
>>>> - New patch
>>>> - New patch
>>>> - Use dev_read_alias_highest_id()
>>>> - Use uclass private data
>>>> - Use private uclass data
>>>> - Fix headers
>>>> - Change patch description to focus only on bus name
>>>
>>> I don't have strong objections to this series, but I'd still like to
>>> try a bit harder on the existing req_seq/seq stuff.
>>>
>>> I don't think we necessarily need to set the 'seq' before probe,
>>> although I suppose we could.
>>>
>>> But is there anything to stop us moving some of the logic which sets
>>> seq to setting req_seq? We could check the aliases and then set
>>> req_seq during bind(), perhaps?
>>
>> Let me put this to my TODO list. But it is not a task which you know
>> will be done in some hours. It requires to study the whole logic if this
>> works for all cases.
>>
>>> This would be better for code size since it would help all subsystems.
>>
>> This approach is used in the linux kernel that's why there is no way to
>> get rid of dev_read_alias_highest_id() as function.
> 
> Yes that function seems useful to me.
> 
>> It means only i2c_uclass_init() and little code in i2c_post_bind() could
>> be removed. That's why I don't think we will improve size a lot.
>> If this is copied to other subsystems then yes it will be more useful.
>> If we have just one now we can't save a lot.
> 
> Well maybe, but of course the problem you are fixing applies to other
> subsystems too, doesn't it?

In Linux i2c came with this solution in connection to DT guys where
aliases are stable and even adding overlay can't update it/change it.

If you grep of_alias_get_highest_id you will find out that there is one
more which is spi and that's it.

This is commit message from Linux regarding spi.

commit 226584aedd94acd61ffa51fb69bcf6b3309a7b8f
Author:     Lucas Stach <l.stach at pengutronix.de>
AuthorDate: Mon Oct 16 12:27:58 2017 +0200
Commit:     Mark Brown <broonie at kernel.org>
CommitDate: Mon Oct 16 21:02:54 2017 +0100

    spi: fix IDR collision on systems with both fixed and dynamic SPI
bus numbers

    On systems where some controllers get a dynamic ID assigned and some
have
    a fixed number from DT, the current implemention might run into an IDR
    collision if the dynamic controllers gets probed first and get an
IDR number,
    which is later requested by the controller with the fixed numbering.
When
    this happens the fixed controller will fail to register with the SPI
core.

    Fix this by skipping all known alias numbers when assigning the
dynamic IDs.

    Fixes: 9b61e302210e (spi: Pick spi bus number from Linux idr or spi
alias)
    Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
    Signed-off-by: Mark Brown <broonie at kernel.org>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190208/f5b0d2ba/attachment.sig>