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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 Jul 2014 13:48:59 +0200 From: Hans de Goede <hdegoede@...hat.com> To: Thierry Reding <thierry.reding@...il.com> CC: Mikko Perttunen <mperttunen@...dia.com>, swarren@...dotorg.org, tj@...nel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-tegra@...r.kernel.org, linux-ide@...r.kernel.org Subject: Re: [PATCH v4 1/8] of: Add NVIDIA Tegra SATA controller binding Hi, On 07/17/2014 01:42 PM, Hans de Goede wrote: > Hi, > > On 07/17/2014 12:52 PM, Thierry Reding wrote: >> On Thu, Jul 17, 2014 at 12:23:47PM +0200, Hans de Goede wrote: > > <snip> > >>> The libahci_platform.c code / ahci_platform.c code is also used for >>> devices going way back who may not yet be using the new clk framework, >>> so where we need to use clk_get(dev, NULL); quoting from libahci_platform.c : >>> >>> for (i = 0; i < AHCI_MAX_CLKS; i++) { >>> /* >>> * For now we must use clk_get(dev, NULL) for the first clock, >>> * because some platforms (da850, spear13xx) are not yet >>> * converted to use devicetree for clocks. For new platforms >>> * this is equivalent to of_clk_get(dev->of_node, 0). >>> */ >>> if (i == 0) >>> clk = clk_get(dev, NULL); >>> else >>> clk = of_clk_get(dev->of_node, i); >>> >>> if (IS_ERR(clk)) { >>> rc = PTR_ERR(clk); >>> if (rc == -EPROBE_DEFER) >>> goto err_out; >>> break; >>> } >>> hpriv->clks[i] = clk; >>> } >>> >>> And there is no devm variant of that, nor is there one to get clocks by index. >>> Note that we also need ahci_platform_put_resources for runtime pm support, so >>> that one is going to stay around anyways and thus there is not that much value >>> in fixing this. >>> >>> So although I like Thierry's idea, if we go this way (which sounds good), we >>> should add support for taking a NULL ahci_platform_resources argument and in >>> that case behave as before, esp. because of the platforms needing the old >>> style clock handling. An advantage of doing this, is that we can simply patch >>> all existing users to pass NULL. >> >> Isn't the "legacy" case really just this: >> >> static const char *const legacy_ahci_clocks[] = { >> NULL >> }; >> >> static const struct ahci_platform_resources legacy_ahci_resources = { >> .num_clocks = ARRAY_SIZE(legacy_ahci_clocks), >> .clocks = legacy_ahci_clocks, >> }; >> >> ? > > Ah yes that would work for the really legacy ones, as well as less legacy > (full dts) ones with only one clk, we need to check if there are current > users which use more then one clk (yes there are which is why MAX_CLKS > was 3) and fixup those to pass in a correct ahci_platform_resources struct > then. > > The checking + fixing up will be a bit of extra work, but I think the end result > will be quite nice. so I'm all in favor of this. Correction, this is not going to work I'm afraid, as not all current dts files set clock-names. So we need a fallback to get clks by index for compatibility with old dts files. At least: arch/arm/boot/dts/sun4i-a10.dtsi and arch/arm/boot/dts/sun7i-a20.dtsi are affected. Note in this case the dts file is typically not burned into a ROM or some such, so IMHO we could get away with requiring a new dts file. So it might be worthwhile to still do a full check if all affected SoCs and see if we can move over to using clock-names for all platforms with more then 1 ahci/sata clk. FWIW I've also just checked imx6q.dts which is the one which has 3 clocks, and that one does define clk names. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists