[PATCH] misc: atsha204a: Add support for atsha204 chip

[Heiko Schocher]

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 <pali at 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.
>>
>> See it, thanks!
> 
> Reviewed-by: Heiko Schocher <hs at denx.de>
> 
> 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: hs at denx.de