Hi Philipp, Thanks for the review.
> -----Original Message----- > From: Philipp Zabel <[email protected]> > Sent: Monday, July 20, 2020 3:18 PM > To: Sai Krishna Potthuri <[email protected]>; Rob Herring > <[email protected]>; Michal Simek <[email protected]> > Cc: [email protected]; [email protected]; linux- > [email protected]; git <[email protected]>; [email protected] > Subject: Re: [PATCH 2/2] reset: reset-zynqmp: Added support for Versal > platform > > On Tue, 2020-07-14 at 11:59 +0530, Sai Krishna Potthuri wrote: > > Updated the reset driver to support Versal platform. > > As part of adding Versal support > > - Added Versal specific compatible string. > > - Reset Id and number of resets are different for Versal and ZynqMP, > > hence taken care of these two based on compatible string. > > > > Signed-off-by: Sai Krishna Potthuri > > <[email protected]> > > --- > > drivers/reset/reset-zynqmp.c | 24 ++++++++++++++++++++---- > > 1 file changed, 20 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/reset/reset-zynqmp.c > > b/drivers/reset/reset-zynqmp.c index 373ea8d4f7a1..17aa4532ec5e > 100644 > > --- a/drivers/reset/reset-zynqmp.c > > +++ b/drivers/reset/reset-zynqmp.c > > @@ -12,9 +12,11 @@ > > > > #define ZYNQMP_NR_RESETS (ZYNQMP_PM_RESET_END - > > ZYNQMP_PM_RESET_START) #define ZYNQMP_RESET_ID > ZYNQMP_PM_RESET_START > > +#define VERSAL_NR_RESETS 95 > > > > struct zynqmp_reset_data { > > struct reset_controller_dev rcdev; > > + u32 reset_id; > > }; > > > > static inline struct zynqmp_reset_data * @@ -26,23 +28,28 @@ > > to_zynqmp_reset_data(struct reset_controller_dev *rcdev) static int > > zynqmp_reset_assert(struct reset_controller_dev *rcdev, > > unsigned long id) > > { > > - return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id, > > + struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev); > > + > > + return zynqmp_pm_reset_assert(priv->reset_id + id, > > PM_RESET_ACTION_ASSERT); > > } > > > > static int zynqmp_reset_deassert(struct reset_controller_dev *rcdev, > > unsigned long id) > > { > > - return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id, > > + struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev); > > + > > + return zynqmp_pm_reset_assert(priv->reset_id + id, > > PM_RESET_ACTION_RELEASE); > > } > > > > static int zynqmp_reset_status(struct reset_controller_dev *rcdev, > > unsigned long id) > > { > > + struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev); > > int val, err; > > > > - err = zynqmp_pm_reset_get_status(ZYNQMP_RESET_ID + id, &val); > > + err = zynqmp_pm_reset_get_status(priv->reset_id + id, &val); > > if (err) > > return err; > > > > @@ -52,7 +59,9 @@ static int zynqmp_reset_status(struct > > reset_controller_dev *rcdev, static int zynqmp_reset_reset(struct > reset_controller_dev *rcdev, > > unsigned long id) > > { > > - return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id, > > + struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev); > > + > > + return zynqmp_pm_reset_assert(priv->reset_id + id, > > PM_RESET_ACTION_PULSE); > > } > > > > @@ -76,13 +85,20 @@ static int zynqmp_reset_probe(struct > platform_device *pdev) > > priv->rcdev.ops = &zynqmp_reset_ops; > > priv->rcdev.owner = THIS_MODULE; > > priv->rcdev.of_node = pdev->dev.of_node; > > + priv->reset_id = ZYNQMP_RESET_ID; > > priv->rcdev.nr_resets = ZYNQMP_NR_RESETS; > > + if (of_device_is_compatible(pdev->dev.of_node, > > + "xlnx,versal-reset")) { > > It would be better to use of_match_device and static const initalization data > for this. Will create soc specific initialization data structure and assign based on of_device_get_match_data(). > > > + priv->reset_id = 0; > > + priv->rcdev.nr_resets = VERSAL_NR_RESETS; > > This won't work. All your reset ids are greater than 95, and this driver is > using > the default of_xlate callback, so of_reset_simple_xlate will fail all reset > control requests with -EINVAL. Will create of_xlate callback that will simply return the reset line number without any checks. We have underlying secure library which will validate the reset line number. Regards Sai Krishna > > regards > Philipp
