diff mbox series

[2/2] reset: reset-zynqmp: Added support for Versal platform

Message ID 1594708149-29944-3-git-send-email-lakshmi.sai.krishna.potthuri@xilinx.com (mailing list archive)
State New, archived
Headers show
Series reset: reset-zynqmp: Added Versal platform support | expand

Commit Message

Sai Krishna Potthuri July 14, 2020, 6:29 a.m. UTC
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 <lakshmi.sai.krishna.potthuri@xilinx.com>
---
 drivers/reset/reset-zynqmp.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Philipp Zabel July 20, 2020, 9:48 a.m. UTC | #1
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 <lakshmi.sai.krishna.potthuri@xilinx.com>
> ---
>  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.

> +		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.

regards
Philipp
Sai Krishna Potthuri July 20, 2020, 1:58 p.m. UTC | #2
Hi Philipp,

Thanks for the review.

> -----Original Message-----
> From: Philipp Zabel <p.zabel@pengutronix.de>
> Sent: Monday, July 20, 2020 3:18 PM
> To: Sai Krishna Potthuri <lakshmis@xilinx.com>; Rob Herring
> <robh+dt@kernel.org>; Michal Simek <michals@xilinx.com>
> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git <git@xilinx.com>; saikrishna12468@gmail.com
> 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
> > <lakshmi.sai.krishna.potthuri@xilinx.com>
> > ---
> >  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
diff mbox series

Patch

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")) {
+		priv->reset_id = 0;
+		priv->rcdev.nr_resets = VERSAL_NR_RESETS;
+	}
 
 	return devm_reset_controller_register(&pdev->dev, &priv->rcdev);
 }
 
 static const struct of_device_id zynqmp_reset_dt_ids[] = {
 	{ .compatible = "xlnx,zynqmp-reset", },
+	{ .compatible = "xlnx,versal-reset", },
 	{ /* sentinel */ },
 };