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