| Message ID | cover.1548948659.git.michal.simek@xilinx.com |
|---|---|
| Headers | show |
| Series | Align U-Boot I2C DM bus ID handling with Linux | expand |
Hi Michal, On Thu, 31 Jan 2019 at 08:31, Michal Simek <michal.simek@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@ff020000 > 20: gpio@20, offset len 1, flags 0 > 21: gpio@21, offset len 1, flags 0 > 75: i2c-mux@75, offset len 1, flags 0 > Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 > Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 > Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 > Bus 4: i2c@ff030000 (active 4) > 74: i2c-mux@74, offset len 1, flags 0 > 75: i2c-mux@75, offset len 1, flags 0 > Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) > 54: eeprom@54, offset len 1, flags 0 > Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 > Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 > Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 > Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 > Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 > Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 > Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 > Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 > Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 > Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 > Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 > Bus 17: i2c@ff030000->i2c-mux@75->i2c@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? This would be better for code size since it would help all subsystems. Regards, Simon
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@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@ff020000 >> 20: gpio@20, offset len 1, flags 0 >> 21: gpio@21, offset len 1, flags 0 >> 75: i2c-mux@75, offset len 1, flags 0 >> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >> Bus 4: i2c@ff030000 (active 4) >> 74: i2c-mux@74, offset len 1, flags 0 >> 75: i2c-mux@75, offset len 1, flags 0 >> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >> 54: eeprom@54, offset len 1, flags 0 >> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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. 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. Heiko: Can you please take a look at this series? I have another 7 patches on the top of this series which depends on it which cleanup all zynq/zynqmp platforms and I would like to close this to be able to move. Thanks, Michal
Hello Michal, Am 05.02.2019 um 08:57 schrieb Michal Simek: > 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@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@ff020000 >>> 20: gpio@20, offset len 1, flags 0 >>> 21: gpio@21, offset len 1, flags 0 >>> 75: i2c-mux@75, offset len 1, flags 0 >>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>> Bus 4: i2c@ff030000 (active 4) >>> 74: i2c-mux@74, offset len 1, flags 0 >>> 75: i2c-mux@75, offset len 1, flags 0 >>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>> 54: eeprom@54, offset len 1, flags 0 >>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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. For me it is okay to do this in a second step. @Simon: Can you give the patcheseries a formal Reviewed-by? >> 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. > 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. > > Heiko: Can you please take a look at this series? I have another 7 > patches on the top of this series which depends on it which cleanup all > zynq/zynqmp platforms and I would like to close this to be able to move. ASAP as I was ill the last 2 weeks ... sorry, give me some time. bye, Heiko
On 05. 02. 19 9:20, Heiko Schocher wrote: > Hello Michal, > > Am 05.02.2019 um 08:57 schrieb Michal Simek: >> 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@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@ff020000 >>>> 20: gpio@20, offset len 1, flags 0 >>>> 21: gpio@21, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>> Bus 4: i2c@ff030000 (active 4) >>>> 74: i2c-mux@74, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>> 54: eeprom@54, offset len 1, flags 0 >>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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. > > For me it is okay to do this in a second step. > > @Simon: Can you give the patcheseries a formal Reviewed-by? Thanks for this I wanted Simon to ask about it too. >>> 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. >> 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. >> >> Heiko: Can you please take a look at this series? I have another 7 >> patches on the top of this series which depends on it which cleanup all >> zynq/zynqmp platforms and I would like to close this to be able to move. > > ASAP as I was ill the last 2 weeks ... sorry, give me some time. Sure. Thanks, Michal
Hi Michal, On Tue, 5 Feb 2019 at 00:58, Michal Simek <michal.simek@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@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@ff020000 > >> 20: gpio@20, offset len 1, flags 0 > >> 21: gpio@21, offset len 1, flags 0 > >> 75: i2c-mux@75, offset len 1, flags 0 > >> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 > >> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 > >> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 > >> Bus 4: i2c@ff030000 (active 4) > >> 74: i2c-mux@74, offset len 1, flags 0 > >> 75: i2c-mux@75, offset len 1, flags 0 > >> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) > >> 54: eeprom@54, offset len 1, flags 0 > >> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 > >> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 > >> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 > >> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 > >> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 > >> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 > >> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 > >> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 > >> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 > >> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 > >> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 > >> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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? > > Heiko: Can you please take a look at this series? I have another 7 > patches on the top of this series which depends on it which cleanup all > zynq/zynqmp platforms and I would like to close this to be able to move. Regards, Simon
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@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@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@ff020000 >>>> 20: gpio@20, offset len 1, flags 0 >>>> 21: gpio@21, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>> Bus 4: i2c@ff030000 (active 4) >>>> 74: i2c-mux@74, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>> 54: eeprom@54, offset len 1, flags 0 >>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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@pengutronix.de> AuthorDate: Mon Oct 16 12:27:58 2017 +0200 Commit: Mark Brown <broonie@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@pengutronix.de> Signed-off-by: Mark Brown <broonie@kernel.org> Thanks, Michal
Hello Michael, Am 31.01.2019 um 16:30 schrieb Michal Simek: > 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@ff020000 > 20: gpio@20, offset len 1, flags 0 > 21: gpio@21, offset len 1, flags 0 > 75: i2c-mux@75, offset len 1, flags 0 > Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 > Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 > Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 > Bus 4: i2c@ff030000 (active 4) > 74: i2c-mux@74, offset len 1, flags 0 > 75: i2c-mux@75, offset len 1, flags 0 > Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) > 54: eeprom@54, offset len 1, flags 0 > Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 > Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 > Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 > Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 > Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 > Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 > Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 > Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 > Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 > Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 > Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 > Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 > > Michal Simek (7): > dm: core: Add of_alias_get_highest_id() > fdt: Introduce fdtdec_get_alias_highest_id() > dm: core: Introduce dev_read_alias_highest_id() > dm: core: Add tests for dev_read_alias_highest_id() > i2c: dm: Record maximum id of devices before probing devices > i2c: Fill req_seq in i2c_post_bind() > i2c: mux: Generate longer i2c mux name > > drivers/core/of_access.c | 18 ++++++++++++++ > drivers/core/read.c | 8 ++++++ > drivers/i2c/i2c-uclass.c | 50 +++++++++++++++++++++++++++++++++++--- > drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- > include/dm/of_access.h | 10 ++++++++ > include/dm/read.h | 16 ++++++++++++ > include/fdtdec.h | 13 ++++++++++ > lib/fdtdec.c | 33 +++++++++++++++++++++++++ > test/dm/test-fdt.c | 23 ++++++++++++++++++ > 9 files changed, 194 insertions(+), 6 deletions(-) > I just applied your patches and triggered a build on travis: It shows error for omap boards: https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 I try to find time to look into it, but may you have time too? Thanks! bye, Heiko
On 08. 02. 19 10:57, Heiko Schocher wrote: > Hello Michael, > > Am 31.01.2019 um 16:30 schrieb Michal Simek: >> 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@ff020000 >> 20: gpio@20, offset len 1, flags 0 >> 21: gpio@21, offset len 1, flags 0 >> 75: i2c-mux@75, offset len 1, flags 0 >> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >> Bus 4: i2c@ff030000 (active 4) >> 74: i2c-mux@74, offset len 1, flags 0 >> 75: i2c-mux@75, offset len 1, flags 0 >> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >> 54: eeprom@54, offset len 1, flags 0 >> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >> >> Michal Simek (7): >> dm: core: Add of_alias_get_highest_id() >> fdt: Introduce fdtdec_get_alias_highest_id() >> dm: core: Introduce dev_read_alias_highest_id() >> dm: core: Add tests for dev_read_alias_highest_id() >> i2c: dm: Record maximum id of devices before probing devices >> i2c: Fill req_seq in i2c_post_bind() >> i2c: mux: Generate longer i2c mux name >> >> drivers/core/of_access.c | 18 ++++++++++++++ >> drivers/core/read.c | 8 ++++++ >> drivers/i2c/i2c-uclass.c | 50 >> +++++++++++++++++++++++++++++++++++--- >> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >> include/dm/of_access.h | 10 ++++++++ >> include/dm/read.h | 16 ++++++++++++ >> include/fdtdec.h | 13 ++++++++++ >> lib/fdtdec.c | 33 +++++++++++++++++++++++++ >> test/dm/test-fdt.c | 23 ++++++++++++++++++ >> 9 files changed, 194 insertions(+), 6 deletions(-) >> > > I just applied your patches and triggered a build on travis: > > It shows error for omap boards: > > https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 > > I try to find time to look into it, but may you have time too? > This should be the fix. (I tried that on omap35_logic_somlv_defconfig) diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c index 6f3fca2d2326..391fb1289983 100644 --- a/drivers/i2c/i2c-uclass.c +++ b/drivers/i2c/i2c-uclass.c @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) if (!priv) return -ENOMEM; +#if CONFIG_IS_ENABLED(OF_CONTROL) /* Get the last allocated alias. */ priv->max_id = dev_read_alias_highest_id("i2c"); +#else + priv->max_id = -1; +#endif debug("%s: highest alias id is %d\n", __func__, priv->max_id); SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to call it. Thanks, Michal
On 08. 02. 19 12:14, Michal Simek wrote: > On 08. 02. 19 10:57, Heiko Schocher wrote: >> Hello Michael, >> >> Am 31.01.2019 um 16:30 schrieb Michal Simek: >>> 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@ff020000 >>> 20: gpio@20, offset len 1, flags 0 >>> 21: gpio@21, offset len 1, flags 0 >>> 75: i2c-mux@75, offset len 1, flags 0 >>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>> Bus 4: i2c@ff030000 (active 4) >>> 74: i2c-mux@74, offset len 1, flags 0 >>> 75: i2c-mux@75, offset len 1, flags 0 >>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>> 54: eeprom@54, offset len 1, flags 0 >>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >>> >>> Michal Simek (7): >>> dm: core: Add of_alias_get_highest_id() >>> fdt: Introduce fdtdec_get_alias_highest_id() >>> dm: core: Introduce dev_read_alias_highest_id() >>> dm: core: Add tests for dev_read_alias_highest_id() >>> i2c: dm: Record maximum id of devices before probing devices >>> i2c: Fill req_seq in i2c_post_bind() >>> i2c: mux: Generate longer i2c mux name >>> >>> drivers/core/of_access.c | 18 ++++++++++++++ >>> drivers/core/read.c | 8 ++++++ >>> drivers/i2c/i2c-uclass.c | 50 >>> +++++++++++++++++++++++++++++++++++--- >>> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >>> include/dm/of_access.h | 10 ++++++++ >>> include/dm/read.h | 16 ++++++++++++ >>> include/fdtdec.h | 13 ++++++++++ >>> lib/fdtdec.c | 33 +++++++++++++++++++++++++ >>> test/dm/test-fdt.c | 23 ++++++++++++++++++ >>> 9 files changed, 194 insertions(+), 6 deletions(-) >>> >> >> I just applied your patches and triggered a build on travis: >> >> It shows error for omap boards: >> >> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 >> >> I try to find time to look into it, but may you have time too? >> > > This should be the fix. (I tried that on omap35_logic_somlv_defconfig) > > diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c > index 6f3fca2d2326..391fb1289983 100644 > --- a/drivers/i2c/i2c-uclass.c > +++ b/drivers/i2c/i2c-uclass.c > @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) > if (!priv) > return -ENOMEM; > > +#if CONFIG_IS_ENABLED(OF_CONTROL) > /* Get the last allocated alias. */ > priv->max_id = dev_read_alias_highest_id("i2c"); > +#else > + priv->max_id = -1; > +#endif > > debug("%s: highest alias id is %d\n", __func__, priv->max_id); > > > SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to call > it. :-) that sentence doesn't make sense. Correction: ...that's why it shouldn't be called. Maybe it should be enough to have dependency on OF_LIBFDT but Kconfig is saying that OF_LIBFDT is bool setup to y when OF_CONTROL is enabled. And in Makefile fdtdec is enabled obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += fdtdec.o or just obj-$(CONFIG_OF_LIBFDT) += fdtdec.o Anyway it should be likely enough to have dependency just on OF_LIBFDT. Thanks, Michal
Hello Michal, Am 08.02.2019 um 12:24 schrieb Michal Simek: > On 08. 02. 19 12:14, Michal Simek wrote: >> On 08. 02. 19 10:57, Heiko Schocher wrote: >>> Hello Michael, >>> >>> Am 31.01.2019 um 16:30 schrieb Michal Simek: >>>> 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@ff020000 >>>> 20: gpio@20, offset len 1, flags 0 >>>> 21: gpio@21, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>> Bus 4: i2c@ff030000 (active 4) >>>> 74: i2c-mux@74, offset len 1, flags 0 >>>> 75: i2c-mux@75, offset len 1, flags 0 >>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>> 54: eeprom@54, offset len 1, flags 0 >>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >>>> >>>> Michal Simek (7): >>>> dm: core: Add of_alias_get_highest_id() >>>> fdt: Introduce fdtdec_get_alias_highest_id() >>>> dm: core: Introduce dev_read_alias_highest_id() >>>> dm: core: Add tests for dev_read_alias_highest_id() >>>> i2c: dm: Record maximum id of devices before probing devices >>>> i2c: Fill req_seq in i2c_post_bind() >>>> i2c: mux: Generate longer i2c mux name >>>> >>>> drivers/core/of_access.c | 18 ++++++++++++++ >>>> drivers/core/read.c | 8 ++++++ >>>> drivers/i2c/i2c-uclass.c | 50 >>>> +++++++++++++++++++++++++++++++++++--- >>>> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >>>> include/dm/of_access.h | 10 ++++++++ >>>> include/dm/read.h | 16 ++++++++++++ >>>> include/fdtdec.h | 13 ++++++++++ >>>> lib/fdtdec.c | 33 +++++++++++++++++++++++++ >>>> test/dm/test-fdt.c | 23 ++++++++++++++++++ >>>> 9 files changed, 194 insertions(+), 6 deletions(-) >>>> >>> >>> I just applied your patches and triggered a build on travis: >>> >>> It shows error for omap boards: >>> >>> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 >>> >>> I try to find time to look into it, but may you have time too? >>> >> >> This should be the fix. (I tried that on omap35_logic_somlv_defconfig) >> >> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >> index 6f3fca2d2326..391fb1289983 100644 >> --- a/drivers/i2c/i2c-uclass.c >> +++ b/drivers/i2c/i2c-uclass.c >> @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) >> if (!priv) >> return -ENOMEM; >> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) >> /* Get the last allocated alias. */ >> priv->max_id = dev_read_alias_highest_id("i2c"); >> +#else >> + priv->max_id = -1; >> +#endif >> >> debug("%s: highest alias id is %d\n", __func__, priv->max_id); >> >> >> SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to call >> it. > > :-) that sentence doesn't make sense. Correction: > ...that's why it shouldn't be called. ;-) > Maybe it should be enough to have dependency on OF_LIBFDT but Kconfig is > saying that OF_LIBFDT is bool setup to y when OF_CONTROL is enabled. > And in Makefile fdtdec is enabled > > obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += fdtdec.o > or just > obj-$(CONFIG_OF_LIBFDT) += fdtdec.o > > Anyway it should be likely enough to have dependency just on OF_LIBFDT. Ok, do you want to send a v3 of the patch: dm: core: Add of_alias_get_highest_id() or is it OK for you, if I add the above fix to this patch directly? bye, Heiko
On 11. 02. 19 7:20, Heiko Schocher wrote: > Hello Michal, > > Am 08.02.2019 um 12:24 schrieb Michal Simek: >> On 08. 02. 19 12:14, Michal Simek wrote: >>> On 08. 02. 19 10:57, Heiko Schocher wrote: >>>> Hello Michael, >>>> >>>> Am 31.01.2019 um 16:30 schrieb Michal Simek: >>>>> 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@ff020000 >>>>> 20: gpio@20, offset len 1, flags 0 >>>>> 21: gpio@21, offset len 1, flags 0 >>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>>> Bus 4: i2c@ff030000 (active 4) >>>>> 74: i2c-mux@74, offset len 1, flags 0 >>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>>> 54: eeprom@54, offset len 1, flags 0 >>>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >>>>> >>>>> Michal Simek (7): >>>>> dm: core: Add of_alias_get_highest_id() >>>>> fdt: Introduce fdtdec_get_alias_highest_id() >>>>> dm: core: Introduce dev_read_alias_highest_id() >>>>> dm: core: Add tests for dev_read_alias_highest_id() >>>>> i2c: dm: Record maximum id of devices before probing devices >>>>> i2c: Fill req_seq in i2c_post_bind() >>>>> i2c: mux: Generate longer i2c mux name >>>>> >>>>> drivers/core/of_access.c | 18 ++++++++++++++ >>>>> drivers/core/read.c | 8 ++++++ >>>>> drivers/i2c/i2c-uclass.c | 50 >>>>> +++++++++++++++++++++++++++++++++++--- >>>>> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >>>>> include/dm/of_access.h | 10 ++++++++ >>>>> include/dm/read.h | 16 ++++++++++++ >>>>> include/fdtdec.h | 13 ++++++++++ >>>>> lib/fdtdec.c | 33 +++++++++++++++++++++++++ >>>>> test/dm/test-fdt.c | 23 ++++++++++++++++++ >>>>> 9 files changed, 194 insertions(+), 6 deletions(-) >>>>> >>>> >>>> I just applied your patches and triggered a build on travis: >>>> >>>> It shows error for omap boards: >>>> >>>> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 >>>> >>>> I try to find time to look into it, but may you have time too? >>>> >>> >>> This should be the fix. (I tried that on omap35_logic_somlv_defconfig) >>> >>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>> index 6f3fca2d2326..391fb1289983 100644 >>> --- a/drivers/i2c/i2c-uclass.c >>> +++ b/drivers/i2c/i2c-uclass.c >>> @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) >>> if (!priv) >>> return -ENOMEM; >>> >>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>> /* Get the last allocated alias. */ >>> priv->max_id = dev_read_alias_highest_id("i2c"); >>> +#else >>> + priv->max_id = -1; >>> +#endif >>> >>> debug("%s: highest alias id is %d\n", __func__, priv->max_id); >>> >>> >>> SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to call >>> it. >> >> :-) that sentence doesn't make sense. Correction: >> ...that's why it shouldn't be called. > > ;-) > >> Maybe it should be enough to have dependency on OF_LIBFDT but Kconfig is >> saying that OF_LIBFDT is bool setup to y when OF_CONTROL is enabled. >> And in Makefile fdtdec is enabled >> >> obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += fdtdec.o >> or just >> obj-$(CONFIG_OF_LIBFDT) += fdtdec.o >> >> Anyway it should be likely enough to have dependency just on OF_LIBFDT. > > Ok, do you want to send a v3 of the patch: > > dm: core: Add of_alias_get_highest_id() > > or is it OK for you, if I add the above fix to this patch directly? Please take it directly. M
Hello Michal, Am 11.02.2019 um 07:58 schrieb Michal Simek: > On 11. 02. 19 7:20, Heiko Schocher wrote: >> Hello Michal, >> >> Am 08.02.2019 um 12:24 schrieb Michal Simek: >>> On 08. 02. 19 12:14, Michal Simek wrote: >>>> On 08. 02. 19 10:57, Heiko Schocher wrote: >>>>> Hello Michael, >>>>> >>>>> Am 31.01.2019 um 16:30 schrieb Michal Simek: >>>>>> 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@ff020000 >>>>>> 20: gpio@20, offset len 1, flags 0 >>>>>> 21: gpio@21, offset len 1, flags 0 >>>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>>>> Bus 4: i2c@ff030000 (active 4) >>>>>> 74: i2c-mux@74, offset len 1, flags 0 >>>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>>>> 54: eeprom@54, offset len 1, flags 0 >>>>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >>>>>> >>>>>> Michal Simek (7): >>>>>> dm: core: Add of_alias_get_highest_id() >>>>>> fdt: Introduce fdtdec_get_alias_highest_id() >>>>>> dm: core: Introduce dev_read_alias_highest_id() >>>>>> dm: core: Add tests for dev_read_alias_highest_id() >>>>>> i2c: dm: Record maximum id of devices before probing devices >>>>>> i2c: Fill req_seq in i2c_post_bind() >>>>>> i2c: mux: Generate longer i2c mux name >>>>>> >>>>>> drivers/core/of_access.c | 18 ++++++++++++++ >>>>>> drivers/core/read.c | 8 ++++++ >>>>>> drivers/i2c/i2c-uclass.c | 50 >>>>>> +++++++++++++++++++++++++++++++++++--- >>>>>> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >>>>>> include/dm/of_access.h | 10 ++++++++ >>>>>> include/dm/read.h | 16 ++++++++++++ >>>>>> include/fdtdec.h | 13 ++++++++++ >>>>>> lib/fdtdec.c | 33 +++++++++++++++++++++++++ >>>>>> test/dm/test-fdt.c | 23 ++++++++++++++++++ >>>>>> 9 files changed, 194 insertions(+), 6 deletions(-) >>>>>> >>>>> >>>>> I just applied your patches and triggered a build on travis: >>>>> >>>>> It shows error for omap boards: >>>>> >>>>> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 >>>>> >>>>> I try to find time to look into it, but may you have time too? >>>>> >>>> >>>> This should be the fix. (I tried that on omap35_logic_somlv_defconfig) >>>> >>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>>> index 6f3fca2d2326..391fb1289983 100644 >>>> --- a/drivers/i2c/i2c-uclass.c >>>> +++ b/drivers/i2c/i2c-uclass.c >>>> @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) >>>> if (!priv) >>>> return -ENOMEM; >>>> >>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>> /* Get the last allocated alias. */ >>>> priv->max_id = dev_read_alias_highest_id("i2c"); >>>> +#else >>>> + priv->max_id = -1; >>>> +#endif >>>> >>>> debug("%s: highest alias id is %d\n", __func__, priv->max_id); >>>> >>>> >>>> SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to call >>>> it. >>> >>> :-) that sentence doesn't make sense. Correction: >>> ...that's why it shouldn't be called. >> >> ;-) >> >>> Maybe it should be enough to have dependency on OF_LIBFDT but Kconfig is >>> saying that OF_LIBFDT is bool setup to y when OF_CONTROL is enabled. >>> And in Makefile fdtdec is enabled >>> >>> obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += fdtdec.o >>> or just >>> obj-$(CONFIG_OF_LIBFDT) += fdtdec.o >>> >>> Anyway it should be likely enough to have dependency just on OF_LIBFDT. >> >> Ok, do you want to send a v3 of the patch: >> >> dm: core: Add of_alias_get_highest_id() >> >> or is it OK for you, if I add the above fix to this patch directly? > > Please take it directly. Ok, done. If all builds fine now, I make a pull request for Tom, thanks! bye, Heiko
On 11. 02. 19 9:45, Heiko Schocher wrote: > Hello Michal, > > Am 11.02.2019 um 07:58 schrieb Michal Simek: >> On 11. 02. 19 7:20, Heiko Schocher wrote: >>> Hello Michal, >>> >>> Am 08.02.2019 um 12:24 schrieb Michal Simek: >>>> On 08. 02. 19 12:14, Michal Simek wrote: >>>>> On 08. 02. 19 10:57, Heiko Schocher wrote: >>>>>> Hello Michael, >>>>>> >>>>>> Am 31.01.2019 um 16:30 schrieb Michal Simek: >>>>>>> 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@ff020000 >>>>>>> 20: gpio@20, offset len 1, flags 0 >>>>>>> 21: gpio@21, offset len 1, flags 0 >>>>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>>>> Bus 1: i2c@ff020000->i2c-mux@75->i2c@0 >>>>>>> Bus 2: i2c@ff020000->i2c-mux@75->i2c@1 >>>>>>> Bus 3: i2c@ff020000->i2c-mux@75->i2c@2 >>>>>>> Bus 4: i2c@ff030000 (active 4) >>>>>>> 74: i2c-mux@74, offset len 1, flags 0 >>>>>>> 75: i2c-mux@75, offset len 1, flags 0 >>>>>>> Bus 5: i2c@ff030000->i2c-mux@74->i2c@0 (active 5) >>>>>>> 54: eeprom@54, offset len 1, flags 0 >>>>>>> Bus 6: i2c@ff030000->i2c-mux@74->i2c@1 >>>>>>> Bus 7: i2c@ff030000->i2c-mux@74->i2c@2 >>>>>>> Bus 8: i2c@ff030000->i2c-mux@74->i2c@3 >>>>>>> Bus 9: i2c@ff030000->i2c-mux@74->i2c@4 >>>>>>> Bus 10: i2c@ff030000->i2c-mux@75->i2c@0 >>>>>>> Bus 11: i2c@ff030000->i2c-mux@75->i2c@1 >>>>>>> Bus 12: i2c@ff030000->i2c-mux@75->i2c@2 >>>>>>> Bus 13: i2c@ff030000->i2c-mux@75->i2c@3 >>>>>>> Bus 14: i2c@ff030000->i2c-mux@75->i2c@4 >>>>>>> Bus 15: i2c@ff030000->i2c-mux@75->i2c@5 >>>>>>> Bus 16: i2c@ff030000->i2c-mux@75->i2c@6 >>>>>>> Bus 17: i2c@ff030000->i2c-mux@75->i2c@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 >>>>>>> >>>>>>> Michal Simek (7): >>>>>>> dm: core: Add of_alias_get_highest_id() >>>>>>> fdt: Introduce fdtdec_get_alias_highest_id() >>>>>>> dm: core: Introduce dev_read_alias_highest_id() >>>>>>> dm: core: Add tests for dev_read_alias_highest_id() >>>>>>> i2c: dm: Record maximum id of devices before probing devices >>>>>>> i2c: Fill req_seq in i2c_post_bind() >>>>>>> i2c: mux: Generate longer i2c mux name >>>>>>> >>>>>>> drivers/core/of_access.c | 18 ++++++++++++++ >>>>>>> drivers/core/read.c | 8 ++++++ >>>>>>> drivers/i2c/i2c-uclass.c | 50 >>>>>>> +++++++++++++++++++++++++++++++++++--- >>>>>>> drivers/i2c/muxes/i2c-mux-uclass.c | 29 +++++++++++++++++++--- >>>>>>> include/dm/of_access.h | 10 ++++++++ >>>>>>> include/dm/read.h | 16 ++++++++++++ >>>>>>> include/fdtdec.h | 13 ++++++++++ >>>>>>> lib/fdtdec.c | 33 >>>>>>> +++++++++++++++++++++++++ >>>>>>> test/dm/test-fdt.c | 23 ++++++++++++++++++ >>>>>>> 9 files changed, 194 insertions(+), 6 deletions(-) >>>>>>> >>>>>> >>>>>> I just applied your patches and triggered a build on travis: >>>>>> >>>>>> It shows error for omap boards: >>>>>> >>>>>> https://travis-ci.org/hsdenx/u-boot-i2c/jobs/490393822 >>>>>> >>>>>> I try to find time to look into it, but may you have time too? >>>>>> >>>>> >>>>> This should be the fix. (I tried that on >>>>> omap35_logic_somlv_defconfig) >>>>> >>>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c >>>>> index 6f3fca2d2326..391fb1289983 100644 >>>>> --- a/drivers/i2c/i2c-uclass.c >>>>> +++ b/drivers/i2c/i2c-uclass.c >>>>> @@ -655,8 +655,12 @@ int i2c_uclass_init(struct uclass *class) >>>>> if (!priv) >>>>> return -ENOMEM; >>>>> >>>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>>> /* Get the last allocated alias. */ >>>>> priv->max_id = dev_read_alias_highest_id("i2c"); >>>>> +#else >>>>> + priv->max_id = -1; >>>>> +#endif >>>>> >>>>> debug("%s: highest alias id is %d\n", __func__, >>>>> priv->max_id); >>>>> >>>>> >>>>> SPL has no OF_CONTROL and also no LIBFDT that's why it is trying to >>>>> call >>>>> it. >>>> >>>> :-) that sentence doesn't make sense. Correction: >>>> ...that's why it shouldn't be called. >>> >>> ;-) >>> >>>> Maybe it should be enough to have dependency on OF_LIBFDT but >>>> Kconfig is >>>> saying that OF_LIBFDT is bool setup to y when OF_CONTROL is enabled. >>>> And in Makefile fdtdec is enabled >>>> >>>> obj-$(CONFIG_$(SPL_TPL_)OF_CONTROL) += fdtdec.o >>>> or just >>>> obj-$(CONFIG_OF_LIBFDT) += fdtdec.o >>>> >>>> Anyway it should be likely enough to have dependency just on OF_LIBFDT. >>> >>> Ok, do you want to send a v3 of the patch: >>> >>> dm: core: Add of_alias_get_highest_id() >>> >>> or is it OK for you, if I add the above fix to this patch directly? >> >> Please take it directly. > > Ok, done. If all builds fine now, I make a pull request for Tom, > thanks! great. Thanks. Michal