Hello Pali. On 10.05.22 06:45, Heiko Schocher wrote: > Hello Pali, > > On 22.04.22 05:59, Heiko Schocher wrote: >> Hello Pali, >> >> On 21.04.22 11:40, Pali Rohár wrote: >>> On Thursday 21 April 2022 06:11:11 Heiko Schocher wrote: >>>> Hello Pali, >>>> >>>> On 05.04.22 16:10, Pali Rohár wrote: >>>>> On Tuesday 05 April 2022 15:52:17 Stefan Roese wrote: >>>>>> On 4/5/22 15:28, Pali Rohár wrote: >>>>>>> On Tuesday 05 April 2022 15:14:52 Stefan Roese wrote: >>>>>>>> On 4/5/22 14:49, Pali Rohár wrote: >>>>>>>>> atsha204 chip is predecessor of atsha204a chip. Current U-Boot driver >>>>>>>>> atsha204a-i2c.c can use both atsha204 and atsha204a chips because it >>>>>>>>> does >>>>>>>>> not call specific functions to just one of these chips. >>>>>>>>> >>>>>>>>> So just add compatible string for atsha204. >>>>>>>>> >>>>>>>>> Signed-off-by: Pali Rohár <[email protected]> >>>>>>>>> --- >>>>>>>>> drivers/misc/atsha204a-i2c.c | 1 + >>>>>>>>> 1 file changed, 1 insertion(+) >>>>>>>>> >>>>>>>>> diff --git a/drivers/misc/atsha204a-i2c.c >>>>>>>>> b/drivers/misc/atsha204a-i2c.c >>>>>>>>> index 63fe541dade3..8b0055f99893 100644 >>>>>>>>> --- a/drivers/misc/atsha204a-i2c.c >>>>>>>>> +++ b/drivers/misc/atsha204a-i2c.c >>>>>>>>> @@ -399,6 +399,7 @@ static int atsha204a_of_to_plat(struct udevice >>>>>>>>> *dev) >>>>>>>>> } >>>>>>>>> static const struct udevice_id atsha204a_ids[] = { >>>>>>>>> + { .compatible = "atmel,atsha204" }, >>>>>>>>> { .compatible = "atmel,atsha204a" }, >>>>>>>>> { } >>>>>>>>> }; >>>>>>>> >>>>>>>> Why do we need this new compatible here in the driver? >>>>>>> >>>>>>> They are different chips, >>>>>> >>>>>> Sure... >>>>>> >>>>>>> so should have different compatible strings. >>>>>> >>>>>> ... but is this really necessary and "best practice"? If the driver >>>>>> can handle both chips without any changes, why do you need the new >>>>>> compatible here? >>>>> >>>>> Well, currently it can handle both of them. >>>>> >>>>> But if driver is going to be extended to support e.g. SHA command >>>>> (Calculate a SHA256 digest) then this command should be issued only for >>>>> atsha204a. atsha204 does not support it. >>>>> >>>>> Similarly, if other DTS-based system is going to implement that SHA >>>>> command, it would mean that U-Boot DTS file would not be compatible with >>>>> that other system. >>>>> >>>>> Also it is a good idea to have DTS files and its compatible strings >>>>> universal and not u-boot specific. So it could be used also by other >>>>> projects (e.g. linux kernel). >>>>> >>>>> And if we mix now two chips which are similar (and supports lot of >>>>> common operations) we would not be able in future to extend drivers in >>>>> backward compatible manner. >>>>> >>>>> Just to note, I'm not going to implement atsha204a specific commands >>>>> (which are not available in atsha204; like SHA command) because I do not >>>>> need them (right now). >>>>> >>>>>> Don't get me wrong. I'm not blocking this change, just want to be sure >>>>>> that it's really necessary. >>>>> >>>>> In case U-Boot driver has compatible string something like >>>>> "atsha204-common" which could say that driver is using only functions >>>>> which are available in all chip family then there would not be need for >>>>> it. But if driver has chip specific name, I think the best is not to >>>>> mask one chip by another which does not have 1:1 SW API compatibility. >>>> >>>> From my side this is full okay to add here a new compatibility string >>>> to differ between the two chips, and to see in DTS immediately which >>>> chip is on the board. Also later if the driver really supports features >>>> the other chip does not have, you do not need to change DTS anymore. >>>> >>>> I would love to see this patch first in linux. Do you plan to sent >>>> similiar change to linux? >>> >>> Hello! We are not using Linux kernel driver for atsha cryptochips (I was >>> told that it decrease lifetime) but I can send also similar change to >>> Linux. >> >> See it, thanks! > > Reviewed-by: Heiko Schocher <[email protected]> > > Will apply soon, as it is accepted in linux.
applied to u-boot-i2c master Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: [email protected]
