diff mbox series

[v2,3/5] cpufreq: sun50i: Add D1 support

Message ID 20231218110543.64044-4-fusibrandon13@gmail.com (mailing list archive)
State New, archived
Headers show
Series cpufreq support for the D1 | expand

Commit Message

Brandon Cheo Fusi Dec. 18, 2023, 11:05 a.m. UTC
Add support for D1 based devices to the Allwinner H6 cpufreq
driver

Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
---
 drivers/cpufreq/sun50i-cpufreq-nvmem.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Conor Dooley Dec. 18, 2023, 2:55 p.m. UTC | #1
On Mon, Dec 18, 2023 at 12:05:41PM +0100, Brandon Cheo Fusi wrote:
> Add support for D1 based devices to the Allwinner H6 cpufreq
> driver
> 
> Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> ---
>  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> index 32a9c88f8..ccf83780f 100644
> --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> @@ -160,6 +160,7 @@ static struct platform_driver sun50i_cpufreq_driver = {
>  
>  static const struct of_device_id sun50i_cpufreq_match_list[] = {
>  	{ .compatible = "allwinner,sun50i-h6" },
> +	{ .compatible = "allwinner,sun20i-d1" },

I thought the feedback in v2 was to drop this change, since the
devicetree has the sun50i-h6 as a fallback compatible?
Andre Przywara Dec. 18, 2023, 3:53 p.m. UTC | #2
On Mon, 18 Dec 2023 14:55:30 +0000
Conor Dooley <conor@kernel.org> wrote:

Hi,

> On Mon, Dec 18, 2023 at 12:05:41PM +0100, Brandon Cheo Fusi wrote:
> > Add support for D1 based devices to the Allwinner H6 cpufreq
> > driver
> > 
> > Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> > ---
> >  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > index 32a9c88f8..ccf83780f 100644
> > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > @@ -160,6 +160,7 @@ static struct platform_driver sun50i_cpufreq_driver = {
> >  
> >  static const struct of_device_id sun50i_cpufreq_match_list[] = {
> >  	{ .compatible = "allwinner,sun50i-h6" },
> > +	{ .compatible = "allwinner,sun20i-d1" },  
> 
> I thought the feedback in v2 was to drop this change, since the
> devicetree has the sun50i-h6 as a fallback compatible?

Well, this is the *board* (fallback) compatible string, so we cannot assign
it as we like. The whole (existing) scheme is admittedly somewhat weird,
because we not only match on a particular device compatible
(like allwinner,sun20i-d1-operating-points), but also need to blocklist and
re-match some parts against the *board compatible*, owing to the
cpufreq-dt driver. The board name is basically used as a placeholder to
find out the SoC, because there is (or was?) no other good way - the
CPU DT nodes don't work for this. Back when this was introduced, this was
the "least worst" solution.

I don't remember all the details, and didn't find time yet to look into
this in more detail, but fixing this is non-trivial. If this isn't 6.8
material, I might have a look at this later this week/month.

Cheers,
Andre
Jernej Škrabec Dec. 18, 2023, 4:15 p.m. UTC | #3
Dne ponedeljek, 18. december 2023 ob 16:53:45 CET je Andre Przywara napisal(a):
> On Mon, 18 Dec 2023 14:55:30 +0000
> Conor Dooley <conor@kernel.org> wrote:
> 
> Hi,
> 
> > On Mon, Dec 18, 2023 at 12:05:41PM +0100, Brandon Cheo Fusi wrote:
> > > Add support for D1 based devices to the Allwinner H6 cpufreq
> > > driver
> > > 
> > > Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> > > ---
> > >  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > index 32a9c88f8..ccf83780f 100644
> > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > @@ -160,6 +160,7 @@ static struct platform_driver sun50i_cpufreq_driver = {
> > >  
> > >  static const struct of_device_id sun50i_cpufreq_match_list[] = {
> > >  	{ .compatible = "allwinner,sun50i-h6" },
> > > +	{ .compatible = "allwinner,sun20i-d1" },  
> > 
> > I thought the feedback in v2 was to drop this change, since the
> > devicetree has the sun50i-h6 as a fallback compatible?
> 
> Well, this is the *board* (fallback) compatible string, so we cannot assign
> it as we like. The whole (existing) scheme is admittedly somewhat weird,
> because we not only match on a particular device compatible
> (like allwinner,sun20i-d1-operating-points), but also need to blocklist and
> re-match some parts against the *board compatible*, owing to the
> cpufreq-dt driver. The board name is basically used as a placeholder to
> find out the SoC, because there is (or was?) no other good way - the
> CPU DT nodes don't work for this. Back when this was introduced, this was
> the "least worst" solution.
> 
> I don't remember all the details, and didn't find time yet to look into
> this in more detail, but fixing this is non-trivial. If this isn't 6.8
> material, I might have a look at this later this week/month.

I would say it's 6.9 material. -rc6 already passed and it's not yet aligned
with all maintainers.

Best regards,
Jernej
Conor Dooley Dec. 18, 2023, 4:19 p.m. UTC | #4
On Mon, Dec 18, 2023 at 03:53:45PM +0000, Andre Przywara wrote:
> On Mon, 18 Dec 2023 14:55:30 +0000
> Conor Dooley <conor@kernel.org> wrote:
> 
> Hi,
> 
> > On Mon, Dec 18, 2023 at 12:05:41PM +0100, Brandon Cheo Fusi wrote:
> > > Add support for D1 based devices to the Allwinner H6 cpufreq
> > > driver
> > > 
> > > Signed-off-by: Brandon Cheo Fusi <fusibrandon13@gmail.com>
> > > ---
> > >  drivers/cpufreq/sun50i-cpufreq-nvmem.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > index 32a9c88f8..ccf83780f 100644
> > > --- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > +++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
> > > @@ -160,6 +160,7 @@ static struct platform_driver sun50i_cpufreq_driver = {
> > >  
> > >  static const struct of_device_id sun50i_cpufreq_match_list[] = {
> > >  	{ .compatible = "allwinner,sun50i-h6" },
> > > +	{ .compatible = "allwinner,sun20i-d1" },  
> > 
> > I thought the feedback in v2 was to drop this change, since the
> > devicetree has the sun50i-h6 as a fallback compatible?
> 
> Well, this is the *board* (fallback) compatible string, so we cannot assign

Oh of course... That's both me and Jernej that tripped up on this.
Brandon, please ignore the comment from me on this patch.

Thanks,
Conor.
diff mbox series

Patch

diff --git a/drivers/cpufreq/sun50i-cpufreq-nvmem.c b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
index 32a9c88f8..ccf83780f 100644
--- a/drivers/cpufreq/sun50i-cpufreq-nvmem.c
+++ b/drivers/cpufreq/sun50i-cpufreq-nvmem.c
@@ -160,6 +160,7 @@  static struct platform_driver sun50i_cpufreq_driver = {
 
 static const struct of_device_id sun50i_cpufreq_match_list[] = {
 	{ .compatible = "allwinner,sun50i-h6" },
+	{ .compatible = "allwinner,sun20i-d1" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sun50i_cpufreq_match_list);