| 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
| ||
|
Message-ID: <5295D102.2010101@ti.com>
Date: Wed, 27 Nov 2013 13:01:22 +0200
From: "ivan.khoronzhuk" <ivan.khoronzhuk@...com>
To: Sekhar Nori <nsekhar@...com>
CC: <rob@...dley.net>, <linux@....linux.org.uk>,
<mark.rutland@....com>, <devicetree@...r.kernel.org>,
<grygorii.strashko@...com>, <pawel.moll@....com>,
<swarren@...dotorg.org>, <ijc+devicetree@...lion.org.uk>,
<galak@...nel.crashing.org>, <rob.herring@...xeda.com>,
<linux-kernel@...r.kernel.org>, <linux-mtd@...ts.infradead.org>,
<linux-arm-kernel@...ts.infradead.org>,
Brian Norris <computersforpeace@...il.com>,
David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH] ARM: davinci: aemif: get rid of davinci-nand driver dependency
on aemif
On 11/27/2013 10:35 AM, Sekhar Nori wrote:
> + MTD maintainers
>
> On Tuesday 26 November 2013 01:30 AM, Ivan Khoronzhuk wrote:
>> The problem that the set timings code contains the call of Davinci
>> platform function davinci_aemif_setup_timing() which is not
>> accessible if kernel is built for another platform like Keystone.
>>
>> The Keysone platform is going to use TI AEMIF driver.
>> If TI AEMIF is used we don't need to set timings and bus width.
>> It is done by AEMIF driver.
>>
>> To get rid of davinci-nand driver dependency on aemif platform code
>> we moved aemif code to davinci platform.
>>
>> The platform AEMIF code (aemif.c) has to be removed once Davinci
>> will be converted to DT and use ti-aemif.c driver.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@...com>
>> ---
>> arch/arm/mach-davinci/aemif.c | 73 ++++++++++++++++++++++-
>> arch/arm/mach-davinci/board-da830-evm.c | 3 +
>> arch/arm/mach-davinci/board-da850-evm.c | 3 +
>> arch/arm/mach-davinci/board-dm355-evm.c | 5 ++
>> arch/arm/mach-davinci/board-dm355-leopard.c | 5 ++
>> arch/arm/mach-davinci/board-dm365-evm.c | 4 ++
>> arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++
>> arch/arm/mach-davinci/board-dm646x-evm.c | 3 +
>> arch/arm/mach-davinci/board-mityomapl138.c | 3 +
>> arch/arm/mach-davinci/board-neuros-osd2.c | 9 ++-
>> arch/arm/mach-davinci/devices-tnetv107x.c | 3 +
>> drivers/mtd/nand/davinci_nand.c | 23 -------
>> include/linux/platform_data/mtd-davinci-aemif.h | 5 +-
>> 13 files changed, 116 insertions(+), 28 deletions(-)
>
> [...]
>
>> +
>> +/**
>> + * davinci_aemif_setup - setup AEMIF interface by davinci_nand_pdata
>> + * @pdev - link to platform device to setup settings for
>> + *
>> + * This function does not use any locking while programming the AEMIF
>> + * because it is expected that there is only one user of a given
>> + * chip-select.
>> + *
>> + * Returns 0 on success, else negative errno.
>> + */
>> +int davinci_aemif_setup(struct platform_device *pdev)
>> +{
>> + struct davinci_nand_pdata *pdata = dev_get_platdata(&pdev->dev);
>> + uint32_t val;
>> + struct resource *res;
>> + void __iomem *base;
>> + int ret = 0;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + if (!res) {
>> + dev_err(&pdev->dev, "cannot get IORESOURCE_MEM\n");
>> + return -ENOMEM;
>> + }
>> +
>> + base = ioremap(res->start, resource_size(res));
>> + if (!base) {
>> + dev_err(&pdev->dev, "ioremap failed for resource %pR\n", res);
>> + ret = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + /*
>> + * Setup Async configuration register in case we did not boot
>> + * from NAND and so bootloader did not bother to set it up.
>> + */
>> + val = davinci_aemif_readl(base, A1CR_OFFSET + pdev->id * 4);
>
> The AEMIF clock has to be enabled before this. The NAND driver might
> load much later.
>
Ok
>> + /*
>> + * Extended Wait is not valid and Select Strobe mode is not
>> + * used
>> + */
>> + val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
>> + if (pdata->options & NAND_BUSWIDTH_16)
>> + val |= 0x1;
>> +
>> + davinci_aemif_writel(base, A1CR_OFFSET + pdev->id * 4, val);
>> +
>> + if (pdata->timing)
>> + ret = davinci_aemif_setup_timing(pdata->timing, base, pdev->id);
>> +
>> + if (ret < 0)
>> + dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
>> +
>> +err:
>> + iounmap(base);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_setup);
>
> No need to export this symbol as nothing apart from platform code uses it.
>
Ok
>> static const short mityomap_mii_pins[] = {
>> diff --git a/arch/arm/mach-davinci/board-neuros-osd2.c b/arch/arm/mach-davinci/board-neuros-osd2.c
>> index bb680af..a7d6668 100644
>> --- a/arch/arm/mach-davinci/board-neuros-osd2.c
>> +++ b/arch/arm/mach-davinci/board-neuros-osd2.c
>> @@ -31,6 +31,7 @@
>> #include <linux/platform_data/mmc-davinci.h>
>> #include <linux/platform_data/mtd-davinci.h>
>> #include <linux/platform_data/usb-davinci.h>
>> +#include <linux/platform_data/mtd-davinci-aemif.h>
>>
>> #include <asm/mach-types.h>
>> #include <asm/mach/arch.h>
>> @@ -192,9 +193,15 @@ static __init void davinci_ntosd2_init(void)
>> davinci_cfg_reg(DM644X_ATAEN_DISABLE);
>>
>> /* only one device will be jumpered and detected */
>> - if (HAS_NAND)
>> + if (HAS_NAND) {
>> platform_device_register(
>> &davinci_ntosd2_nandflash_device);
>> +
>> + if (davinci_aemif_setup(
>> + &davinci_ntosd2_nandflash_device))
>> + pr_warn("%s: Cannot configure AEMIF.\n",
>> + __func__);
>
> This is looking really ugly. Can you shorten
> davinci_ntosd2_nandflash_device to just "ntosd2_nandflash" or similar?
The rename is not related to the patch, so I won't do this.
>
> I am yet to test this. Will test once the next version is posted.
>
> Overall, I cannot say I am fan of this approach (mostly because we are
> ending up having two drivers for the same hardware in kernel). But given
> that the other option of adding platform device support to AEMIF driver
> is not acceptable to you, I guess I will live with this.
>
> Thanks,
> Sekhar
>
--
Regards,
Ivan Khoronzhuk
--
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