On 11/21/2015 at 3:51 PM, Hauke Mehrtens wrote: > On 11/20/2015 05:52 AM, Martin Schiller wrote: > > This patch introduces new dedicated "lantiq,pinctrl-<chip>" > devicetree > > bindings, where <chip> is one of "ase", "danube", "xrx100", "xrx200" > or > > "xrx300" and marks the "lantiq,pinctrl-xway" and "lantiq,pinctrl-xr9" > bindings as > > DEPRECATED. (snip) > > diff --git a/drivers/pinctrl/pinctrl-lantiq.h > b/drivers/pinctrl/pinctrl-lantiq.h > > index eb89ba0..e137d13 100644 > > --- a/drivers/pinctrl/pinctrl-lantiq.h > > +++ b/drivers/pinctrl/pinctrl-lantiq.h > > @@ -162,6 +162,14 @@ enum ltq_pin { > > GPIO53, > > GPIO54, > > GPIO55, > > +GPIO56, > > +GPIO57, > > +GPIO58, > > +GPIO59, > > +GPIO60, /* 60 */ > > +GPIO61, > > +GPIO62, > > +GPIO63, > > > > GPIO64, > > GPIO65, > > What is the advantage of using this enum compared to just use integers > directly? Was it intentionally that GPIO64 has the number 56 before? Maybe this GPIO56 - GPIO63 gap has some relation to the xRX100 (aka ar9) SoC, which has 3 1/2 GPIO Ports of 16bits (GPIO0 - GPIO55). > > > diff --git a/drivers/pinctrl/pinctrl-xway.c > b/drivers/pinctrl/pinctrl-xway.c > > index ae724bd..234d3f4 100644 > > --- a/drivers/pinctrl/pinctrl-xway.c > > +++ b/drivers/pinctrl/pinctrl-xway.c > > @@ -7,6 +7,7 @@ > > * publishhed by the Free Software Foundation. > > * > > * Copyright (C) 2012 John Crispin <blogic@xxxxxxxxxxx> > > + * Copyright (C) 2015 Martin Schiller <mschiller@xxxxxx> > > */ > > > > #include <linux/err.h> > > @@ -24,7 +25,7 @@ > > > > #include <lantiq_soc.h> > > > > -/* we have 3 1/2 banks of 16 bit each */ > > +/* we have up to 4 banks of 16 bit each */ > > #define PINS16 > > #define PORT33 > > #define PORT(x)(x / PINS) > > @@ -35,7 +36,7 @@ > > #define MUX_ALT10x2 > > > > /* > > - * each bank has this offset apart from the 1/2 bank that is mixed > into the > > + * each bank has this offset apart from the 4th bank that is mixed > into the > > * other 3 ranges > > */ > > #define REG_OFF0x30 > > @@ -51,7 +52,7 @@ > > #define GPIO_PUDSEL(p)(GPIO_BASE(p) + 0x1c) > > #define GPIO_PUDEN(p)(GPIO_BASE(p) + 0x20) > > > > -/* the 1/2 port needs special offsets for some registers */ > > +/* the 4th port needs special offsets for some registers */ > > #define GPIO3_OD(GPIO_BASE(0) + 0x24) > > #define GPIO3_PUDSEL(GPIO_BASE(0) + 0x28) > > #define GPIO3_PUDEN(GPIO_BASE(0) + 0x2C) > > @@ -80,17 +81,18 @@ > > #define FUNC_MUX(f, m)\ > > { .func = f, .mux = XWAY_MUX_##m, } > > (snip) > > /* --------- pinconf related code --------- */ > > static int xway_pinconf_get(struct pinctrl_dev *pctldev, > > unsigned pin, > > @@ -695,9 +1593,6 @@ static struct gpio_chip xway_chip = { > > > > > > /* --------- register the pinctrl layer --------- */ > > -static const unsigned xway_exin_pin_map[] = {GPIO0, GPIO1, GPIO2, > GPIO39, GPIO46, GPIO9}; > > -static const unsigned ase_exin_pins_map[] = {GPIO6, GPIO29, GPIO0}; > > - > > static struct pinctrl_xway_soc { > > int pin_count; > > const struct ltq_mfp_pin *mfp; > > @@ -708,21 +1603,41 @@ static struct pinctrl_xway_soc { > > const unsigned *exin; > > unsigned int num_exin; > > } soc_cfg[] = { > > -/* legacy xway */ > > +/* legacy xway (DEPRECATED: Use XWAY DANUBE Family) */ > > {XWAY_MAX_PIN, xway_mfp, > > xway_grps, ARRAY_SIZE(xway_grps), > > -danube_funcs, ARRAY_SIZE(danube_funcs), > > +legacy_danube_funcs, ARRAY_SIZE(legacy_danube_funcs), > > xway_exin_pin_map, 3}, > > What is the difference between this entry and the new danube entry? Is > this legay entry needed here our can we map that to the danube entry? > If > it is not needed you should probably change it directly in the > xway_match table. Yes, you are right. We could simply match the old "lantiq,pinctrl-xway" compatible string to the new danube entry, when we also add the old "spi" mux group (DEPRECATED in the ase code) here too. > > > -/* xway xr9 series */ > > +/* xway xr9 series (DEPRECATED: Use XWAY xRX100/xRX200 Family) */ > > {XR9_MAX_PIN, xway_mfp, > > xway_grps, ARRAY_SIZE(xway_grps), > > xrx_funcs, ARRAY_SIZE(xrx_funcs), > > xway_exin_pin_map, 6}, > > -/* xway ase series */ > > -{XWAY_MAX_PIN, ase_mfp, > > +/* XWAY AMAZON Family */ > > +{ASE_MAX_PIN, ase_mfp, > > ase_grps, ARRAY_SIZE(ase_grps), > > ase_funcs, ARRAY_SIZE(ase_funcs), > > -ase_exin_pins_map, 3}, > > +ase_exin_pin_map, 3}, > > +/* XWAY DANUBE Family */ > > +{DANUBE_MAX_PIN, danube_mfp, > > +danube_grps, ARRAY_SIZE(danube_grps), > > +danube_funcs, ARRAY_SIZE(danube_funcs), > > +danube_exin_pin_map, 3}, > > +/* XWAY xRX100 Family */ > > +{XRX100_MAX_PIN, xrx100_mfp, > > +xrx100_grps, ARRAY_SIZE(xrx100_grps), > > +xrx100_funcs, ARRAY_SIZE(xrx100_funcs), > > +xrx100_exin_pin_map, 6}, > > +/* XWAY xRX200 Family */ > > +{XRX200_MAX_PIN, xrx200_mfp, > > +xrx200_grps, ARRAY_SIZE(xrx200_grps), > > +xrx200_funcs, ARRAY_SIZE(xrx200_funcs), > > +xrx200_exin_pin_map, 6}, > > +/* XWAY xRX300 Family */ > > +{XRX300_MAX_PIN, xrx300_mfp, > > +xrx300_grps, ARRAY_SIZE(xrx300_grps), > > +xrx300_funcs, ARRAY_SIZE(xrx300_funcs), > > +xrx300_exin_pin_map, 5}, > > }; > > > > static struct pinctrl_gpio_range xway_gpio_range = { > > @@ -734,6 +1649,10 @@ static const struct of_device_id xway_match[] = > { > > { .compatible = "lantiq,pinctrl-xway", .data = &soc_cfg[0]}, > > { .compatible = "lantiq,pinctrl-xr9", .data = &soc_cfg[1]}, > > { .compatible = "lantiq,pinctrl-ase", .data = &soc_cfg[2]}, > > +{ .compatible = "lantiq,pinctrl-danube", .data = &soc_cfg[3]}, > > +{ .compatible = "lantiq,pinctrl-xrx100", .data = &soc_cfg[4]}, > > +{ .compatible = "lantiq,pinctrl-xrx200", .data = &soc_cfg[5]}, > > +{ .compatible = "lantiq,pinctrl-xrx300", .data = &soc_cfg[6]}, > > Using pointers to an array elemnt here looks error prone to me. Why not > use one static entry for each SoC and reference it directly. > > Something like this: > > static struct pinctrl_xway_soc pinctrl_xrx300 = { > XRX300_MAX_PIN, xrx300_mfp, > xrx300_grps, ARRAY_SIZE(xrx300_grps), > xrx300_funcs, ARRAY_SIZE(xrx300_funcs), > xrx300_exin_pin_map, 5 > }; > > Then you can just do this: > { .compatible = "lantiq,pinctrl-xrx300", .data = &pinctrl_xrx300}, > Good point, I will change that. Martin ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f