linux-kernel - Re: [PATCH] ARM: dts: omap3-gta04: Drop superfluous omap36xx compatible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 4 Oct 2023 14:42:47 +0200
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Andreas Kemnade <andreas@...nade.info>
Cc:     bcousson@...libre.com, Tony Lindgren <tony@...mide.com>,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, linux-omap@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ARM: dts: omap3-gta04: Drop superfluous omap36xx
 compatible



> Am 04.10.2023 um 13:54 schrieb Andreas Kemnade <andreas@...nade.info>:
> 
> On Wed, 4 Oct 2023 13:39:03 +0200
> "H. Nikolaus Schaller" <hns@...delico.com> wrote:
> 
>>> Am 04.10.2023 um 13:03 schrieb Andreas Kemnade <andreas@...nade.info>:
>>> 
>>> On Wed, 4 Oct 2023 12:50:16 +0200
>>> "H. Nikolaus Schaller" <hns@...delico.com> wrote:
>>> 
>>>> Hi Andreas,
>>>> 
>>>>> Am 04.10.2023 um 08:53 schrieb Andreas Kemnade <andreas@...nade.info>:
>>>>> 
>>>>> Drop omap36xx compatible as done in other omap3630 devices.
>>>>> This has apparently fallen through the lattice.
>>>>> 
>>>>> Signed-off-by: Andreas Kemnade <andreas@...nade.info>
>>>>> ---
>>>>> arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
>>>>> index b6b27e93857f..3661340009e7 100644
>>>>> --- a/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
>>>>> +++ b/arch/arm/boot/dts/ti/omap/omap3-gta04.dtsi
>>>>> @@ -11,7 +11,7 @@
>>>>> 
>>>>> / {
>>>>> 	model = "OMAP3 GTA04";
>>>>> -	compatible = "goldelico,gta04", "ti,omap3630", "ti,omap36xx", "ti,omap3";    
>>>> 
>>>> there seem to be some more references to ti,omap36xx:
>>>> 
>>>> arch/arm/boot/dts/ti/omap/omap3-lilly-a83x.dtsi:	compatible = "incostartec,omap3-lilly-a83x", "ti,omap3630", "ti,omap36xx", "ti,omap3";  
>>> 
>>> apperently all the dtsi are fallen through the lattice when handling the dts.
>>> 
>>> 
>>>> arch/arm/mach-omap2/board-generic.c:	"ti,omap36xx",
>>>> drivers/clk/ti/dpll.c:	     of_machine_is_compatible("ti,omap36xx")) &&
>>>> drivers/cpufreq/ti-cpufreq.c:	{ .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, },
>>>> 
>>>> So are you sure that we can remove it without replacement or code fixes in dpll and cpufreq (board-generic is probably no issue)?
>>>> 
>>> see discussion of:
>>> 
>>> commit e341f338180c84cd98af3016cf5bcfde45a041fb
>>> Author: Andrew Davis <afd@...com>
>>> Date:   Thu Feb 16 09:33:38 2023 -0600
>>> 
>>>   ARM: dts: omap: Drop ti,omap36xx compatible  
>> 
>> Ah, I wasn't aware of this.
>> 
>>> 
>>> all the places also basically check for omap36xx || omap3630.  
>> 
>> 
>> Yes, I have checked but drivers/cpufreq/ti-cpufreq.c seems to be an
>> exception (unless I am missing some other patch).
>> 
> No:
>        { .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, },
>        { .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },

Well, in v6.6-rc4 I see:

static const struct of_device_id ti_cpufreq_of_match[] = {
	{ .compatible = "ti,am33xx", .data = &am3x_soc_data, },
	{ .compatible = "ti,am3517", .data = &am3517_soc_data, },
	{ .compatible = "ti,am43", .data = &am4x_soc_data, },
	{ .compatible = "ti,dra7", .data = &dra7_soc_data },
	{ .compatible = "ti,omap34xx", .data = &omap34xx_soc_data, },
	{ .compatible = "ti,omap36xx", .data = &omap36xx_soc_data, },
	{ .compatible = "ti,am625", .data = &am625_soc_data, },
	{ .compatible = "ti,am62a7", .data = &am625_soc_data, },
	/* legacy */
	{ .compatible = "ti,omap3430", .data = &omap34xx_soc_data, },
	{ .compatible = "ti,omap3630", .data = &omap36xx_soc_data, },
	{},
};

Either the "/* legacy */" or the "ti,omap36xx" seems as if it should be removed.
But it seems to break the systematic approach of this table.

> The bindings also only specify omap3630.

What I think is that the background was (before bindings documentation
was invented) that there are drivers covering all variants of omap36xx
(incl. am37xx and dm37xx) and some parts specific to a single version.

So maybe it could have been missing in the bindings?

Anyways it seems to need some cleanup to remove all references to 
omap36xx before it is forgotten...

BR,
Nikolaus