| Message ID | 20231218110543.64044-4-fusibrandon13@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | cpufreq support for the D1 | expand |
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?
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
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
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 --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);
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(+)