[PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting

Icenowy Zheng posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting
Posted by Icenowy Zheng 1 month, 2 weeks ago
The reboot / power off operations require communication with the AON
firmware too.

As the driver is already present, create an auxiliary device with name
"reboot" to match that driver, and pass the AON channel by using
platform_data.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 drivers/pmdomain/thead/th1520-pm-domains.c | 35 ++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c
index 9040b698e7f7f..8285f552897b0 100644
--- a/drivers/pmdomain/thead/th1520-pm-domains.c
+++ b/drivers/pmdomain/thead/th1520-pm-domains.c
@@ -129,12 +129,39 @@ static void th1520_pd_init_all_off(struct generic_pm_domain **domains,
 	}
 }
 
-static void th1520_pd_pwrseq_unregister_adev(void *adev)
+static void th1520_pd_unregister_adev(void *adev)
 {
 	auxiliary_device_delete(adev);
 	auxiliary_device_uninit(adev);
 }
 
+static int th1520_pd_reboot_init(struct device *dev, struct th1520_aon_chan *aon_chan)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return -ENOMEM;
+
+	adev->name = "reboot";
+	adev->dev.parent = dev;
+	adev->dev.platform_data = aon_chan;
+
+	ret = auxiliary_device_init(adev);
+	if (ret)
+		return ret;
+
+	ret = auxiliary_device_add(adev);
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		return ret;
+	}
+
+	return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
+					adev);
+}
+
 static int th1520_pd_pwrseq_gpu_init(struct device *dev)
 {
 	struct auxiliary_device *adev;
@@ -169,7 +196,7 @@ static int th1520_pd_pwrseq_gpu_init(struct device *dev)
 		return ret;
 	}
 
-	return devm_add_action_or_reset(dev, th1520_pd_pwrseq_unregister_adev,
+	return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
 					adev);
 }
 
@@ -235,6 +262,10 @@ static int th1520_pd_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_clean_provider;
 
+	ret = th1520_pd_reboot_init(dev, aon_chan);
+	if (ret)
+		goto err_clean_provider;
+
 	return 0;
 
 err_clean_provider:
-- 
2.50.1
Re: [PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting
Posted by Ulf Hansson 1 month ago
On Mon, 18 Aug 2025 at 09:49, Icenowy Zheng <uwu@icenowy.me> wrote:
>
> The reboot / power off operations require communication with the AON
> firmware too.
>
> As the driver is already present, create an auxiliary device with name
> "reboot" to match that driver, and pass the AON channel by using
> platform_data.
>
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> ---
>  drivers/pmdomain/thead/th1520-pm-domains.c | 35 ++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c
> index 9040b698e7f7f..8285f552897b0 100644
> --- a/drivers/pmdomain/thead/th1520-pm-domains.c
> +++ b/drivers/pmdomain/thead/th1520-pm-domains.c
> @@ -129,12 +129,39 @@ static void th1520_pd_init_all_off(struct generic_pm_domain **domains,
>         }
>  }
>
> -static void th1520_pd_pwrseq_unregister_adev(void *adev)
> +static void th1520_pd_unregister_adev(void *adev)
>  {
>         auxiliary_device_delete(adev);
>         auxiliary_device_uninit(adev);
>  }
>
> +static int th1520_pd_reboot_init(struct device *dev, struct th1520_aon_chan *aon_chan)
> +{
> +       struct auxiliary_device *adev;
> +       int ret;
> +
> +       adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);
> +       if (!adev)
> +               return -ENOMEM;
> +
> +       adev->name = "reboot";
> +       adev->dev.parent = dev;
> +       adev->dev.platform_data = aon_chan;
> +
> +       ret = auxiliary_device_init(adev);
> +       if (ret)
> +               return ret;
> +
> +       ret = auxiliary_device_add(adev);
> +       if (ret) {
> +               auxiliary_device_uninit(adev);
> +               return ret;
> +       }
> +
> +       return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
> +                                       adev);

We have devm_auxiliary_device_create() now, I suggest we use that instead.

That said, I think it would make sense to convert the pwrseq-gpu
auxiliary device to be registered with devm_auxiliary_device_create()
too, but that's a separate change, of course.

> +}
> +
>  static int th1520_pd_pwrseq_gpu_init(struct device *dev)
>  {
>         struct auxiliary_device *adev;
> @@ -169,7 +196,7 @@ static int th1520_pd_pwrseq_gpu_init(struct device *dev)
>                 return ret;
>         }
>
> -       return devm_add_action_or_reset(dev, th1520_pd_pwrseq_unregister_adev,
> +       return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
>                                         adev);
>  }
>
> @@ -235,6 +262,10 @@ static int th1520_pd_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_clean_provider;
>
> +       ret = th1520_pd_reboot_init(dev, aon_chan);
> +       if (ret)
> +               goto err_clean_provider;
> +
>         return 0;
>
>  err_clean_provider:
> --
> 2.50.1
>

