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

[Heiko Schocher]

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.

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