| Message ID | 20240902093222.2828345-3-d-gole@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dts: ti: k3-am62: Use opp_efuse_table for opp-table | expand |
On September 2, 2024 thus sayeth Dhruva Gole: > Since the efuse_offset is basically derived from the syscon node, we no > longer need to use any efuse_offset for AM625. This is in line with how > the AM62Ax and AM62Px are already doing. > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > --- > drivers/cpufreq/ti-cpufreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c > index ba621ce1cdda..98e320832f78 100644 > --- a/drivers/cpufreq/ti-cpufreq.c > +++ b/drivers/cpufreq/ti-cpufreq.c > @@ -313,7 +313,7 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = { > > static struct ti_cpufreq_soc_data am625_soc_data = { > .efuse_xlate = am625_efuse_xlate, > - .efuse_offset = 0x0018, > + .efuse_offset = 0x0, > .efuse_mask = 0x07c0, > .efuse_shift = 0x6, > .rev_offset = 0x0014, This may work but it really shouldn't. Unfortunately when I sent out the am62ax and am62px support I missed the .rev_offset and now it's pointed to some random spot in the WKUP_CTRL_MMR space. How it worked on my bench was complete luck (or bad luck depending on how we view this). We'll need to pull out a OMAP3 chip and try to separate the OMAP and K3 OPN decoding before we can move the .efuse_offset ~Bryan
Hi Bryan, On Sep 02, 2024 at 18:34:39 -0500, Bryan Brattlof wrote: > On September 2, 2024 thus sayeth Dhruva Gole: > > Since the efuse_offset is basically derived from the syscon node, we no > > longer need to use any efuse_offset for AM625. This is in line with how > > the AM62Ax and AM62Px are already doing. > > > > Signed-off-by: Dhruva Gole <d-gole@ti.com> > > --- > > drivers/cpufreq/ti-cpufreq.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c > > index ba621ce1cdda..98e320832f78 100644 > > --- a/drivers/cpufreq/ti-cpufreq.c > > +++ b/drivers/cpufreq/ti-cpufreq.c > > @@ -313,7 +313,7 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = { > > > > static struct ti_cpufreq_soc_data am625_soc_data = { > > .efuse_xlate = am625_efuse_xlate, > > - .efuse_offset = 0x0018, > > + .efuse_offset = 0x0, > > .efuse_mask = 0x07c0, > > .efuse_shift = 0x6, > > .rev_offset = 0x0014, > > This may work but it really shouldn't. Unfortunately when I sent out the > am62ax and am62px support I missed the .rev_offset and now it's pointed > to some random spot in the WKUP_CTRL_MMR space. How it worked on my Thanks for taking the time to review. If by "this" you mean the rev offset, then it's anyway unused. I have mentioned the dependency [1] in the cover letter where I am using the revision info based on the socinfo driver. I have also applied the AM62Ax [2] patches that you'd sent and tested it here on AM62A [3]. > bench was complete luck (or bad luck depending on how we view this). > > We'll need to pull out a OMAP3 chip and try to separate the OMAP and K3 > OPN decoding before we can move the .efuse_offset > > ~Bryan The efuse part of this driver is completely fine, and IMHO doesn't need any playing with. What does need MAJOR fixing is the rev part of it. I am wondering if we even really use the revision info to determine what OPP the devices support in most cases (I am confident that it's useless for AM62, 62A and 62P). In such cases we should rather even make the get_revision part optional. I am open to suggestions if there's another way to do it than what I have done in this series and in the dependency[1]. Looking at `drivers/cpufreq/sti-cpufreq.c`: If they fail to obtain a version then they simply do version[0] = BIT(major); If that's acceptable for devices that have revision as _don't care_ then I can do that as well. I am also open to the idea of moving the new K3 devices into a new ti-k3-cpufreq driver if required, to avoid over complicating in this driver itself with more quirks and legacy vs new SoC differences which exist at a both SW and HW layer. Perhaps Viresh/ Nishanth can comment on what they think is the best way to move forward. But otherwise, I don't think these patches are fundamentally wrong or that they won't work, unless there's something I've missed. [1] https://lore.kernel.org/all/20240902092135.2826470-1-d-gole@ti.com [2] https://lore.kernel.org/all/20240826-opp-v3-1-0934f8309e13@ti.com/ [3] https://gist.github.com/DhruvaG2000/d0c360b0bd7e43d0fd28cfe3eab941d2
diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c index ba621ce1cdda..98e320832f78 100644 --- a/drivers/cpufreq/ti-cpufreq.c +++ b/drivers/cpufreq/ti-cpufreq.c @@ -313,7 +313,7 @@ static const struct soc_device_attribute k3_cpufreq_soc[] = { static struct ti_cpufreq_soc_data am625_soc_data = { .efuse_xlate = am625_efuse_xlate, - .efuse_offset = 0x0018, + .efuse_offset = 0x0, .efuse_mask = 0x07c0, .efuse_shift = 0x6, .rev_offset = 0x0014,
Since the efuse_offset is basically derived from the syscon node, we no longer need to use any efuse_offset for AM625. This is in line with how the AM62Ax and AM62Px are already doing. Signed-off-by: Dhruva Gole <d-gole@ti.com> --- drivers/cpufreq/ti-cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)