Otherwise this looks good to me!

Kind regards
Uffe
Re: [PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting
Posted by Icenowy Zheng 2 weeks, 1 day ago
在 2025-09-04星期四的 12:14 +0200,Ulf Hansson写道:
> On Mon, 18 Aug 2025 at 09:49, Icenowy Zheng <uwu@icenowy.me> wrote:
> > 
> > The reboot / power off operations require communication with the
> > AON
> > firmware too.
> > 
> > As the driver is already present, create an auxiliary device with
> > name
> > "reboot" to match that driver, and pass the AON channel by using
> > platform_data.
> > 
> > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > ---
> >  drivers/pmdomain/thead/th1520-pm-domains.c | 35
> > ++++++++++++++++++++--
> >  1 file changed, 33 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c
> > b/drivers/pmdomain/thead/th1520-pm-domains.c
> > index 9040b698e7f7f..8285f552897b0 100644
> > --- a/drivers/pmdomain/thead/th1520-pm-domains.c
> > +++ b/drivers/pmdomain/thead/th1520-pm-domains.c
> > @@ -129,12 +129,39 @@ static void th1520_pd_init_all_off(struct
> > generic_pm_domain **domains,
> >         }
> >  }
> > 
> > -static void th1520_pd_pwrseq_unregister_adev(void *adev)
> > +static void th1520_pd_unregister_adev(void *adev)
> >  {
> >         auxiliary_device_delete(adev);
> >         auxiliary_device_uninit(adev);
> >  }
> > 
> > +static int th1520_pd_reboot_init(struct device *dev, struct
> > th1520_aon_chan *aon_chan)
> > +{
> > +       struct auxiliary_device *adev;
> > +       int ret;
> > +
> > +       adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);
> > +       if (!adev)
> > +               return -ENOMEM;
> > +
> > +       adev->name = "reboot";
> > +       adev->dev.parent = dev;
> > +       adev->dev.platform_data = aon_chan;
> > +
> > +       ret = auxiliary_device_init(adev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = auxiliary_device_add(adev);
> > +       if (ret) {
> > +               auxiliary_device_uninit(adev);
> > +               return ret;
> > +       }
> > +
> > +       return devm_add_action_or_reset(dev,
> > th1520_pd_unregister_adev,
> > +                                       adev);
> 
> We have devm_auxiliary_device_create() now, I suggest we use that
> instead.

Should I send a v2 to convert to use this?

> 
> That said, I think it would make sense to convert the pwrseq-gpu
> auxiliary device to be registered with devm_auxiliary_device_create()
> too, but that's a separate change, of course.
> 
> > +}
> > +
> >  static int th1520_pd_pwrseq_gpu_init(struct device *dev)
> >  {
> >         struct auxiliary_device *adev;
> > @@ -169,7 +196,7 @@ static int th1520_pd_pwrseq_gpu_init(struct
> > device *dev)
> >                 return ret;
> >         }
> > 
> > -       return devm_add_action_or_reset(dev,
> > th1520_pd_pwrseq_unregister_adev,
> > +       return devm_add_action_or_reset(dev,
> > th1520_pd_unregister_adev,
> >                                         adev);
> >  }
> > 
> > @@ -235,6 +262,10 @@ static int th1520_pd_probe(struct
> > platform_device *pdev)
> >         if (ret)
> >                 goto err_clean_provider;
> > 
> > +       ret = th1520_pd_reboot_init(dev, aon_chan);
> > +       if (ret)
> > +               goto err_clean_provider;
> > +
> >         return 0;
> > 
> >  err_clean_provider:
> > --
> > 2.50.1
> > 
> 
> Otherwise this looks good to me!
> 
> Kind regards
> Uffe

