Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751554AbaANOU3 (ORCPT ); Tue, 14 Jan 2014 09:20:29 -0500 Received: from mo3.mail-out.ovh.net ([178.32.228.3]:53210 "EHLO mo3.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751329AbaANOUY (ORCPT ); Tue, 14 Jan 2014 09:20:24 -0500 MIME-Version: 1.0 In-Reply-To: <52D0FB78.6010102@overkiz.com> References: <1389270709-32662-1-git-send-email-jjhiblot@traphandler.com> <1389270709-32662-8-git-send-email-jjhiblot@traphandler.com> <52CED586.7030602@overkiz.com> <52D0FB78.6010102@overkiz.com> Date: Tue, 14 Jan 2014 15:20:18 +0100 Message-ID: Subject: Re: [PATCH v2 07/12] at91: dt: smc: Added smc bus driver From: Jean-Jacques Hiblot To: boris brezillon , nicolas.ferre@atmel.com, plagnioj@jcrosoft.com Cc: Jean-Jacques Hiblot , linux-arm-kernel@lists.infradead.org, Linux Kernel Mailing List Content-Type: text/plain; charset=ISO-8859-1 X-Ovh-Tracer-Id: 5174917448793937944 X-Ovh-Remote: 209.85.192.181 (mail-pd0-f181.google.com) X-Ovh-Local: 213.186.33.20 (ns0.ovh.net) X-OVH-SPAMSTATE: OK X-OVH-SPAMSCORE: -100 X-OVH-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeejtddrfeehucetufdoteggodetrfcurfhrohhfihhlvgemucfqggfjnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-Spam-Check: DONE|U 0.5/N X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeejtddrfeehucetufdoteggodetrfcurfhrohhfihhlvgemucfqggfjnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2014/1/11 boris brezillon : > On 09/01/2014 22:04, Jean-Jacques Hiblot wrote: >> >> Hi Boris, >> >> 2014/1/9 boris brezillon : >>> >>> Hello JJ, >>> >>> >>> On 09/01/2014 13:31, Jean-Jacques Hiblot wrote: >>>> >>>> The EBI/SMC external interface is used to access external peripherals >>>> (NAND >>>> and Ethernet controller in the case of sam9261ek). Different >>>> configurations and >>>> timings are required for those peripherals. This bus driver can be used >>>> to >>>> setup the bus timings/configuration from the device tree. >>>> It currently accepts timings in clock period and in nanoseconds. >>>> >>>> Signed-off-by: Jean-Jacques Hiblot >>>> --- >>>> drivers/memory/Kconfig | 10 ++ >>>> drivers/memory/Makefile | 1 + >>>> drivers/memory/atmel-smc.c | 431 >>>> +++++++++++++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 442 insertions(+) >>>> create mode 100644 drivers/memory/atmel-smc.c >>>> >>>> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig >>>> index 29a11db..fbdfd63 100644 >>>> --- a/drivers/memory/Kconfig >>>> +++ b/drivers/memory/Kconfig >>>> @@ -50,4 +50,14 @@ config TEGRA30_MC >>>> analysis, especially for IOMMU/SMMU(System Memory Management >>>> Unit) module. >>>> +config ATMEL_SMC >>>> + bool "Atmel SMC/EBI driver" >>>> + default y >>>> + depends on SOC_AT91SAM9 && OF >>>> + help >>>> + Driver for Atmel SMC/EBI controller. >>>> + Used to configure the EBI (external bus interface) when the >>>> device- >>>> + tree is used. This bus supports NANDs, external ethernet >>>> controller, >>>> + SRAMs, ATA devices, etc. >>>> + >>>> endif >>>> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile >>>> index 969d923..101abc4 100644 >>>> --- a/drivers/memory/Makefile >>>> +++ b/drivers/memory/Makefile >>>> @@ -9,3 +9,4 @@ obj-$(CONFIG_TI_EMIF) += emif.o >>>> obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o >>>> obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o >>>> obj-$(CONFIG_TEGRA30_MC) += tegra30-mc.o >>>> +obj-$(CONFIG_ATMEL_SMC) += atmel-smc.o >>>> diff --git a/drivers/memory/atmel-smc.c b/drivers/memory/atmel-smc.c >>>> new file mode 100644 >>>> index 0000000..0a1d9ba >>>> --- /dev/null >>>> +++ b/drivers/memory/atmel-smc.c >>>> @@ -0,0 +1,431 @@ >>>> +/* >>>> + * EBI driver for Atmel SAM9 chips >>>> + * inspired by the fsl weim bus driver >>>> + * >>>> + * Copyright (C) 2013 JJ Hiblot. >>>> + * >>>> + * This file is licensed under the terms of the GNU General Public >>>> + * License version 2. This program is licensed "as is" without any >>>> + * warranty of any kind, whether express or implied. >>>> + */ >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>> >>> >>> You should avoid machine specific headers inclusions: we're trying to get >>> rid of them. >>> >>> Duplicate the code and macros you need in your driver instead. >> >> Is this the right way? > > Not necessarily. But in any case you should not reference machine specific > headers in new drivers, because the ARM architecture maintainers are > trying to get all architecture specific code moved into regular subsystems. > > There surely is a lot of pros to this approach (I'll let others detail > these). > The main one I see is that the subsystem maintainer is then able to track > common designs in all available drivers and provide a common framework > in order to : > 1) ease other drivers development > 2) avoid code duplication > > In your case, this leaves 2 solutions: > 1) move the sam9_smc fonctions in the new driver and move the sam9_smc > into include/linux/memory (which apparently does not exist). > 2) copy all the functions and definition you need in your driver in order to > get > rid of the old implementation, and choose which one to compile using > Kconfig > options. > > If you think the sam9_smc existing implementation already match your new > driver > needs, I strongly recommend choosing solution 1, as it will help smoothly > move to your > new driver without having to modify already existing drivers using sam9_smc > functions > (except for the new header path ;)). > I agree. Merging the code of arch/arm/mach-at91/sam9_smc.c into this new driver makes sense. Nicolas, Jean-Christophe, what is the stand point of the atmel's maintainers on this ? > >> We usually try to avoid duplication. > > > > >> >>> >>>> + >>>> +struct smc_data { >>>> + struct clk *bus_clk; >>>> + void __iomem *base; >>>> + struct device *dev; >>>> +}; >>>> + >>>> +struct at91_smc_devtype { >>>> + unsigned int cs_count; >>>> +}; >>>> + >>>> +static const struct at91_smc_devtype sam9261_smc_devtype = { >>>> + .cs_count = 6, >>>> +}; >>>> + >>>> +static const struct of_device_id smc_id_table[] = { >>>> + { .compatible = "atmel,at91sam9261-smc", .data = >>>> &sam9261_smc_devtype}, >>>> + { } >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, smc_id_table); >>>> + >>>> +struct smc_parameters_type { >>>> + const char *name; >>>> + u16 width; >>>> + u16 shift; >>>> +}; >>>> + >>>> +static const struct smc_parameters_type smc_parameters[] = { >>>> + {"smc,burst_size", 2, 28}, >>>> + {"smc,burst_enabled", 1, 24}, >>>> + {"smc,tdf_mode", 1, 20}, >>>> + {"smc,bus_width", 2, 12}, >>>> + {"smc,byte_access_type", 1, 8}, >>>> + {"smc,nwait_mode", 2, 4}, >>>> + {"smc,write_mode", 1, 0}, >>>> + {"smc,read_mode", 1, 1}, >>>> + {NULL} >>>> +}; >>>> + >>>> +static int get_mode_register_from_dt(struct smc_data *smc, >>>> + struct device_node *np, >>>> + struct sam9_smc_config *cfg) >>>> +{ >>>> + int ret; >>>> + u32 val; >>>> + struct device *dev = smc->dev; >>>> + const struct smc_parameters_type *p = smc_parameters; >>>> + u32 mode = cfg->mode; >>>> + >>>> + while (p->name) { >>>> + ret = of_property_read_u32(np, p->name , &val); >>>> + if (ret == -EINVAL) { >>>> + dev_dbg(dev, "%s: property %s not set.\n", >>>> np->name, >>>> + p->name); >>>> + p++; >>>> + continue; >>>> + } else if (ret) { >>>> + dev_err(dev, "%s: can't get property %s.\n", >>>> np->name, >>>> + p->name); >>>> + return ret; >>>> + } >>>> + if (val >= (1<width)) { >>>> + dev_err(dev, "%s: property %s out of range.\n", >>>> + np->name, p->name); >>>> + return -ERANGE; >>>> + } >>>> + mode &= ~(((1<width)-1) << p->shift); >>>> + mode |= (val << p->shift); >>>> + p++; >>>> + } >>>> + cfg->mode = mode; >>>> + return 0; >>>> +} >>>> + >>>> +static int generic_timing_from_dt(struct smc_data *smc, struct >>>> device_node *np, >>>> + struct sam9_smc_config *cfg) >>>> +{ >>>> + u32 val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_read_setup" , &val)) >>>> + cfg->ncs_read_setup = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,nrd_setup" , &val)) >>>> + cfg->nrd_setup = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_write_setup" , &val)) >>>> + cfg->ncs_write_setup = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,nwe_setup" , &val)) >>>> + cfg->nwe_setup = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_read_pulse" , &val)) >>>> + cfg->ncs_read_pulse = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,nrd_pulse" , &val)) >>>> + cfg->nrd_pulse = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_write_pulse" , &val)) >>>> + cfg->ncs_write_pulse = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,nwe_pulse" , &val)) >>>> + cfg->nwe_pulse = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,read_cycle" , &val)) >>>> + cfg->read_cycle = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,write_cycle" , &val)) >>>> + cfg->write_cycle = val; >>>> + >>>> + if (!of_property_read_u32(np, "smc,tdf_cycles" , &val)) >>>> + cfg->tdf_cycles = val; >>>> + >>>> + return get_mode_register_from_dt(smc, np, cfg); >>>> +} >>>> + >>>> +/* convert the time in ns in a number of clock cycles */ >>>> +static u32 ns_to_cycles(u32 ns, u32 clk) >>>> +{ >>>> + /* >>>> + * convert the clk to kHz for the rest of the calculation to >>>> avoid >>>> + * overflow >>>> + */ >>>> + u32 clk_kHz = clk / 1000; >>>> + u32 ncycles = ((ns * clk_kHz) + 1000000 - 1) / 1000000; >>> >>> What about using an u64 type and do_div ? >> >> easier and faster (though it's not the point here) this way, and kHz >> ist not so imprecise :-) >> >>>> + return ncycles; >>>> +} >>>> + >>>> +static u32 cycles_to_coded_cycle(u32 cycles, int a, int b) >>>> +{ >>>> + u32 mask_high = (1 << a) - 1; >>>> + u32 mask_low = (1 << b) - 1; >>>> + u32 coded; >>>> + >>>> + /* check if the value can be described with the coded format */ >>>> + if (cycles & (mask_high & ~mask_low)) { >>>> + /* not representable. we need to round up */ >>>> + cycles |= mask_high; >>>> + cycles += 1; >>>> + } >>>> + /* Now the value can be represented in the coded format */ >>>> + coded = (cycles & ~mask_high) >> (a - b); >>>> + coded |= (cycles & mask_low); >>>> + return coded; >>>> +} >>>> + >>>> +static u32 ns_to_rw_cycles(u32 ns, u32 clk) >>>> +{ >>>> + return cycles_to_coded_cycle(ns_to_cycles(ns, clk), 8, 7); >>>> +} >>>> + >>>> +static u32 ns_to_pulse_cycles(u32 ns, u32 clk) >>>> +{ >>>> + return cycles_to_coded_cycle(ns_to_cycles(ns, clk), 8, 6); >>>> +} >>>> + >>>> +static u32 ns_to_setup_cycles(u32 ns, u32 clk) >>>> +{ >>>> + return cycles_to_coded_cycle(ns_to_cycles(ns, clk), 7, 5); >>>> +} >>>> + >>>> +static u32 cycles_to_ns(u32 cycles, u32 clk) >>>> +{ >>>> + /* >>>> + * convert the clk to kHz for the rest of the calculation to >>>> avoid >>>> + * overflow >>>> + */ >>>> + u32 clk_kHz = clk / 1000; >>> >>> >>> Ditto (u64 + do_div). >>> >>>> + return (cycles * 1000000) / clk_kHz; >>>> +} >>>> + >>>> +static u32 coded_cycle_to_cycles(u32 coded, int a, int b) >>>> +{ >>>> + u32 cycles = (coded >> b) << a; >>>> + u32 mask_low = (1 << b) - 1; >>>> + cycles |= (coded & mask_low); >>>> + return cycles; >>>> +} >>>> + >>>> +static u32 rw_cycles_to_ns(u32 reg, u32 clk) >>>> +{ >>>> + return cycles_to_ns(coded_cycle_to_cycles(reg, 8, 7), clk); >>>> +} >>>> + >>>> +static u32 pulse_cycles_to_ns(u32 reg, u32 clk) >>>> +{ >>>> + return cycles_to_ns(coded_cycle_to_cycles(reg, 8, 6), clk); >>>> +} >>>> + >>>> +static u32 setup_cycles_to_ns(u32 reg, u32 clk) >>>> +{ >>>> + return cycles_to_ns(coded_cycle_to_cycles(reg, 7, 5), clk); >>>> +} >>>> + >>>> +static void dump_timing(struct smc_data *smc, struct sam9_smc_config >>>> *config) >>>> +{ >>>> + u32 freq = clk_get_rate(smc->bus_clk); >>>> + struct device *dev = smc->dev; >>>> + >>>> +#define DUMP(fn, y) dev_info(dev, "%s : 0x%x (%d ns)\n", #y, >>>> config->y,\ >>>> + fn(config->y, freq)) >>>> +#define DUMP_PULSE(y) DUMP(pulse_cycles_to_ns, y) >>>> +#define DUMP_RWCYCLE(y) DUMP(rw_cycles_to_ns, y) >>>> +#define DUMP_SETUP(y) DUMP(setup_cycles_to_ns, y) >>>> +#define DUMP_SIMPLE(y) DUMP(cycles_to_ns, y) >>>> + >>>> + DUMP_SETUP(nwe_setup); >>>> + DUMP_SETUP(ncs_write_setup); >>>> + DUMP_SETUP(nrd_setup); >>>> + DUMP_SETUP(ncs_read_setup); >>>> + DUMP_PULSE(nwe_pulse); >>>> + DUMP_PULSE(ncs_write_pulse); >>>> + DUMP_PULSE(nrd_pulse); >>>> + DUMP_PULSE(ncs_read_pulse); >>>> + DUMP_RWCYCLE(write_cycle); >>>> + DUMP_RWCYCLE(read_cycle); >>>> + DUMP_SIMPLE(tdf_cycles); >>>> +} >>>> + >>>> +static int ns_timing_from_dt(struct smc_data *smc, struct device_node >>>> *np, >>>> + struct sam9_smc_config *cfg) >>>> +{ >>>> + u32 t_ns; >>>> + u32 freq = clk_get_rate(smc->bus_clk); >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_read_setup" , &t_ns)) >>>> + cfg->ncs_read_setup = ns_to_setup_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,nrd_setup" , &t_ns)) >>>> + cfg->nrd_setup = ns_to_setup_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_write_setup" , &t_ns)) >>>> + cfg->ncs_write_setup = ns_to_setup_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,nwe_setup" , &t_ns)) >>>> + cfg->nwe_setup = ns_to_setup_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_read_pulse" , &t_ns)) >>>> + cfg->ncs_read_pulse = ns_to_pulse_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,nrd_pulse" , &t_ns)) >>>> + cfg->nrd_pulse = ns_to_pulse_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,ncs_write_pulse" , &t_ns)) >>>> + cfg->ncs_write_pulse = ns_to_pulse_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,nwe_pulse" , &t_ns)) >>>> + cfg->nwe_pulse = ns_to_pulse_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,read_cycle" , &t_ns)) >>>> + cfg->read_cycle = ns_to_rw_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,write_cycle" , &t_ns)) >>>> + cfg->write_cycle = ns_to_rw_cycles(t_ns, freq); >>>> + >>>> + if (!of_property_read_u32(np, "smc,tdf_cycles" , &t_ns)) >>>> + cfg->tdf_cycles = ns_to_cycles(t_ns, freq); >>>> + >>>> + return get_mode_register_from_dt(smc, np, cfg); >>>> +} >>>> + >>>> +struct converter { >>>> + const char *name; >>>> + int (*fn) (struct smc_data *smc, struct device_node *np, >>>> + struct sam9_smc_config *cfg); >>>> +}; >>>> +static const struct converter converters[] = { >>>> + {"raw", generic_timing_from_dt}, >>>> + {"nanosec", ns_timing_from_dt}, >>>> +}; >>> >>> >>> Could you use more specific names like: >>> "atmel,smc-converter-generic" >>> "atmel,smc-converter-nand" >>> ... >> >> Isn't it a bit redudant? smc,converter = "atmel,smc-converter-generic"; >> >>> IMHO the timing unit should be specified in the property names: >>> smc,ncs_read_setup-ns >>> smc,ncs_read_setup-cycles >>> >> True. Although cycles is misleading. It's more a raw register value. >> For pulse, setup and rw cycle, the register value is not identical to >> the number of cycles. >>> >>> >>> >>>> + >>>> +/* Parse and set the timing for this device. */ >>>> +static int smc_timing_setup(struct smc_data *smc, struct device_node >>>> *np, >>>> + const struct at91_smc_devtype *devtype) >>>> +{ >>>> + int ret; >>>> + u32 cs; >>>> + int i; >>>> + struct device *dev = smc->dev; >>>> + const struct converter *converter; >>>> + const char *converter_name = NULL; >>>> + struct sam9_smc_config cfg; >>>> + >>>> + ret = of_property_read_u32(np, "smc,cs" , &cs); >>> >>> >>> Shouldn't this be stored in the reg property ? >>> After all, in your DM9000 patch you use "@cs,offset" to identify the >>> node... >> >> True >> >>> >>>> + if (ret < 0) { >>>> + dev_err(dev, "missing mandatory property : smc,cs\n"); >>>> + return ret; >>>> + } >>>> + if (cs >= devtype->cs_count) { >>>> + dev_err(dev, "invalid value for property smc,cs (=%d)." >>>> + "Must be in range 0 to %d\n", cs, devtype->cs_count-1); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + of_property_read_string(np, "smc,converter", &converter_name); >>> >>> >>> What about using the "compatible" property + struct of_device_id instead >>> of >>> "smc,converter" property + struct converter ? >> >> Because one instance of the driver handles several chip selects and >> each may use a different converter. >> >>> >>>> + if (converter_name) { >>>> + for (i = 0; i < ARRAY_SIZE(converters); i++) >>>> + if (strcmp(converters[i].name, converter_name) >>>> == >>>> 0) >>>> + converter = &converters[i]; >>>> + if (!converter) { >>>> + dev_info(dev, "unknown converter. aborting\n"); >>>> + return -EINVAL; >>>> + } >>>> + } else { >>>> + dev_dbg(dev, "cs %d: no smc converter provided. using " >>>> + "raw register values\n", cs); >>>> + converter = &converters[0]; >>>> + } >>>> + dev_dbg(dev, "cs %d using converter : %s\n", cs, >>>> converter->name); >>>> + sam9_smc_cs_read(smc->base + (0x10 * cs), &cfg); >>>> + converter->fn(smc, np, &cfg); >>>> + ret = sam9_smc_check_cs_configuration(&cfg); >>>> + if (ret < 0) { >>>> + dev_info(dev, "invalid smc configuration for cs %d." >>>> + "clipping values\n", cs); >>>> + sam9_smc_clip_cs_configuration(&cfg); >>>> + dump_timing(smc, &cfg); >>>> + } >>>> +#ifdef DEBUG >>>> + else >>>> + dump_timing(smc, &cfg); >>>> +#endif >>> >>> >>> I'm not a big fan of #ifdef blocks inside the code. >>> You could define a dummy dump_timing function if DEBUG is not defined: >>> >>> #ifdef DEBUG >>> >>> >>> static void dump_timing(struct smc_data *smc, struct sam9_smc_config >>> *config) >>> { >>> /* your implementation */ >>> } >>> >>> #else >>> >>> static inline void dump_timing(struct smc_data *smc, struct >>> sam9_smc_config >>> *config) >>> { >>> } >>> >>> #endif >>> >>> Or just use dev_dbg when printing things in dump_timing. >>> >> I wanted to know the values when they were modified (clipped) by the >> driver. But it could be removed, knowing that clipping occurred is >> enough. >>> >>> >>>> + >>>> + sam9_smc_cs_configure(smc->base + (0x10 * cs), &cfg); >>>> + return 0; >>>> +} >>>> + >>>> +static int smc_parse_dt(struct smc_data *smc) >>>> +{ >>>> + struct device *dev = smc->dev; >>>> + const struct of_device_id *of_id = of_match_device(smc_id_table, >>>> dev); >>>> + const struct at91_smc_devtype *devtype = of_id->data; >>>> + struct device_node *child; >>>> + int ret; >>>> + >>>> + for_each_child_of_node(dev->of_node, child) { >>>> + if (!child->name) >>>> + continue; >>>> + if (!of_device_is_available(child)) >>>> + continue; >>>> + ret = smc_timing_setup(smc, child, devtype); >>>> + if (ret) { >>>> + static struct property status = { >>>> + .name = "status", >>>> + .value = "disabled", >>>> + .length = sizeof("disabled"), >>>> + }; >>>> + dev_err(dev, "%s set timing failed. This node >>>> will >>>> be disabled.\n", >>>> + child->full_name); >>>> + ret = of_update_property(child, &status); >>>> + if (ret < 0) { >>>> + dev_err(dev, "can't disable %s. aborting >>>> probe\n", >>>> + child->full_name); >>>> + break; >>> >>> >>> The concept of disabling the device if timings cannot be met sounds >>> interresting... >>> Let's see what other maintainers say about this :). >>> >>> >>>> + } >>>> + } >>>> + } >>>> + >>>> + ret = of_platform_populate(dev->of_node, >>>> of_default_bus_match_table, >>>> + NULL, dev); >>>> + if (ret) >>>> + dev_err(dev, "%s fail to create devices.\n", >>>> + dev->of_node->full_name); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int smc_probe(struct platform_device *pdev) >>>> +{ >>>> + struct resource *res; >>>> + int ret; >>>> + void __iomem *base; >>>> + struct clk *clk; >>>> + struct smc_data *smc = devm_kzalloc(&pdev->dev, sizeof(struct >>>> smc_data), >>>> + GFP_KERNEL); >>>> + >>>> + if (!smc) >>>> + return -ENOMEM; >>>> + >>>> + smc->dev = &pdev->dev; >>>> + >>>> + /* get the resource */ >>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + base = devm_request_and_ioremap(&pdev->dev, res); >>>> + if (IS_ERR(base)) { >>>> + dev_err(&pdev->dev, "can't map SMC base address\n"); >>>> + return PTR_ERR(base); >>>> + } >>>> + >>>> + /* get the clock */ >>>> + clk = devm_clk_get(&pdev->dev, "smc"); >>>> + if (IS_ERR(clk)) >>>> + return PTR_ERR(clk); >>>> + >>>> + smc->bus_clk = clk; >>>> + smc->base = base; >>>> + >>>> + /* parse the device node */ >>>> + ret = smc_parse_dt(smc); >>>> + if (!ret) >>>> + dev_info(&pdev->dev, "Driver registered.\n"); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static struct platform_driver smc_driver = { >>>> + .driver = { >>>> + .name = "atmel-smc", >>>> + .owner = THIS_MODULE, >>>> + .of_match_table = smc_id_table, >>>> + }, >>>> +}; >>>> +module_platform_driver_probe(smc_driver, smc_probe); >>>> + >>>> +MODULE_AUTHOR("JJ Hiblot"); >>>> +MODULE_DESCRIPTION("Atmel's SMC/EBI driver"); >>>> +MODULE_LICENSE("GPL"); >>> >>> >>> >>> That's all for now. :) >>> >>> I'll try to test it this week end on a sama5 board. >> >> Thanks >> >>> Best Regards, >>> >>> Boris > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/