Hi, > -----Original Message----- > From: Michal Simek <michal.si...@amd.com> > Sent: Tuesday, July 11, 2023 3:28 PM > To: Jaehoon Chung <jh80.ch...@samsung.com>; u-boot@lists.denx.de; > g...@xilinx.com > Cc: 'Ashok Reddy Soma' <ashok.reddy.s...@amd.com>; 'Peng Fan' > <peng....@nxp.com> > Subject: Re: [PATCH] mmc: zynq_sdhci: Dll reset only for ZynqMP platform > > Hi, > > On 7/11/23 07:00, Jaehoon Chung wrote: > > Hi, > > > >> -----Original Message----- > >> From: Michal Simek <michal.si...@amd.com> > >> Sent: Monday, July 10, 2023 9:12 PM > >> To: u-boot@lists.denx.de; g...@xilinx.com > >> Cc: Ashok Reddy Soma <ashok.reddy.s...@amd.com>; Jaehoon Chung > >> <jh80.ch...@samsung.com>; Peng Fan > >> <peng....@nxp.com> > >> Subject: [PATCH] mmc: zynq_sdhci: Dll reset only for ZynqMP platform > >> > >> From: Ashok Reddy Soma <ashok.reddy.s...@amd.com> > >> > >> Dll reset is needed only for ZynqMP platforms, add condition in tuning > >> to call arasan_zynqmp_dll_reset() just for ZynqMP platforms. > >> > >> On other platforms like Versal NET, If this condition is not added, we > >> see PLM error messages when dll reset smc is called. > >> > >> Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@amd.com> > >> Signed-off-by: Michal Simek <michal.si...@amd.com> > >> --- > >> > >> drivers/mmc/zynq_sdhci.c | 7 +++++-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c > >> index e779251ce34f..935540d17194 100644 > >> --- a/drivers/mmc/zynq_sdhci.c > >> +++ b/drivers/mmc/zynq_sdhci.c > >> @@ -422,7 +422,8 @@ static int arasan_sdhci_execute_tuning(struct mmc > >> *mmc, u8 opcode) > >> > >> mdelay(1); > >> > >> - arasan_zynqmp_dll_reset(host, priv->node_id); > >> + if (device_is_compatible(mmc->dev, "xlnx,zynqmp-8.9a")) > >> + arasan_zynqmp_dll_reset(host, priv->node_id); > > > > How about using local variable to check whether it needs to reset or not? > > It's not efficient to call device_is_compatible() everytime. > > (I'm not sure that it will be added more in future.) > > > > e.g) > > bool reset = device_is_compatible(mmc->dev, "xlx,zynmp-8.8a"); > > > > if (reset) > > arasan_zynqmp_dll_reset(host, priv->node_id); > > > > .. > > > > If (reset) > > arasan_zynqmp_dll_reset(host, priv->node_id); > > This is very valid request and TBH I have already added this to our TODO list > to > convert all device_is_compatible() to flags because over time the driver was > extended and this construct is used more than it should be. > > This is going to be the last device_is_compatible() patch. > Is it fine for you?
It's fine. Thanks! Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com> Best Regards, Jaehoon Chung > > Thanks, > Michal > >