Re: [PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting
Posted by Ulf Hansson 2 weeks, 1 day ago
On Thu, 18 Sept 2025 at 17:44, Icenowy Zheng <uwu@icenowy.me> wrote:
>
> 在 2025-09-04星期四的 12:14 +0200,Ulf Hansson写道:
> > On Mon, 18 Aug 2025 at 09:49, Icenowy Zheng <uwu@icenowy.me> wrote:
> > >
> > > The reboot / power off operations require communication with the
> > > AON
> > > firmware too.
> > >
> > > As the driver is already present, create an auxiliary device with
> > > name
> > > "reboot" to match that driver, and pass the AON channel by using
> > > platform_data.
> > >
> > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> > > ---
> > >  drivers/pmdomain/thead/th1520-pm-domains.c | 35
> > > ++++++++++++++++++++--
> > >  1 file changed, 33 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c
> > > b/drivers/pmdomain/thead/th1520-pm-domains.c
> > > index 9040b698e7f7f..8285f552897b0 100644
> > > --- a/drivers/pmdomain/thead/th1520-pm-domains.c
> > > +++ b/drivers/pmdomain/thead/th1520-pm-domains.c
> > > @@ -129,12 +129,39 @@ static void th1520_pd_init_all_off(struct
> > > generic_pm_domain **domains,
> > >         }
> > >  }
> > >
> > > -static void th1520_pd_pwrseq_unregister_adev(void *adev)
> > > +static void th1520_pd_unregister_adev(void *adev)
> > >  {
> > >         auxiliary_device_delete(adev);
> > >         auxiliary_device_uninit(adev);
> > >  }
> > >
> > > +static int th1520_pd_reboot_init(struct device *dev, struct
> > > th1520_aon_chan *aon_chan)
> > > +{
> > > +       struct auxiliary_device *adev;
> > > +       int ret;
> > > +
> > > +       adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);
> > > +       if (!adev)
> > > +               return -ENOMEM;
> > > +
> > > +       adev->name = "reboot";
> > > +       adev->dev.parent = dev;
> > > +       adev->dev.platform_data = aon_chan;
> > > +
> > > +       ret = auxiliary_device_init(adev);
> > > +       if (ret)
> > > +               return ret;
> > > +
> > > +       ret = auxiliary_device_add(adev);
> > > +       if (ret) {
> > > +               auxiliary_device_uninit(adev);
> > > +               return ret;
> > > +       }
> > > +
> > > +       return devm_add_action_or_reset(dev,
> > > th1520_pd_unregister_adev,
> > > +                                       adev);
> >
> > We have devm_auxiliary_device_create() now, I suggest we use that
> > instead.
>
> Should I send a v2 to convert to use this?

Please do, then I am ready to pick up the series.

[...]

Kind regards
Uffe
Re: [PATCH 2/2] pmdomain: thead: create auxiliary device for rebooting
Posted by Troy Mitchell 1 month, 2 weeks ago
On Mon, Aug 18, 2025 at 03:49:06PM +0800, Icenowy Zheng wrote:
> The reboot / power off operations require communication with the AON
> firmware too.
> 
> As the driver is already present, create an auxiliary device with name
> "reboot" to match that driver, and pass the AON channel by using
> platform_data.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>

Best regards,
Troy
> ---
>  drivers/pmdomain/thead/th1520-pm-domains.c | 35 ++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pmdomain/thead/th1520-pm-domains.c b/drivers/pmdomain/thead/th1520-pm-domains.c
> index 9040b698e7f7f..8285f552897b0 100644
> --- a/drivers/pmdomain/thead/th1520-pm-domains.c
> +++ b/drivers/pmdomain/thead/th1520-pm-domains.c
> @@ -129,12 +129,39 @@ static void th1520_pd_init_all_off(struct generic_pm_domain **domains,
>  	}
>  }
>  
> -static void th1520_pd_pwrseq_unregister_adev(void *adev)
> +static void th1520_pd_unregister_adev(void *adev)
>  {
>  	auxiliary_device_delete(adev);
>  	auxiliary_device_uninit(adev);
>  }
>  
> +static int th1520_pd_reboot_init(struct device *dev, struct th1520_aon_chan *aon_chan)
> +{
> +	struct auxiliary_device *adev;
> +	int ret;
> +
> +	adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);
> +	if (!adev)
> +		return -ENOMEM;
> +
> +	adev->name = "reboot";
> +	adev->dev.parent = dev;
> +	adev->dev.platform_data = aon_chan;
> +
> +	ret = auxiliary_device_init(adev);
> +	if (ret)
> +		return ret;
> +
> +	ret = auxiliary_device_add(adev);
> +	if (ret) {
> +		auxiliary_device_uninit(adev);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
> +					adev);
> +}
> +
>  static int th1520_pd_pwrseq_gpu_init(struct device *dev)
>  {
>  	struct auxiliary_device *adev;
> @@ -169,7 +196,7 @@ static int th1520_pd_pwrseq_gpu_init(struct device *dev)
>  		return ret;
>  	}
>  
> -	return devm_add_action_or_reset(dev, th1520_pd_pwrseq_unregister_adev,
> +	return devm_add_action_or_reset(dev, th1520_pd_unregister_adev,
>  					adev);
>  }
>  
> @@ -235,6 +262,10 @@ static int th1520_pd_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_clean_provider;
>  
> +	ret = th1520_pd_reboot_init(dev, aon_chan);
> +	if (ret)
> +		goto err_clean_provider;
> +
>  	return 0;
>  
>  err_clean_provider:
> -- 
> 2.50.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv