On Thursday 21 April 2022 11:40:47 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 <p...@kernel.org> > > >>>>> --- > > >>>>> 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.
Now I sent patch to Linux: https://lore.kernel.org/linux-crypto/20220421134457.5867-1-p...@kernel.org/T/#u > > And not forget, please add a documentation for the compatible string > > in u-boot:/doc/device-tree-bindings/ > > Currently I do not see any information about atsha in > u-boot/doc/device-tree-bindings. > > > Thanks! > > > > bye, > > Heiko > > > > > >> Thanks, > > >> Stefan > > >> > > >>>> A quick grep > > >>>> doesn't show this in any of the dts files, not in U-Boot and not in the > > >>>> Kernel. > > >>> > > >>> Not yet. I'm preparing patches for a board which has atsha204 and will > > >>> use this u-boot driver. > > >>> > > >>>> Just checking... > > >>>> > > >>>> Thanks, > > >>>> Stefan > > >> > > > > -- > > 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: h...@denx.de