Re: [PATCH 03/10] soc: xilinx: zynqmp: Add machine identification support

[Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>]

Am 17.06.2022 um 12:41 schrieb Michal Simek:
> On 6/8/22 18:20, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Add machine identification support based on the
>> zynqmp_get_silicon_idcode_name function in board/xilinx/zynqmp/zynqmp.c.
>>
>> Signed-off-by: Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier@weidmueller.com>
>> ---
>>
>>   drivers/soc/soc_xilinx_zynqmp.c | 289 +++++++++++++++++++++++++++++++-
>>   1 file changed, 286 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/soc_xilinx_zynqmp.c 
>> b/drivers/soc/soc_xilinx_zynqmp.c
>> index a71115b17c..45592ed534 100644
>> --- a/drivers/soc/soc_xilinx_zynqmp.c
>> +++ b/drivers/soc/soc_xilinx_zynqmp.c

>> @@ -54,8 +318,7 @@ static int soc_xilinx_zynqmp_probe(struct udevice 
>> *dev)
>>
>>          priv->family = zynqmp_family;
>>
>> -       if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3 ||
>> -           !IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
>> +       if (!IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE))
>>                  ret = zynqmp_mmio_read(ZYNQMP_PS_VERSION, 
>> &ret_payload[2]);
>>          else
>>                  ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,
> 
> I was looking at code and this change is very interesting.
> I think that it can be just
> ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0,...
> because message can be sent via IPI directly.

Without zynqmp_mmio_read this driver depends on the 
CONFIG_ZYNQMP_FIRMWARE driver.

> That means that this should be completely separate patch.

I will add a separate patch.

> 
> 
>> @@ -65,6 +328,26 @@ static int soc_xilinx_zynqmp_probe(struct udevice 
>> *dev)
>>
>>          priv->revision = ret_payload[2] & ZYNQMP_PS_VER_MASK;
>>
>> +       if (IS_ENABLED(CONFIG_ZYNQMP_FIRMWARE)) {
> 
> When above change is there you should be able to remove this checking 
> because
> you should get all payloads back in proper shape.
> 
> 
> 
>> +               /*
>> +                * Firmware returns:
>> +                * payload[0][31:0] = status of the operation
>> +                * payload[1] = IDCODE
>> +                * payload[2][19:0] = Version
>> +                * payload[2][28:20] = EXTENDED_IDCODE
>> +                * payload[2][29] = PL_INIT
>> +                */
>> +               u32 idcode = ret_payload[1];
>> +               u32 idcode2 = ret_payload[2] >>
>> +                                  ZYNQMP_CSU_VERSION_EMPTY_SHIFT;
>> +               dev_dbg(dev, "IDCODE: 0x%0x, IDCODE2: 0x%0x\n", idcode,
>> +                       idcode2);
>> +
>> +               ret = soc_xilinx_zynqmp_detect_machine(dev, idcode, 
>> idcode2);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>>          return 0;
>>   }
>>
>> -- 
>> 2.30.2
>>

Regards
   Stefan