On 3/19/24 12:25 AM, Krzysztof Kozlowski wrote: > On 19/03/2024 02:06, Tanmay Shah wrote: >> >> >> On 3/17/24 1:55 PM, Krzysztof Kozlowski wrote: >>> On 15/03/2024 22:15, Tanmay Shah wrote: >>>> AMD-Xilinx Versal and Versal-NET are successor of ZynqMP platform. ZynqMP >>>> remoteproc driver is mostly compatible with new platforms except few >>>> platform specific differences. >>>> >>>> Versal has same IP of cortex-R5 cores hence maintained compatible string >>>> same as ZynqMP platform. However, hardcode TCM addresses are not >>>> supported for new platforms and must be provided in device-tree as per >>>> new bindings. This makes TCM representation data-driven and easy to >>>> maintain. This check is provided in the driver. >>>> >>>> For Versal-NET platform, TCM doesn't need to be configured in lockstep >>>> mode or split mode. Hence that call to PMC firmware is avoided in the >>>> driver for Versal-NET platform. >>>> >>>> Signed-off-by: Tanmay Shah <tanmay.shah@xxxxxxx> >>>> --- >>>> drivers/remoteproc/xlnx_r5_remoteproc.c | 19 +++++++++++++++---- >>>> 1 file changed, 15 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c >>>> index d4a22caebaad..193bc159d1b4 100644 >>>> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c >>>> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c >>>> @@ -323,9 +323,12 @@ static int zynqmp_r5_set_mode(struct zynqmp_r5_core *r5_core, >>>> return ret; >>>> } >>>> >>>> - ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode); >>>> - if (ret < 0) >>>> - dev_err(r5_core->dev, "failed to configure TCM\n"); >>>> + /* TCM configuration is not needed in versal-net */ >>>> + if (device_is_compatible(r5_core->dev, "xlnx,zynqmp-r5f")) { >>>> + ret = zynqmp_pm_set_tcm_config(r5_core->pm_domain_id, tcm_mode); >>>> + if (ret < 0) >>>> + dev_err(r5_core->dev, "failed to configure TCM\n"); >>>> + } >>>> >>>> return ret; >>>> } >>>> @@ -933,10 +936,17 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster, >>>> int ret, i; >>>> >>>> r5_core = cluster->r5_cores[0]; >>>> + >>>> + /* >>>> + * New platforms must use device tree for TCM parsing. >>>> + * Only ZynqMP uses hardcode TCM. >>>> + */ >>>> if (of_find_property(r5_core->np, "reg", NULL)) >>>> ret = zynqmp_r5_get_tcm_node_from_dt(cluster); >>>> - else >>>> + else if (of_machine_is_compatible("xlnx,zynqmp")) >>>> ret = zynqmp_r5_get_tcm_node(cluster); >>> >>> That's poor code. Your drivers should not depend on platform. I don't >>> understand why you need to do this and how is even related to this patch. >> >> You are correct, ideally this shouldn't be needed. However, this driver contains >> hardcode TCM addresses that were used before TCM bindings were designed and available in >> device-tree. This check is provided to maintain backward compatibility with device-tree >> where TCM isn't expected. >> >> For new platforms (Versal and Versal-NET) TCM must be provided in device-tree and for >> ZynqMP if it's not in device-tree then to maintain backward compatibility hardcode >> addresses are used. > > That does not work like this. You cannot bind to some sort of different > compatible. If you disagree, please list the compatibles the driver > binds to. Okay understood. I believe then removing hardcode addresses makes more sense in that case. So, driver will become same for all the devices. > > Best regards, > Krzysztof >