Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm
From: Thierry Reding @ 2016-12-05 11:30 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Mark Rutland, devicetree, Lars-Peter Clausen, alexandre.torgue,
	linux-pwm, linux-iio, Linus Walleij, Arnaud Pouliquen,
	Linux Kernel Mailing List, robh+dt, linux-arm-kernel,
	Peter Meerwald-Stadler, knaack.h, Gerald Baeza, Fabrice Gasnier,
	Lee Jones, Linaro Kernel Mailman List, Jonathan Cameron,
	Benjamin Gaignard
In-Reply-To: <CA+M3ks7_utdw1c0A0m+RzuPTfvFAv5d8HoQvQDzXwHR8M6RKJA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 4037 bytes --]

On Mon, Dec 05, 2016 at 12:02:45PM +0100, Benjamin Gaignard wrote:
> 2016-12-05 8:23 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Fri, Dec 02, 2016 at 11:17:19AM +0100, Benjamin Gaignard wrote:
> >> This driver add support for pwm driver on stm32 platform.
> >
> > "adds". Also please use PWM in prose because it's an abbreviation.
> >
> >> The SoC have multiple instances of the hardware IP and each
> >> of them could have small differences: number of channels,
> >> complementary output, counter register size...
> >> Use DT parameters to handle those differentes configuration
> >
> > "different configurations"
> 
> ok
> 
> >
> >>
> >> version 2:
> >> - only keep one comptatible
> >> - use DT paramaters to discover hardware block configuration
> >
> > "parameters"
> 
> ok
> 
> >
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>  drivers/pwm/Kconfig     |   8 ++
> >>  drivers/pwm/Makefile    |   1 +
> >>  drivers/pwm/pwm-stm32.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 294 insertions(+)
> >>  create mode 100644 drivers/pwm/pwm-stm32.c
> >>
> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> >> index bf01288..a89fdba 100644
> >> --- a/drivers/pwm/Kconfig
> >> +++ b/drivers/pwm/Kconfig
> >> @@ -388,6 +388,14 @@ config PWM_STI
> >>         To compile this driver as a module, choose M here: the module
> >>         will be called pwm-sti.
> >>
> >> +config PWM_STM32
> >> +     bool "STMicroelectronics STM32 PWM"
> >> +     depends on ARCH_STM32
> >> +     depends on OF
> >> +     select MFD_STM32_GP_TIMER
> >
> > Should that be a "depends on"?
> 
> I think select is fine because the wanted feature is PWM not the mfd part

I think in that case you may want to hide the MFD Kconfig option. See
Documentation/kbuild/kconfig-language.txt (notes about select) for the
details.

[...]
> >> +};
> >> +
> >> +#define to_stm32_pwm_dev(x) container_of(chip, struct stm32_pwm_dev, chip)
> >
> > Please turn this into a static inline.
> 
> with putting struct pwm_chip as first filed I will just cast the structure

Don't do that, please. container_of() is still preferred because it is
correct and won't break even if you (or somebody else) changes the order
in the future. The fact that it might be optimized away is a detail, or
a micro-optimization. Force-casting is a bad idea because it won't catch
errors if for some reason the field doesn't remain in the first position
forever.

> >> +static void stm32_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >> +{
> >> +     u32 mask;
> >> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
> >> +
> >> +     /* Disable channel */
> >> +     mask = TIM_CCER_CC1E << (pwm->hwpwm * 4);
> >> +     if (dev->caps & CAP_COMPLEMENTARY)
> >> +             mask |= TIM_CCER_CC1NE << (pwm->hwpwm * 4);
> >> +
> >> +     regmap_update_bits(dev->regmap, TIM_CCER, mask, 0);
> >> +
> >> +     /* When all channels are disabled, we can disable the controller */
> >> +     if (!__active_channels(dev))
> >> +             regmap_update_bits(dev->regmap, TIM_CR1, TIM_CR1_CEN, 0);
> >> +
> >> +     clk_disable(dev->clk);
> >> +}
> >
> > All of the above can be folded into the ->apply() hook for atomic PWM
> > support.
> >
> > Also, in the above you use clk_enable() in the ->enable() callback and
> > clk_disable() in ->disable(). If you need the clock to access registers
> > you'll have to enabled it in the others as well because there are no
> > guarantees that configuration will only happen between ->enable() and
> > ->disable(). Atomic PWM simplifies this a bit, but you still need to be
> > careful about when to enable the clock in your ->apply() hook.
> 
> I have used regmap functions enable/disable clk for me when accessing to
> the registers.
> I only have to take care of clk enable/disable when PWM state change.

Okay, that's fine then.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/3] ARM: dts: imx6: Add Savageboard common file
From: Fabio Estevam @ 2016-12-05 11:36 UTC (permalink / raw)
  To: Milo Kim
  Cc: Shawn Guo, Sascha Hauer, Fabio Estevam,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel
In-Reply-To: <20161205010729.7047-2-woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Sun, Dec 4, 2016 at 11:07 PM, Milo Kim <woogyom.kim-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> +       regulators {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               reg_3p3v: regulator@0 {
> +                       compatible = "regulator-fixed";
> +                       reg = <0>;
> +                       regulator-name = "3P3V";
> +                       regulator-min-microvolt = <3300000>;
> +                       regulator-max-microvolt = <3300000>;
> +                       regulator-always-on;
> +               };

Please remove the regulators container and put the regulator node
directly as follows:

reg_3p3v: regulator-3p3v {
   compatible = "regulator-fixed";
   regulator-name = "3P3V";
   regulator-min-microvolt = <3300000>;
   regulator-max-microvolt = <3300000>;
   regulator-always-on;
}

> +       };
> +};
> +
> +&clks {
> +       assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> +                         <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> +       assigned-clock-parents = <&clks IMX6QDL_CLK_PLL3_USB_OTG>,
> +                                <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> +};
> +
> +&fec {
> +       phy-mode = "rgmii";
> +       phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_HIGH>;

I think you meant
phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;

> +&iomuxc {
> +       savageboard {
> +               pinctrl_emmc: emmcgrp {
> +                       fsl,pins = <
> +                               MX6QDL_PAD_SD4_CMD__SD4_CMD             0x17059
> +                               MX6QDL_PAD_SD4_CLK__SD4_CLK             0x10059
> +                               MX6QDL_PAD_SD4_DAT0__SD4_DATA0          0x17059
> +                               MX6QDL_PAD_SD4_DAT1__SD4_DATA1          0x17059
> +                               MX6QDL_PAD_SD4_DAT2__SD4_DATA2          0x17059
> +                               MX6QDL_PAD_SD4_DAT3__SD4_DATA3          0x17059
> +                               MX6QDL_PAD_SD4_DAT4__SD4_DATA4          0x17059
> +                               MX6QDL_PAD_SD4_DAT5__SD4_DATA5          0x17059
> +                               MX6QDL_PAD_SD4_DAT6__SD4_DATA6          0x17059
> +                               MX6QDL_PAD_SD4_DAT7__SD4_DATA7          0x17059
> +                       >;
> +               };

You can remove the savegeboard level. Please check
arch/arm/boot/dts/imx6q-tbs2910.dts.

iomux usually go as the last node of the dts file.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Mark Brown @ 2016-12-05 12:05 UTC (permalink / raw)
  To: Romain Perier
  Cc: Mark Rutland, Andrew Lunn, Jason Cooper, Pawel Moll, devicetree,
	Ian Campbell, linux-spi, Nadav Haklai, Rob Herring, Kumar Gala,
	Gregory Clement, xigu, dingwei, Thomas Petazzoni,
	linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161201102719.4291-2-romain.perier@free-electrons.com>


[-- Attachment #1.1: Type: text/plain, Size: 1933 bytes --]

On Thu, Dec 01, 2016 at 11:27:15AM +0100, Romain Perier wrote:

> +config SPI_ARMADA_3700
> +	tristate "Marvell Armada 3700 SPI Controller"
> +	depends on ARCH_MVEBU && OF

Why does this not have a COMPILE_TEST dependency?

> +	/* Reset SPI unit */
> +	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> +	val |= A3700_SPI_SRST;
> +	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> +
> +	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
> +		udelay(1);

Why not just do a single udelay()?

> +static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
> +{
> +	struct spi_master *master = dev_id;
> +	struct a3700_spi *a3700_spi;
> +	u32 cause;
> +
> +	a3700_spi = spi_master_get_devdata(master);
> +
> +	/* Get interrupt causes */
> +	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
> +
> +	/* mask and acknowledge the SPI interrupts */
> +	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
> +	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
> +
> +	/* Wake up the transfer */
> +	if (a3700_spi->wait_mask & cause)
> +		complete(&a3700_spi->done);
> +
> +	return IRQ_HANDLED;
> +}

This reports that we handled an interrupt even if we did not in fact
handle an interrupt.  It's also a bit dodgy that it doesn't check what
the interrupt was but that's less serious.

> +	master->bus_num = (pdev->id != -1) ? pdev->id : 0;

At most this should be just setting the bus number to pdev->id like
other drivers do.

> +	ret = clk_prepare_enable(spi->clk);
> +	if (ret) {
> +		dev_err(dev, "could not prepare clk: %d\n", ret);
> +		goto error;
> +	}

I'd expect the hardware prepare/unprepare to be managing the enable and
disable of the clock in order to save a little power.

> +	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
> +		 (unsigned long)res->start, spi->irq);

This is just adding noise to the boot, remove it.  It's not telling us
anything we read from the hardware or anything.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
From: Benjamin Gaignard @ 2016-12-05 12:12 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mark Rutland, devicetree, Lars-Peter Clausen, alexandre.torgue,
	linux-pwm, linux-iio, Linus Walleij, Arnaud Pouliquen,
	Linux Kernel Mailing List, robh+dt, linux-arm-kernel,
	Peter Meerwald-Stadler, knaack.h, Gerald Baeza, Fabrice Gasnier,
	Lee Jones, Linaro Kernel Mailman List, Jonathan Cameron,
	Benjamin Gaignard
In-Reply-To: <20161205112347.GF19891@ulmo.ba.sec>

2016-12-05 12:23 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> On Mon, Dec 05, 2016 at 12:08:32PM +0100, Benjamin Gaignard wrote:
>> 2016-12-05 7:53 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
>> > On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
>> >> Define bindings for pwm-stm32
>> >>
>> >> version 2:
>> >> - use parameters instead of compatible of handle the hardware configuration
>> >>
>> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> >> ---
>> >>  .../devicetree/bindings/pwm/pwm-stm32.txt          | 38 ++++++++++++++++++++++
>> >>  1 file changed, 38 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> >> new file mode 100644
>> >> index 0000000..575b9fb
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> >> @@ -0,0 +1,38 @@
>> >> +STMicroelectronics PWM driver bindings for STM32
>> >
>> > Technically this bindings describe devices, so "driver binding" is a
>> > somewhat odd wording. Perhaps:
>> >
>> >         STMicroelectronics STM32 General Purpose Timer PWM bindings
>> >
>> > ?
>>  done
>>
>> >
>> >> +
>> >> +Must be a sub-node of STM32 general purpose timer driver
>> >> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
>> >
>> > Again, "driver parent node" is odd. Perhaps:
>> >
>> >         Must be a sub-node of an STM32 General Purpose Timer device tree
>> >         node. See ../mfd/stm32-general-purpose-timer.txt for details about
>> >         the parent node.
>> >
>> > ?
>>
>> done
>>
>> >
>> >> +Required parameters:
>> >> +- compatible:                Must be "st,stm32-pwm"
>> >> +- pinctrl-names:     Set to "default".
>> >> +- pinctrl-0:                 List of phandles pointing to pin configuration nodes
>> >> +                     for PWM module.
>> >> +                     For Pinctrl properties, please refer to [1].
>> >
>> > Your indentation and capitalization are inconsistent. Also, please refer
>> > to the pinctrl bindings by relative path and inline, rather than as a
>> > footnote reference.
>>
>> OK
>>
>> >
>> >> +
>> >> +Optional parameters:
>> >> +- st,breakinput:     Set if the hardware have break input capabilities
>> >> +- st,breakinput-polarity: Set break input polarity. Default is 0
>> >> +                      The value define the active polarity:
>> >> +                       - 0 (active LOW)
>> >> +                       - 1 (active HIGH)
>> >
>> > Could we fold these into a single property? If st,breakinput-polarity is
>> > not present it could simply mean that there is no break input, and if it
>> > is present you don't have to rely on a default.
>>
>> I need to know if I have to activate breakinput feature and on which level
>> I will rewrite it like that:
>> Optional parameters:
>> - st,breakinput-polarity-high: Set if break input polarity is active
>> on high level.
>> - st,breakinput-polarity-high: Set if break input polarity is active
>> on low level.
>
> How is that different from a single property:
>
>         Optional properties:
>         - st,breakinput-polarity: If present, a break input is available
>             for the channel. In that case the property value denotes the
>             polarity of the break input:
>             - 0: active low
>             - 1: active high
>
> ?

For break input feature I need two information: do I have to active it
and if activated
on which level.
I have two solutions:
- one parameter with a value (0 or 1) -> st,breakinput-polarity
- two parameters without value -> st,breakinput-active-high and
st,breakinput-active-low

By default break input feature is disabled

>
>> > The pwm- prefix is rather redundant since the node is already named pwm.
>> > Why not simply st,channels? Or simply channels, since it's not really
>> > anything specific to this hardware.
>> >
>> > Come to think of it, might be worth having a discussion with our DT
>> > gurus about what their stance is on using the # as prefix for numbers
>> > (such as in #address-cells or #size-cells). This could be #channels to
>> > mark it more explicitly as representing a count.
>> >
>> >> +- st,32bits-counter: Set if the hardware have a 32 bits counter
>> >> +- st,complementary:  Set if the hardware have complementary output channels
>> >
>> > "hardware has" and also maybe mention explicitly that this is a boolean
>> > property. Otherwise people might be left wondering what it should be set
>> > to. Or maybe word this differently to imply that it's boolean:
>> >
>> >         - st,32bits-counter: if present, the hardware has a 32 bit counter
>> >         - st,complementary: if present, the hardware has a complementary
>> >                             output channel
>>
>> I found a way to detect, at probe time, the number of channels, counter size,
>> break input capability and complementary channels so I will remove
>> "st,breakinput", "st,32bits-counter", "st,complementary" and "st,pwm-num-chan"
>> parameters
>
> Oh hey, that's very neat. I suppose in that case my comment above about
> the break input polarity is somewhat obsoleted. Still I think you won't
> need two properties. Instead you can follow what other similar
> properties have done: choose a default (often low-active) and have a
> single optional property to override the default (often high-active).
>
> In your case:
>
>         - st,breakinput-active-high: Some channels have a break input,
>             whose polarity will be active low by default. If this
>             property is present, the channel will be configured with an
>             active high polarity for the break input.
>
> Thierry



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
From: Thierry Reding @ 2016-12-05 12:21 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Lee Jones, robh+dt, Mark Rutland, alexandre.torgue, devicetree,
	Linux Kernel Mailing List, linux-pwm, Jonathan Cameron, knaack.h,
	Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
	linux-arm-kernel, Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
	Linus Walleij, Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks5j2e-EQEf74YSduboNnCiWC7+e_PD5aAz5XdFA1tGztQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4599 bytes --]

On Mon, Dec 05, 2016 at 01:12:25PM +0100, Benjamin Gaignard wrote:
> 2016-12-05 12:23 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Mon, Dec 05, 2016 at 12:08:32PM +0100, Benjamin Gaignard wrote:
> >> 2016-12-05 7:53 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> >> > On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
> >> >> Define bindings for pwm-stm32
> >> >>
> >> >> version 2:
> >> >> - use parameters instead of compatible of handle the hardware configuration
> >> >>
> >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> >> ---
> >> >>  .../devicetree/bindings/pwm/pwm-stm32.txt          | 38 ++++++++++++++++++++++
> >> >>  1 file changed, 38 insertions(+)
> >> >>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> >> new file mode 100644
> >> >> index 0000000..575b9fb
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> >> @@ -0,0 +1,38 @@
> >> >> +STMicroelectronics PWM driver bindings for STM32
> >> >
> >> > Technically this bindings describe devices, so "driver binding" is a
> >> > somewhat odd wording. Perhaps:
> >> >
> >> >         STMicroelectronics STM32 General Purpose Timer PWM bindings
> >> >
> >> > ?
> >>  done
> >>
> >> >
> >> >> +
> >> >> +Must be a sub-node of STM32 general purpose timer driver
> >> >> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
> >> >
> >> > Again, "driver parent node" is odd. Perhaps:
> >> >
> >> >         Must be a sub-node of an STM32 General Purpose Timer device tree
> >> >         node. See ../mfd/stm32-general-purpose-timer.txt for details about
> >> >         the parent node.
> >> >
> >> > ?
> >>
> >> done
> >>
> >> >
> >> >> +Required parameters:
> >> >> +- compatible:                Must be "st,stm32-pwm"
> >> >> +- pinctrl-names:     Set to "default".
> >> >> +- pinctrl-0:                 List of phandles pointing to pin configuration nodes
> >> >> +                     for PWM module.
> >> >> +                     For Pinctrl properties, please refer to [1].
> >> >
> >> > Your indentation and capitalization are inconsistent. Also, please refer
> >> > to the pinctrl bindings by relative path and inline, rather than as a
> >> > footnote reference.
> >>
> >> OK
> >>
> >> >
> >> >> +
> >> >> +Optional parameters:
> >> >> +- st,breakinput:     Set if the hardware have break input capabilities
> >> >> +- st,breakinput-polarity: Set break input polarity. Default is 0
> >> >> +                      The value define the active polarity:
> >> >> +                       - 0 (active LOW)
> >> >> +                       - 1 (active HIGH)
> >> >
> >> > Could we fold these into a single property? If st,breakinput-polarity is
> >> > not present it could simply mean that there is no break input, and if it
> >> > is present you don't have to rely on a default.
> >>
> >> I need to know if I have to activate breakinput feature and on which level
> >> I will rewrite it like that:
> >> Optional parameters:
> >> - st,breakinput-polarity-high: Set if break input polarity is active
> >> on high level.
> >> - st,breakinput-polarity-high: Set if break input polarity is active
> >> on low level.
> >
> > How is that different from a single property:
> >
> >         Optional properties:
> >         - st,breakinput-polarity: If present, a break input is available
> >             for the channel. In that case the property value denotes the
> >             polarity of the break input:
> >             - 0: active low
> >             - 1: active high
> >
> > ?
> 
> For break input feature I need two information: do I have to active it
> and if activated
> on which level.
> I have two solutions:
> - one parameter with a value (0 or 1) -> st,breakinput-polarity
> - two parameters without value -> st,breakinput-active-high and
> st,breakinput-active-low
> 
> By default break input feature is disabled

Right, what I was suggesting is that you use the first solution because
it's the typical way to use for tristate options. It's also much easier
to test for in practice because for the second solution you have to
parse two properties before you know whether it is active or not.

The second is typically the solution for required properties where only
one of the properties is used to override the default.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Hans Verkuil @ 2016-12-05 12:27 UTC (permalink / raw)
  To: Laurent Pinchart, Sakari Ailus
  Cc: Kevin Hilman, linux-media, devicetree, Sekhar Nori, Axel Haslam,
	Bartosz Gołaszewski, Alexandre Bailon, David Lechner
In-Reply-To: <2453889.B9pO7dWgEo@avalon>

On 12/01/2016 10:16 AM, Laurent Pinchart wrote:
> Hello,
> 
> On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
>> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
>>> Sakari Ailus <sakari.ailus@iki.fi> writes:
>>>> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>>>>> Sakari Ailus <sakari.ailus@iki.fi> writes:
>>>>>> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>>>>>>> Allow getting of subdevs from DT ports and endpoints.
>>>>>>>
>>>>>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>>>>>>> am437x-vpfe.c
>>>>>>>
>>>>>>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++-
>>>>>>>  include/media/davinci/vpif_types.h     
>>>>>>>        |   9 +-
>>>>>>>  2 files changed, 133 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>> b/drivers/media/platform/davinci/vpif_capture.c index
>>>>>>> 94ee6cf03f02..47a4699157e7 100644
>>>>>>> --- a/drivers/media/platform/davinci/vpif_capture.c
>>>>>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>>>>>>> @@ -26,6 +26,8 @@
>>>>>>>  #include <linux/slab.h>
>>>>>>>
>>>>>>>  #include <media/v4l2-ioctl.h>
>>>>>>> +#include <media/v4l2-of.h>
>>>>>>> +#include <media/i2c/tvp514x.h>
>>>>>>
>>>>>> Do you need this header?
>>>>>
>>>>> Yes, based on discussion with Hans, since there is no DT binding for
>>>>> selecting the input pins of the TVP514x, I have to select it in the
>>>>> driver, so I need the defines from this header.  More on this below...
> 
> That's really ugly :-( The problem should be fixed properly instead of adding 
> one more offender.

Do you have time for that, Laurent? I don't. Until that time we just need to
make do with this workaround.

> 
>>>>>>>  #include "vpif.h"
>>>>>>>  #include "vpif_capture.h"
>>>>>>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>>>>>>>
>>>>>>>  	vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>>>>>>
>>>>>>> +	if (!chan_cfg)
>>>>>>> +		return -1;
>>>>>>> +	if (input_index >= chan_cfg->input_count)
>>>>>>> +		return -1;
>>>>>>>  	subdev_name = chan_cfg->inputs[input_index].subdev_name;
>>>>>>>  	if (subdev_name == NULL)
>>>>>>>  		return -1;
>>>>>>> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
>>>>>>>  	/* loop through the sub device list to get the sub device info
>>>>>>>  	*/
>>>>>>>  	for (i = 0; i < vpif_cfg->subdev_count; i++) {
>>>>>>>  		subdev_info = &vpif_cfg->subdev_info[i];
>>>>>>> -		if (!strcmp(subdev_info->name, subdev_name))
>>>>>>> +		if (subdev_info && !strcmp(subdev_info->name,
>>>>>>> subdev_name))
>>>>>>>  			return i;
>>>>>>>  	}
>>>>>>>  	return -1;
>>>>>>> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct
>>>>>>> v4l2_async_notifier *notifier,> >> >> 
>>>>>>>  {
>>>>>>>  	int i;
>>>>>>>
>>>>>>> +	for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>>>>>>> +		struct v4l2_async_subdev *_asd = vpif_obj.config
>>>>>>> ->asd[i];
>>>>>>> +		const struct device_node *node = _asd->match.of.node;
>>>>>>> +
>>>>>>> +		if (node == subdev->of_node) {
>>>>>>> +			vpif_obj.sd[i] = subdev;
>>>>>>> +			vpif_obj.config->chan_config
>>>>>>> ->inputs[i].subdev_name =
>>>>>>> +				(char *)subdev->of_node->full_name;
> 
> Can subdev_name be made const instead of blindly casting the full_name pointer 
> ? If not this is probably unsafe, and if yes it should be done :-)
> 
>>>>>>> +			vpif_dbg(2, debug,
>>>>>>> +				 "%s: setting input %d subdev_name =
>>>>>>> %s\n",
>>>>>>> +				 __func__, i, subdev->of_node
>>>>>>> ->full_name);
>>>>>>> +			return 0;
>>>>>>> +		}
>>>>>>> +	}
>>>>>>> +
>>>>>>>  	for (i = 0; i < vpif_obj.config->subdev_count; i++)
>>>>>>>  		if (!strcmp(vpif_obj.config->subdev_info[i].name,
>>>>>>>  			    subdev->name)) {
>>>>>>> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct
>>>>>>> v4l2_async_notifier *notifier)
>>>>>>>  	return vpif_probe_complete();
>>>>>>>  }
>>>>>>>
>>>>>>> +static struct vpif_capture_config *
>>>>>>> +vpif_capture_get_pdata(struct platform_device *pdev)
>>>>>>> +{
>>>>>>> +	struct device_node *endpoint = NULL;
>>>>>>> +	struct v4l2_of_endpoint bus_cfg;
>>>>>>> +	struct vpif_capture_config *pdata;
>>>>>>> +	struct vpif_subdev_info *sdinfo;
>>>>>>> +	struct vpif_capture_chan_config *chan;
>>>>>>> +	unsigned int i;
>>>>>>> +
>>>>>>> +	dev_dbg(&pdev->dev, "vpif_get_pdata\n");
>>>>>>> +
>>>>>>> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>>>>>>> +		return pdev->dev.platform_data;
>>>>>>> +
>>>>>>> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>>>>>>> +	if (!pdata)
>>>>>>> +		return NULL;
>>>>>>> +	pdata->subdev_info =
>>>>>>> +		devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
>>>>>>> +			     VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
>>>>>>> +
>>>>>>> +	if (!pdata->subdev_info)
>>>>>>> +		return NULL;
>>>>>>> +	dev_dbg(&pdev->dev, "%s\n", __func__);
>>>>>>> +
>>>>>>> +	for (i = 0; ; i++) {
>>>>>>> +		struct device_node *rem;
>>>>>>> +		unsigned int flags;
>>>>>>> +		int err;
>>>>>>> +
>>>>>>> +		endpoint = of_graph_get_next_endpoint(pdev
>>>>>>> ->dev.of_node,
>>>>>>> +						      endpoint);
>>>>>>> +		if (!endpoint)
>>>>>>> +			break;
>>>>>>> +
>>>>>>> +		sdinfo = &pdata->subdev_info[i];
>>>>>>
>>>>>> subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
>>>>>
>>>>> Right, I need to make the loop only go for a max of
>>>>> VPIF_CAPTURE_MAX_CHANNELS iterations.
>>>>>
>>>>>>> +		chan = &pdata->chan_config[i];
>>>>>>> +		chan->inputs = devm_kzalloc(&pdev->dev,
>>>>>>> +					    sizeof(*chan->inputs) *
>>>>>>> +					    VPIF_DISPLAY_MAX_CHANNELS,
>>>>>>> +					    GFP_KERNEL);
>>>>>>> +
>>>>>>> +		chan->input_count++;
>>>>>>> +		chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
>>>>>>
>>>>>> I wonder what's the purpose of using index i on this array as well.
>>>>>
>>>>> The number of endpoints in DT is the number of input channels
>>>>> configured (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
>>>>>
>>>>>> If you use that to access a corresponding entry in a different array,
>>>>>> I'd just create a struct that contains the port configuration and the
>>>>>> async sub-device. The omap3isp driver does that, for instance; see
>>>>>> isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if
>>>>>> you're interested. Up to you.
>>>>>
>>>>> OK, I'll have a look at that driver. The goal here with this series is
>>>>> just to get this working with DT, but also not break the existing
>>>>> legacy platform_device support, so I'm trying not to mess with the
>>>>> driver-interal data structures too much.
>>>>
>>>> Ack.
>>>>
>>>>>>> +		chan->inputs[i].input.std = V4L2_STD_ALL;
>>>>>>> +		chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
>>>>>>> +
>>>>>>> +		/* FIXME: need a new property? ch0:composite ch1:
>>>>>>> s-video */
>>>>>>> +		if (i == 0)
>>>>>>
>>>>>> Can you assume that the first endopoint has got a particular kind of
>>>>>> input? What if it's not connected?
>>>>>
>>>>> On all the boards I know of (there aren't many using this SoC), it's a
>>>>> safe assumption.
>>>>>
>>>>>> If this is a different physical port (not in the meaning another) in
>>>>>> the device, I'd use the reg property for this. Please see
>>>>>> Documentation/devicetree/bindings/media/video-interfaces.txt .
>>>>>
>>>>> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
>>>>> that it's not physically a different port.  Instead, it's just telling
>>>>> the TVP514x which pin(s) will be active inputs (and what kind of signal
>>>>> will be present.)
>>>>>
>>>>> I'm open to a better way to describe this input select from DT, but
>>>>> based on what I heard from Hans, there isn't currently a good way to do
>>>>> that except for in the driver:
>>>>> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
>>>>>
>>>>> Based on further discussion in that thread, it sounds like there may be
>>>>> a way forward coming soon, and I'll be glad to switch to that when it
>>>>> arrives.
> 
> I'm afraid I have to disappoint Hans here, I don't have code for that yet.
> 
>>>> I'm not sure that properly supporting connectors will provide any help
>>>> here.
>>>>
>>>> Looking at the s_routing() API, it's the calling driver that has to be
>>>> aware of sub-device specific function parameters. As such it's not a
>>>> very good idea to require that a driver is aware of the value range of
>>>> another driver's parameter. I wonder if a simple enumeration interface
>>>> would help here --- if I understand correctly, the purpose is just to
>>>> provide a way to choose the input using VIDIOC_S_INPUT.
>>>>
>>>> I guess that's somehow ok as long as you have no other combinations of
>>>> these devices but this is hardly future-proof. (And certainly not a
>>>> problem created by this patch.)
>>>
>>> Yeah, this is far from future proof.
>>>
>>>> It'd be still nice to fix that as presumably we don't have the option of
>>>> reworking how we expect the device tree to look like.
>>>
>>> Agreed.
>>>
>>> I'm just hoping someone can shed som light on "how we expect the device
>>> tree to look".  ;)
>>
>> :-)
>>
>> For the tvp514x, do you need more than a single endpoint on the receiver
>> side? Does the input that's selected affect the bus parameters?
>>
>> If it doesn't, you could create a custom endpoint property for the possible
>> input values. The s_routing() really should be fixed though, but that could
>> be postponed I guess. There are quite a few drivers using it.
> 
> There's two ways to look at s_routing() in my opinion, as the calling driver 
> should really not hardcode any knowledge specific to a particular subdev. We 
> can either have the calling driver discover the possible routing options at 
> runtime through the subdev API, or modify the s_routing() API.
> 

Some historical perspective: s_routing was added well before the device tree
was ever used for ARM. And at that time the vast majority of drivers were PCI
or USB drivers, very few platform drivers existed (and those typically used
sensors, not video receivers).

Before s_routing existed the situation was even worse.

Basically what s_routing does is a poor-man's device tree entry, telling the
subdev how to route video or audio from connector to the output of the chip.
Typically the card tables in PCI or USB drivers contain the correct arguments
for s_routing. Of course, today we'd do that with the DT, but that was not an
option years ago.

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH v3 1/2] ARM: dts: da850-lcdk: add the dumb-vga-dac node
From: Tomi Valkeinen @ 2016-12-05 12:49 UTC (permalink / raw)
  To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Sekhar Nori,
	Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi,
	Russell King
  Cc: linux-devicetree, LKML, linux-drm, Jyri Sarha, arm-soc,
	Laurent Pinchart
In-Reply-To: <1480420624-23544-2-git-send-email-bgolaszewski@baylibre.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 3213 bytes --]

On 29/11/16 13:57, Bartosz Golaszewski wrote:
> Add the dumb-vga-dac node to the board DT together with corresponding
> ports and vga connector. This allows to retrieve the edid info from
> the display automatically.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/da850.dtsi     | 17 ++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index 711b9ad..d864f11 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -50,6 +50,53 @@
>  			system-clock-frequency = <24576000>;
>  		};
>  	};
> +
> +	vga_bridge {
> +		compatible = "dumb-vga-dac";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&lcd_pins>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <0>;
> +
> +				vga_bridge_in: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&display_out_vga>;
> +				};
> +			};
> +
> +			port@1 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				reg = <1>;
> +
> +				vga_bridge_out: endpoint@0 {
> +					reg = <0>;
> +					remote-endpoint = <&vga_con_in>;
> +				};
> +			};
> +		};
> +	};
> +
> +	vga {
> +		compatible = "vga-connector";
> +
> +		ddc-i2c-bus = <&i2c0>;
> +
> +		port {
> +			vga_con_in: endpoint {
> +				remote-endpoint = <&vga_bridge_out>;
> +			};
> +		};
> +	};
>  };
>  
>  &pmx_core {
> @@ -235,3 +282,14 @@
>  &memctrl {
>  	status = "okay";
>  };
> +
> +&display {
> +	status = "okay";
> +};
> +
> +&display_out {
> +	display_out_vga: endpoint@0 {
> +		reg = <0>;
> +		remote-endpoint = <&vga_bridge_in>;
> +	};
> +};
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 4070619..5f4ba2e 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -454,6 +454,23 @@
>  			reg = <0x213000 0x1000>;
>  			interrupts = <52>;
>  			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				display_in: port@0 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <0>;
> +				};
> +
> +				display_out: port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +				};
> +			};
>  		};

It's a bit difficult to follow this as there's been so many patches
going around. But I take the above is the LCDC node in the base da850
dtsi file? In that case, what is the display_in supposed to present?
It's the first node in the "display chain", so it has no input.

Also, don't touch da850.dtsi here, just add the "ports" node in the
da850-lcdk.dts file.

If the da850.dtsi has not been merged yet, I'd change the name of the
lcdc node to something else than "display". It's rather vague. If it's
named "lcdc", reading da850-lcdk.dts becomes much easier, as you'll
refer to "lcdc".

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2] v4l: async: make v4l2 coexist with devicetree nodes in a dt overlay
From: Javier Martinez Canillas @ 2016-12-05 13:13 UTC (permalink / raw)
  To: Javi Merino, Sakari Ailus
  Cc: linux-media, linux-kernel, devicetree, Mauro Carvalho Chehab
In-Reply-To: <1480932596-4108-1-git-send-email-javi.merino@kernel.org>

Hello Javi,

On 12/05/2016 07:09 AM, Javi Merino wrote:
> In asds configured with V4L2_ASYNC_MATCH_OF, the v4l2 subdev can be
> part of a devicetree overlay, for example:
> 
> &media_bridge {
> 	...
> 	my_port: port@0 {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		reg = <0>;
> 		ep: endpoint@0 {
> 			remote-endpoint = <&camera0>;
> 		};
> 	};
> };
> 
> / {
> 	fragment@0 {
> 		target = <&i2c0>;
> 		__overlay__ {
> 			my_cam {
> 				compatible = "foo,bar";
> 				port {
> 					camera0: endpoint {
> 						remote-endpoint = <&my_port>;
> 						...
> 					};
> 				};
> 			};
> 		};
> 	};
> };
> 
> Each time the overlay is applied, its of_node pointer will be
> different.  We are not interested in matching the pointer, what we
> want to match is that the path is the one we are expecting.  Change to
> use of_node_cmp() so that we continue matching after the overlay has
> been removed and reapplied.
> 
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Javi Merino <javi.merino@kernel.org>
> ---

I already reviewed v1 but you didn't carry the tag. So again:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply

* RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation
From: Punnaiah Choudary Kalluri @ 2016-12-05 13:24 UTC (permalink / raw)
  To: Marek Vasut, dwmw2@infradead.org, computersforpeace@gmail.com,
	boris.brezillon@free-electrons.com, richard@nod.at,
	cyrille.pitchen@atmel.com, robh+dt@kernel.org,
	mark.rutland@arm.com
  Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Michal Simek, kalluripunnaiahchoudary@gmail.com, kpc528@gmail.com,
	linux-mtd@lists.infradead.org
In-Reply-To: <74842d92-840d-a7c2-fb1b-ddab1ac2cf42@gmail.com>

Hi Marek,

 Thanks for the review.

> -----Original Message-----
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: Monday, December 05, 2016 9:56 AM
> To: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com;
> boris.brezillon@free-electrons.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > This patch adds the dts binding document for arasan nand flash
> > controller.
> >
> > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > Acked-by: Rob Herring <robh@kernel.org>
> > ---
> > changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller
> > changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> >  .../devicetree/bindings/mtd/arasan_nfc.txt         | 38
> ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > new file mode 100644
> > index 0000000..dcbe7ad
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > @@ -0,0 +1,38 @@
> > +Arasan Nand Flash Controller with ONFI 3.1 support
> 
> Arasan NAND Flash ...
> 
> > +Required properties:
> > +- compatible: Should be "arasan,nfc-v3p10"
> 
> This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> some fallback option which doesn't encode IP version in the compat
> string ?
> 

This is a third-party IP and v3p10 is the version that we are using in our SOC.
Also this IP doesn’t have the IP version information in the register space to
read dynamically and having generic compatible name. So, any new versions
can be added through  of_match_table with config data inside the driver.

> Also, shouldn't quirks be handled by DT props instead of effectively
> encoding them into the compatible string ?
>
I feel the above mentioned method will be more appropriate rather than defining
the quirks through DT properties.

I will fix all other comments

Regards,
Punnaiah
> > +- reg: Memory map for module access
> > +- interrupt-parent: Interrupt controller the interrupt is routed through
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > +	      (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Optional properties:
> > +- arasan,has-mdma: Enables Dma support
> 
> 'Enables DMA support' , with DMA in caps.
> 
> > +for nand partition information please refer the below file
> 
> For NAND ...
> 
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > +	nand0: nand@ff100000 {
> > +		compatible = "arasan,nfc-v3p10"
> > +		reg = <0x0 0xff100000 0x1000>;
> > +		clock-name = "clk_sys", "clk_flash"
> > +		clocks = <&misc_clk &misc_clk>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 14 4>;
> > +		arasan,has-mdma;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>
> > +
> > +		nand@0 {
> > +			reg = <0>
> > +			partition@0 {
> > +				label = "filesystem";
> > +				reg = <0x0 0x0 0x1000000>;
> > +			};
> > +			(...)
> > +		};
> > +	};
> >
> 
> 
> --
> Best regards,
> Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply

* [PATCH v2 0/4] net: hix5hd2_gmac: add tx sg feature and reset/clock control signals
From: Dongpo Li @ 2016-12-05 13:27 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
	yisen.zhuang, salil.mehta, davem, arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li

The "hix5hd2" is SoC name, add the generic ethernet driver compatible string.
The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
the SG/TXCSUM/TSO/UFO features.
This patch set only adds the SG(scatter-gather) driver for transmitting,
the drivers of other features will be submitted later.

Add the MAC reset control signals and clock signals.
We make these signals optional to be backward compatible with
the hix5hd2 SoC.

Changes in v2:
- Make the compatible string changes be a separate patch and
the most specific string come first than the generic string
as advised by Rob.
- Make the MAC reset control signals and clock signals optional
to be backward compatible with the hix5hd2 SoC.
- Change the compatible string and give the clock a specific name
in hix5hd2 dts file.

Dongpo Li (4):
  net: hix5hd2_gmac: add generic compatible string
  net: hix5hd2_gmac: add tx scatter-gather feature
  net: hix5hd2_gmac: add reset control and clock signals
  ARM: dts: hix5hd2: add gmac generic compatible and clock names

 .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  27 +-
 arch/arm/boot/dts/hisi-x5hd2.dtsi                  |   6 +-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 352 +++++++++++++++++++--
 3 files changed, 352 insertions(+), 33 deletions(-)

-- 
2.8.2

^ permalink raw reply

* [PATCH v2 1/4] net: hix5hd2_gmac: add generic compatible string
From: Dongpo Li @ 2016-12-05 13:27 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
	yisen.zhuang, salil.mehta, davem, arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li
In-Reply-To: <1480944481-118803-1-git-send-email-lidongpo@hisilicon.com>

The "hix5hd2" is SoC name, add the generic ethernet driver name.
The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
the SG/TXCSUM/TSO/UFO features.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt    |  9 +++++++--
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c             | 15 +++++++++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index 75d398b..75920f0 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -1,7 +1,12 @@
 Hisilicon hix5hd2 gmac controller
 
 Required properties:
-- compatible: should be "hisilicon,hix5hd2-gmac".
+- compatible: should contain one of the following SoC strings:
+	* "hisilicon,hix5hd2-gemac"
+	* "hisilicon,hi3798cv200-gemac"
+	and one of the following version string:
+	* "hisilicon,hisi-gemac-v1"
+	* "hisilicon,hisi-gemac-v2"
 - reg: specifies base physical address(s) and size of the device registers.
   The first region is the MAC register base and size.
   The second region is external interface control register.
@@ -20,7 +25,7 @@ Required properties:
 
 Example:
 	gmac0: ethernet@f9840000 {
-		compatible = "hisilicon,hix5hd2-gmac";
+		compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
 		interrupts = <0 71 4>;
 		#address-cells = <1>;
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index e69a6be..27cb2e6 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -189,6 +189,10 @@
 #define dma_cnt(n)			((n) >> 5)
 #define dma_byte(n)			((n) << 5)
 
+#define HW_CAP_TSO			BIT(0)
+#define GEMAC_V1			0
+#define GEMAC_V2			(GEMAC_V1 | HW_CAP_TSO)
+
 struct hix5hd2_desc {
 	__le32 buff_addr;
 	__le32 cmd;
@@ -1021,7 +1025,10 @@ static int hix5hd2_dev_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id hix5hd2_of_match[] = {
-	{.compatible = "hisilicon,hix5hd2-gmac",},
+	{ .compatible = "hisilicon,hisi-gemac-v1", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hisi-gemac-v2", .data = (void *)GEMAC_V2 },
+	{ .compatible = "hisilicon,hix5hd2-gemac", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hi3798cv200-gemac", .data = (void *)GEMAC_V2 },
 	{},
 };
 
@@ -1029,7 +1036,7 @@ MODULE_DEVICE_TABLE(of, hix5hd2_of_match);
 
 static struct platform_driver hix5hd2_dev_driver = {
 	.driver = {
-		.name = "hix5hd2-gmac",
+		.name = "hisi-gemac",
 		.of_match_table = hix5hd2_of_match,
 	},
 	.probe = hix5hd2_dev_probe,
@@ -1038,6 +1045,6 @@ static struct platform_driver hix5hd2_dev_driver = {
 
 module_platform_driver(hix5hd2_dev_driver);
 
-MODULE_DESCRIPTION("HISILICON HIX5HD2 Ethernet driver");
+MODULE_DESCRIPTION("HISILICON Gigabit Ethernet MAC driver");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:hix5hd2-gmac");
+MODULE_ALIAS("platform:hisi-gemac");
-- 
2.8.2

^ permalink raw reply related

* [PATCH v2 2/4] net: hix5hd2_gmac: add tx scatter-gather feature
From: Dongpo Li @ 2016-12-05 13:27 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
	yisen.zhuang, salil.mehta, davem, arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li
In-Reply-To: <1480944481-118803-1-git-send-email-lidongpo@hisilicon.com>

"hisi-gemac-v2" adds the SG/TXCSUM/TSO/UFO features.
This patch only adds the SG(scatter-gather) driver for transmitting,
the drivers of other features will be submitted later.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 198 ++++++++++++++++++++++++--
 1 file changed, 187 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 27cb2e6..18af55b 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -11,6 +11,7 @@
 #include <linux/interrupt.h>
 #include <linux/etherdevice.h>
 #include <linux/platform_device.h>
+#include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
 #include <linux/clk.h>
@@ -183,6 +184,8 @@
 #define DESC_DATA_LEN_OFF		16
 #define DESC_BUFF_LEN_OFF		0
 #define DESC_DATA_MASK			0x7ff
+#define DESC_SG				BIT(30)
+#define DESC_FRAGS_NUM_OFF		11
 
 /* DMA descriptor ring helpers */
 #define dma_ring_incr(n, s)		(((n) + 1) & ((s) - 1))
@@ -192,6 +195,7 @@
 #define HW_CAP_TSO			BIT(0)
 #define GEMAC_V1			0
 #define GEMAC_V2			(GEMAC_V1 | HW_CAP_TSO)
+#define HAS_CAP_TSO(hw_cap)		((hw_cap) & HW_CAP_TSO)
 
 struct hix5hd2_desc {
 	__le32 buff_addr;
@@ -205,6 +209,27 @@ struct hix5hd2_desc_sw {
 	unsigned int	size;
 };
 
+struct hix5hd2_sg_desc_ring {
+	struct sg_desc *desc;
+	dma_addr_t phys_addr;
+};
+
+struct frags_info {
+	__le32 addr;
+	__le32 size;
+};
+
+/* hardware supported max skb frags num */
+#define SG_MAX_SKB_FRAGS	17
+struct sg_desc {
+	__le32 total_len;
+	__le32 resvd0;
+	__le32 linear_addr;
+	__le32 linear_len;
+	/* reserve one more frags for memory alignment */
+	struct frags_info frags[SG_MAX_SKB_FRAGS + 1];
+};
+
 #define QUEUE_NUMS	4
 struct hix5hd2_priv {
 	struct hix5hd2_desc_sw pool[QUEUE_NUMS];
@@ -212,6 +237,7 @@ struct hix5hd2_priv {
 #define rx_bq		pool[1]
 #define tx_bq		pool[2]
 #define tx_rq		pool[3]
+	struct hix5hd2_sg_desc_ring tx_ring;
 
 	void __iomem *base;
 	void __iomem *ctrl_base;
@@ -225,6 +251,7 @@ struct hix5hd2_priv {
 	struct device_node *phy_node;
 	phy_interface_t	phy_mode;
 
+	unsigned long hw_cap;
 	unsigned int speed;
 	unsigned int duplex;
 
@@ -515,6 +542,27 @@ static int hix5hd2_rx(struct net_device *dev, int limit)
 	return num;
 }
 
+static void hix5hd2_clean_sg_desc(struct hix5hd2_priv *priv,
+				  struct sk_buff *skb, u32 pos)
+{
+	struct sg_desc *desc;
+	dma_addr_t addr;
+	u32 len;
+	int i;
+
+	desc = priv->tx_ring.desc + pos;
+
+	addr = le32_to_cpu(desc->linear_addr);
+	len = le32_to_cpu(desc->linear_len);
+	dma_unmap_single(priv->dev, addr, len, DMA_TO_DEVICE);
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		addr = le32_to_cpu(desc->frags[i].addr);
+		len = le32_to_cpu(desc->frags[i].size);
+		dma_unmap_page(priv->dev, addr, len, DMA_TO_DEVICE);
+	}
+}
+
 static void hix5hd2_xmit_reclaim(struct net_device *dev)
 {
 	struct sk_buff *skb;
@@ -542,8 +590,15 @@ static void hix5hd2_xmit_reclaim(struct net_device *dev)
 		pkts_compl++;
 		bytes_compl += skb->len;
 		desc = priv->tx_rq.desc + pos;
-		addr = le32_to_cpu(desc->buff_addr);
-		dma_unmap_single(priv->dev, addr, skb->len, DMA_TO_DEVICE);
+
+		if (skb_shinfo(skb)->nr_frags) {
+			hix5hd2_clean_sg_desc(priv, skb, pos);
+		} else {
+			addr = le32_to_cpu(desc->buff_addr);
+			dma_unmap_single(priv->dev, addr, skb->len,
+					 DMA_TO_DEVICE);
+		}
+
 		priv->tx_skb[pos] = NULL;
 		dev_consume_skb_any(skb);
 		pos = dma_ring_incr(pos, TX_DESC_NUM);
@@ -604,12 +659,66 @@ static irqreturn_t hix5hd2_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static u32 hix5hd2_get_desc_cmd(struct sk_buff *skb, unsigned long hw_cap)
+{
+	u32 cmd = 0;
+
+	if (HAS_CAP_TSO(hw_cap)) {
+		if (skb_shinfo(skb)->nr_frags)
+			cmd |= DESC_SG;
+		cmd |= skb_shinfo(skb)->nr_frags << DESC_FRAGS_NUM_OFF;
+	} else {
+		cmd |= DESC_FL_FULL |
+			((skb->len & DESC_DATA_MASK) << DESC_BUFF_LEN_OFF);
+	}
+
+	cmd |= (skb->len & DESC_DATA_MASK) << DESC_DATA_LEN_OFF;
+	cmd |= DESC_VLD_BUSY;
+
+	return cmd;
+}
+
+static int hix5hd2_fill_sg_desc(struct hix5hd2_priv *priv,
+				struct sk_buff *skb, u32 pos)
+{
+	struct sg_desc *desc;
+	dma_addr_t addr;
+	int ret;
+	int i;
+
+	desc = priv->tx_ring.desc + pos;
+
+	desc->total_len = cpu_to_le32(skb->len);
+	addr = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
+			      DMA_TO_DEVICE);
+	if (unlikely(dma_mapping_error(priv->dev, addr)))
+		return -EINVAL;
+	desc->linear_addr = cpu_to_le32(addr);
+	desc->linear_len = cpu_to_le32(skb_headlen(skb));
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+		int len = frag->size;
+
+		addr = skb_frag_dma_map(priv->dev, frag, 0, len, DMA_TO_DEVICE);
+		ret = dma_mapping_error(priv->dev, addr);
+		if (unlikely(ret))
+			return -EINVAL;
+		desc->frags[i].addr = cpu_to_le32(addr);
+		desc->frags[i].size = cpu_to_le32(len);
+	}
+
+	return 0;
+}
+
 static int hix5hd2_net_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct hix5hd2_priv *priv = netdev_priv(dev);
 	struct hix5hd2_desc *desc;
 	dma_addr_t addr;
 	u32 pos;
+	u32 cmd;
+	int ret;
 
 	/* software write pointer */
 	pos = dma_cnt(readl_relaxed(priv->base + TX_BQ_WR_ADDR));
@@ -620,18 +729,31 @@ static int hix5hd2_net_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_BUSY;
 	}
 
-	addr = dma_map_single(priv->dev, skb->data, skb->len, DMA_TO_DEVICE);
-	if (dma_mapping_error(priv->dev, addr)) {
-		dev_kfree_skb_any(skb);
-		return NETDEV_TX_OK;
-	}
-
 	desc = priv->tx_bq.desc + pos;
+
+	cmd = hix5hd2_get_desc_cmd(skb, priv->hw_cap);
+	desc->cmd = cpu_to_le32(cmd);
+
+	if (skb_shinfo(skb)->nr_frags) {
+		ret = hix5hd2_fill_sg_desc(priv, skb, pos);
+		if (unlikely(ret)) {
+			dev_kfree_skb_any(skb);
+			dev->stats.tx_dropped++;
+			return NETDEV_TX_OK;
+		}
+		addr = priv->tx_ring.phys_addr + pos * sizeof(struct sg_desc);
+	} else {
+		addr = dma_map_single(priv->dev, skb->data, skb->len,
+				      DMA_TO_DEVICE);
+		if (unlikely(dma_mapping_error(priv->dev, addr))) {
+			dev_kfree_skb_any(skb);
+			dev->stats.tx_dropped++;
+			return NETDEV_TX_OK;
+		}
+	}
 	desc->buff_addr = cpu_to_le32(addr);
+
 	priv->tx_skb[pos] = skb;
-	desc->cmd = cpu_to_le32(DESC_VLD_BUSY | DESC_FL_FULL |
-				(skb->len & DESC_DATA_MASK) << DESC_DATA_LEN_OFF |
-				(skb->len & DESC_DATA_MASK) << DESC_BUFF_LEN_OFF);
 
 	/* ensure desc updated */
 	wmb();
@@ -866,10 +988,40 @@ static int hix5hd2_init_hw_desc_queue(struct hix5hd2_priv *priv)
 	return -ENOMEM;
 }
 
+static int hix5hd2_init_sg_desc_queue(struct hix5hd2_priv *priv)
+{
+	struct sg_desc *desc;
+	dma_addr_t phys_addr;
+
+	desc = (struct sg_desc *)dma_alloc_coherent(priv->dev,
+				TX_DESC_NUM * sizeof(struct sg_desc),
+				&phys_addr, GFP_KERNEL);
+	if (!desc)
+		return -ENOMEM;
+
+	priv->tx_ring.desc = desc;
+	priv->tx_ring.phys_addr = phys_addr;
+
+	return 0;
+}
+
+static void hix5hd2_destroy_sg_desc_queue(struct hix5hd2_priv *priv)
+{
+	if (priv->tx_ring.desc) {
+		dma_free_coherent(priv->dev,
+				  TX_DESC_NUM * sizeof(struct sg_desc),
+				  priv->tx_ring.desc, priv->tx_ring.phys_addr);
+		priv->tx_ring.desc = NULL;
+	}
+}
+
+static const struct of_device_id hix5hd2_of_match[];
+
 static int hix5hd2_dev_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
+	const struct of_device_id *of_id = NULL;
 	struct net_device *ndev;
 	struct hix5hd2_priv *priv;
 	struct resource *res;
@@ -887,6 +1039,13 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->netdev = ndev;
 
+	of_id = of_match_device(hix5hd2_of_match, dev);
+	if (!of_id) {
+		ret = -EINVAL;
+		goto out_free_netdev;
+	}
+	priv->hw_cap = (unsigned long)of_id->data;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	priv->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(priv->base)) {
@@ -976,11 +1135,24 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	ndev->ethtool_ops = &hix5hd2_ethtools_ops;
 	SET_NETDEV_DEV(ndev, dev);
 
+	if (HAS_CAP_TSO(priv->hw_cap))
+		ndev->hw_features |= NETIF_F_SG;
+
+	ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
+	ndev->vlan_features |= ndev->features;
+
 	ret = hix5hd2_init_hw_desc_queue(priv);
 	if (ret)
 		goto out_phy_node;
 
 	netif_napi_add(ndev, &priv->napi, hix5hd2_poll, NAPI_POLL_WEIGHT);
+
+	if (HAS_CAP_TSO(priv->hw_cap)) {
+		ret = hix5hd2_init_sg_desc_queue(priv);
+		if (ret)
+			goto out_destroy_queue;
+	}
+
 	ret = register_netdev(priv->netdev);
 	if (ret) {
 		netdev_err(ndev, "register_netdev failed!");
@@ -992,6 +1164,8 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	return ret;
 
 out_destroy_queue:
+	if (HAS_CAP_TSO(priv->hw_cap))
+		hix5hd2_destroy_sg_desc_queue(priv);
 	netif_napi_del(&priv->napi);
 	hix5hd2_destroy_hw_desc_queue(priv);
 out_phy_node:
@@ -1016,6 +1190,8 @@ static int hix5hd2_dev_remove(struct platform_device *pdev)
 	mdiobus_unregister(priv->bus);
 	mdiobus_free(priv->bus);
 
+	if (HAS_CAP_TSO(priv->hw_cap))
+		hix5hd2_destroy_sg_desc_queue(priv);
 	hix5hd2_destroy_hw_desc_queue(priv);
 	of_node_put(priv->phy_node);
 	cancel_work_sync(&priv->tx_timeout_task);
-- 
2.8.2

^ permalink raw reply related

* [PATCH v2 3/4] net: hix5hd2_gmac: add reset control and clock signals
From: Dongpo Li @ 2016-12-05 13:28 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
	yisen.zhuang, salil.mehta, davem, arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li
In-Reply-To: <1480944481-118803-1-git-send-email-lidongpo@hisilicon.com>

Add three reset control signals, "mac_core_rst", "mac_ifc_rst" and
"phy_rst".
The following diagram explained how the reset signals work.

                        SoC
|-----------------------------------------------------
|                               ------                |
|                               | cpu |               |
|                               ------                |
|                                  |                  |
|                              ------------ AMBA bus  |
|                         GMAC     |                  |
|                            ----------------------   |
| ------------- mac_core_rst | --------------      |  |
| |clock and   |-------------->|   mac core  |     |  |
| |reset       |             | --------------      |  |
| |generator   |----         |       |             |  |
| -------------     |        | ----------------    |  |
|          |        ---------->| mac interface |   |  |
|          |     mac_ifc_rst | ----------------    |  |
|          |                 |       |             |  |
|          |                 | ------------------  |  |
|          |phy_rst          | | RGMII interface | |  |
|          |                 | ------------------  |  |
|          |                 ----------------------   |
|----------|------------------------------------------|
           |                          |
           |                      ----------
           |--------------------- |PHY chip |
                                  ----------

The "mac_core_rst" represents "mac core reset signal", it resets
the mac core including packet processing unit, descriptor processing unit,
tx engine, rx engine, control unit.
The "mac_ifc_rst" represents "mac interface reset signal", it resets
the mac interface. The mac interface unit connects mac core and
data interface like MII/RMII/RGMII. After we set a new value of
interface mode, we must reset mac interface to reload the new mode value.
The "mac_core_rst" and "mac_ifc_rst" are both optional to be
backward compatible with the hix5hd2 SoC.
The "phy_rst" represents "phy reset signal", it does a hardware reset
on the PHY chip. This reset signal is optional if the PHY can work well
without the hardware reset.

Add one more clock signal, the existing is MAC core clock,
and the new one is MAC interface clock.
The MAC interface clock is optional to be backward compatible with
the hix5hd2 SoC.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 .../bindings/net/hisilicon-hix5hd2-gmac.txt        |  20 ++-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c      | 139 +++++++++++++++++++--
 2 files changed, 144 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index 75920f0..063c02d 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -17,6 +17,16 @@ Required properties:
 - phy-handle: see ethernet.txt [1].
 - mac-address: see ethernet.txt [1].
 - clocks: clock phandle and specifier pair.
+- clock-names: contain the clock name "mac_core"(required) and "mac_ifc"(optional).
+- resets: should contain the phandle to the MAC core reset signal(optional),
+	the MAC interface reset signal(optional)
+	and the PHY reset signal(optional).
+- reset-names: contain the reset signal name "mac_core"(optional),
+	"mac_ifc"(optional) and "phy"(optional).
+- hisilicon,phy-reset-delays-us: triplet of delays if PHY reset signal given.
+	The 1st cell is reset pre-delay in micro seconds.
+	The 2nd cell is reset pulse in micro seconds.
+	The 3rd cell is reset post-delay in micro seconds.
 
 - PHY subnode: inherits from phy binding [2]
 
@@ -25,15 +35,19 @@ Required properties:
 
 Example:
 	gmac0: ethernet@f9840000 {
-		compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
+		compatible = "hisilicon,hi3798cv200-gemac", "hisilicon,hisi-gemac-v2";
 		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
 		interrupts = <0 71 4>;
 		#address-cells = <1>;
 		#size-cells = <0>;
-		phy-mode = "mii";
+		phy-mode = "rgmii";
 		phy-handle = <&phy2>;
 		mac-address = [00 00 00 00 00 00];
-		clocks = <&clock HIX5HD2_MAC0_CLK>;
+		clocks = <&crg HISTB_ETH0_MAC_CLK>, <&crg HISTB_ETH0_MACIF_CLK>;
+		clock-names = "mac_core", "mac_ifc";
+		resets = <&crg 0xcc 8>, <&crg 0xcc 10>, <&crg 0xcc 12>;
+		reset-names = "mac_core", "mac_ifc", "phy";
+		hisilicon,phy-reset-delays-us = <10000 10000 30000>;
 
 		phy2: ethernet-phy@2 {
 			reg = <2>;
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 18af55b..ee7e9ce 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -14,6 +14,7 @@
 #include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
+#include <linux/reset.h>
 #include <linux/clk.h>
 #include <linux/circ_buf.h>
 
@@ -197,6 +198,15 @@
 #define GEMAC_V2			(GEMAC_V1 | HW_CAP_TSO)
 #define HAS_CAP_TSO(hw_cap)		((hw_cap) & HW_CAP_TSO)
 
+#define PHY_RESET_DELAYS_PROPERTY	"hisilicon,phy-reset-delays-us"
+
+enum phy_reset_delays {
+	PRE_DELAY,
+	PULSE,
+	POST_DELAY,
+	DELAYS_NUM,
+};
+
 struct hix5hd2_desc {
 	__le32 buff_addr;
 	__le32 cmd;
@@ -255,12 +265,26 @@ struct hix5hd2_priv {
 	unsigned int speed;
 	unsigned int duplex;
 
-	struct clk *clk;
+	struct clk *mac_core_clk;
+	struct clk *mac_ifc_clk;
+	struct reset_control *mac_core_rst;
+	struct reset_control *mac_ifc_rst;
+	struct reset_control *phy_rst;
+	u32 phy_reset_delays[DELAYS_NUM];
 	struct mii_bus *bus;
 	struct napi_struct napi;
 	struct work_struct tx_timeout_task;
 };
 
+static inline void hix5hd2_mac_interface_reset(struct hix5hd2_priv *priv)
+{
+	if (!priv->mac_ifc_rst)
+		return;
+
+	reset_control_assert(priv->mac_ifc_rst);
+	reset_control_deassert(priv->mac_ifc_rst);
+}
+
 static void hix5hd2_config_port(struct net_device *dev, u32 speed, u32 duplex)
 {
 	struct hix5hd2_priv *priv = netdev_priv(dev);
@@ -293,6 +317,7 @@ static void hix5hd2_config_port(struct net_device *dev, u32 speed, u32 duplex)
 	if (duplex)
 		val |= GMAC_FULL_DUPLEX;
 	writel_relaxed(val, priv->ctrl_base);
+	hix5hd2_mac_interface_reset(priv);
 
 	writel_relaxed(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN);
 	if (speed == SPEED_1000)
@@ -807,16 +832,26 @@ static int hix5hd2_net_open(struct net_device *dev)
 	struct phy_device *phy;
 	int ret;
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(priv->mac_core_clk);
+	if (ret < 0) {
+		netdev_err(dev, "failed to enable mac core clk %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->mac_ifc_clk);
 	if (ret < 0) {
-		netdev_err(dev, "failed to enable clk %d\n", ret);
+		clk_disable_unprepare(priv->mac_core_clk);
+		netdev_err(dev, "failed to enable mac ifc clk %d\n", ret);
 		return ret;
 	}
 
 	phy = of_phy_connect(dev, priv->phy_node,
 			     &hix5hd2_adjust_link, 0, priv->phy_mode);
-	if (!phy)
+	if (!phy) {
+		clk_disable_unprepare(priv->mac_ifc_clk);
+		clk_disable_unprepare(priv->mac_core_clk);
 		return -ENODEV;
+	}
 
 	phy_start(phy);
 	hix5hd2_hw_init(priv);
@@ -847,7 +882,8 @@ static int hix5hd2_net_close(struct net_device *dev)
 		phy_disconnect(dev->phydev);
 	}
 
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->mac_ifc_clk);
+	clk_disable_unprepare(priv->mac_core_clk);
 
 	return 0;
 }
@@ -1015,6 +1051,48 @@ static void hix5hd2_destroy_sg_desc_queue(struct hix5hd2_priv *priv)
 	}
 }
 
+static inline void hix5hd2_mac_core_reset(struct hix5hd2_priv *priv)
+{
+	if (!priv->mac_core_rst)
+		return;
+
+	reset_control_assert(priv->mac_core_rst);
+	reset_control_deassert(priv->mac_core_rst);
+}
+
+static void hix5hd2_sleep_us(u32 time_us)
+{
+	u32 time_ms;
+
+	if (!time_us)
+		return;
+
+	time_ms = DIV_ROUND_UP(time_us, 1000);
+	if (time_ms < 20)
+		usleep_range(time_us, time_us + 500);
+	else
+		msleep(time_ms);
+}
+
+static void hix5hd2_phy_reset(struct hix5hd2_priv *priv)
+{
+	/* To make sure PHY hardware reset success,
+	 * we must keep PHY in deassert state first and
+	 * then complete the hardware reset operation
+	 */
+	reset_control_deassert(priv->phy_rst);
+	hix5hd2_sleep_us(priv->phy_reset_delays[PRE_DELAY]);
+
+	reset_control_assert(priv->phy_rst);
+	/* delay some time to ensure reset ok,
+	 * this depends on PHY hardware feature
+	 */
+	hix5hd2_sleep_us(priv->phy_reset_delays[PULSE]);
+	reset_control_deassert(priv->phy_rst);
+	/* delay some time to ensure later MDIO access */
+	hix5hd2_sleep_us(priv->phy_reset_delays[POST_DELAY]);
+}
+
 static const struct of_device_id hix5hd2_of_match[];
 
 static int hix5hd2_dev_probe(struct platform_device *pdev)
@@ -1060,23 +1138,55 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 		goto out_free_netdev;
 	}
 
-	priv->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		netdev_err(ndev, "failed to get clk\n");
+	priv->mac_core_clk = devm_clk_get(&pdev->dev, "mac_core");
+	if (IS_ERR(priv->mac_core_clk)) {
+		netdev_err(ndev, "failed to get mac core clk\n");
 		ret = -ENODEV;
 		goto out_free_netdev;
 	}
 
-	ret = clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(priv->mac_core_clk);
 	if (ret < 0) {
-		netdev_err(ndev, "failed to enable clk %d\n", ret);
+		netdev_err(ndev, "failed to enable mac core clk %d\n", ret);
 		goto out_free_netdev;
 	}
 
+	priv->mac_ifc_clk = devm_clk_get(&pdev->dev, "mac_ifc");
+	if (IS_ERR(priv->mac_ifc_clk))
+		priv->mac_ifc_clk = NULL;
+
+	ret = clk_prepare_enable(priv->mac_ifc_clk);
+	if (ret < 0) {
+		netdev_err(ndev, "failed to enable mac ifc clk %d\n", ret);
+		goto out_disable_mac_core_clk;
+	}
+
+	priv->mac_core_rst = devm_reset_control_get(dev, "mac_core");
+	if (IS_ERR(priv->mac_core_rst))
+		priv->mac_core_rst = NULL;
+	hix5hd2_mac_core_reset(priv);
+
+	priv->mac_ifc_rst = devm_reset_control_get(dev, "mac_ifc");
+	if (IS_ERR(priv->mac_ifc_rst))
+		priv->mac_ifc_rst = NULL;
+
+	priv->phy_rst = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(priv->phy_rst)) {
+		priv->phy_rst = NULL;
+	} else {
+		ret = of_property_read_u32_array(node,
+						 PHY_RESET_DELAYS_PROPERTY,
+						 priv->phy_reset_delays,
+						 DELAYS_NUM);
+		if (ret)
+			goto out_disable_clk;
+		hix5hd2_phy_reset(priv);
+	}
+
 	bus = mdiobus_alloc();
 	if (bus == NULL) {
 		ret = -ENOMEM;
-		goto out_free_netdev;
+		goto out_disable_clk;
 	}
 
 	bus->priv = priv;
@@ -1159,7 +1269,8 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 		goto out_destroy_queue;
 	}
 
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->mac_ifc_clk);
+	clk_disable_unprepare(priv->mac_core_clk);
 
 	return ret;
 
@@ -1174,6 +1285,10 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
 	mdiobus_unregister(bus);
 err_free_mdio:
 	mdiobus_free(bus);
+out_disable_clk:
+	clk_disable_unprepare(priv->mac_ifc_clk);
+out_disable_mac_core_clk:
+	clk_disable_unprepare(priv->mac_core_clk);
 out_free_netdev:
 	free_netdev(ndev);
 
-- 
2.8.2

^ permalink raw reply related

* [PATCH v2 4/4] ARM: dts: hix5hd2: add gmac generic compatible and clock names
From: Dongpo Li @ 2016-12-05 13:28 UTC (permalink / raw)
  To: robh+dt, mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
	yisen.zhuang, salil.mehta, davem, arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li
In-Reply-To: <1480944481-118803-1-git-send-email-lidongpo@hisilicon.com>

Add gmac generic compatible and clock names.

Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 arch/arm/boot/dts/hisi-x5hd2.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi b/arch/arm/boot/dts/hisi-x5hd2.dtsi
index fdcc23d..0da76c5 100644
--- a/arch/arm/boot/dts/hisi-x5hd2.dtsi
+++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi
@@ -436,18 +436,20 @@
 		};
 
 		gmac0: ethernet@1840000 {
-			compatible = "hisilicon,hix5hd2-gmac";
+			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 			reg = <0x1840000 0x1000>,<0x184300c 0x4>;
 			interrupts = <0 71 4>;
 			clocks = <&clock HIX5HD2_MAC0_CLK>;
+			clock-names = "mac_core";
 			status = "disabled";
 		};
 
 		gmac1: ethernet@1841000 {
-			compatible = "hisilicon,hix5hd2-gmac";
+			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
 			reg = <0x1841000 0x1000>,<0x1843010 0x4>;
 			interrupts = <0 72 4>;
 			clocks = <&clock HIX5HD2_MAC1_CLK>;
+			clock-names = "mac_core";
 			status = "disabled";
 		};
 
-- 
2.8.2

^ permalink raw reply related

* Re: [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Mark Brown @ 2016-12-05 13:37 UTC (permalink / raw)
  To: Romain Perier
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Jason Cooper, Andrew Lunn,
	Sebastian Hesselbarth, Gregory Clement,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
	dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-3-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]

On Thu, Dec 01, 2016 at 11:27:16AM +0100, Romain Perier wrote:

> Changes in v3:
>  - Don't enable the fifo mode based on the compatible string, we introduce
>    a module parameter "pio_mode". By default this option is set to zero, so
>    the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
>    mode.

Why?  If the hardware supports a more efficient mode of operation it
doesn't seem sensible not to use it.

> -	int i;
> +	int i, ret = 0;

Coding style - don't combine initialized and non-initalized variables on
one line.

>  static int a3700_spi_transfer_one(struct spi_master *master,
>  				  struct spi_device *spi,
>  				  struct spi_transfer *xfer)
>  {
>  	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
> -	int ret = 0;
> +	int ret;
>  
>  	a3700_spi_transfer_setup(spi, xfer);
>  
> @@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
>  	return ret;
>  }

This appears to be a random unrelated change, should probably be part of
the initial driver commit.

> +static int a3700_spi_fifo_transfer_one(struct spi_master *master,
> +				       struct spi_device *spi,
> +				       struct spi_transfer *xfer)
> +{
> +	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
> +	int ret = 0, timeout = A3700_SPI_TIMEOUT;
> +	unsigned int nbits = 0;
> +	u32 val;
> +
> +	a3700_spi_transfer_setup(spi, xfer);
> +
> +	a3700_spi->tx_buf  = xfer->tx_buf;
> +	a3700_spi->rx_buf  = xfer->rx_buf;
> +	a3700_spi->buf_len = xfer->len;
> +
> +	/* SPI transfer headers */
> +	a3700_spi_header_set(a3700_spi);
> +
> +	if (xfer->tx_buf)
> +		nbits = xfer->tx_nbits;
> +	else if (xfer->rx_buf)
> +		nbits = xfer->rx_nbits;
> +
> +	a3700_spi_pin_mode_set(a3700_spi, nbits);
> +
> +	if (xfer->rx_buf) {
> +		/* Set read data length */
> +		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
> +			     a3700_spi->buf_len);
> +		/* Start READ transfer */
> +		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
> +		val &= ~A3700_SPI_RW_EN;
> +		val |= A3700_SPI_XFER_START;
> +		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
> +	} else if (xfer->tx_buf) {
> +		/* Start Write transfer */

So this only supports single duplex transfers but there doesn't seem to
be anything that enforces this.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH v6 2/2] mtd: nand: Add support for Arasan Nand Flash Controller
From: Punnaiah Choudary Kalluri @ 2016-12-05 13:55 UTC (permalink / raw)
  To: Marek Vasut, dwmw2@infradead.org, computersforpeace@gmail.com,
	boris.brezillon@free-electrons.com, richard@nod.at,
	cyrille.pitchen@atmel.com, robh+dt@kernel.org,
	mark.rutland@arm.com
  Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Michal Simek, kalluripunnaiahchoudary@gmail.com, kpc528@gmail.com,
	linux-mtd@lists.infradead.org
In-Reply-To: <7a8fe6c9-b53f-a16f-19f7-6ce4a83672a2@gmail.com>

Hi Marek,

  Thanks for the review and comments.

> -----Original Message-----
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: Monday, December 05, 2016 10:10 AM
> To: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com;
> boris.brezillon@free-electrons.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com
> Cc: linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 2/2] mtd: nand: Add support for Arasan Nand Flash
> Controller
> 
> On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > Added the basic driver for Arasan Nand Flash Controller used in
> > Zynq UltraScale+ MPSoC. It supports only Hw Ecc and upto 24bit
> > correction.
> 
> Ummm, NAND, ECC, can you fix the acronyms to be in caps ?
>
 
Ok

> > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > ---
> > Chnages in v6:
> > - Addressed most of the Brian and Boris comments
> > - Separated the nandchip from the nand controller
> > - Removed the ecc lookup table from driver
> > - Now use framework nand waitfunction and readoob
> > - Fixed the compiler warning
> > - Adapted the new frameowrk changes related to ecc and ooblayout
> > - Disabled the clocks after the nand_reelase
> > - Now using only one completion object
> > - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
> >   are not implemented and i will patch them later
> > - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr
> will
> >   implement later once the basic driver is mainlined.
> > Changes in v5:
> > - Renamed the driver filei as arasan_nand.c
> > - Fixed all comments relaqted coding style
> > - Fixed comments related to propagating the errors
> > - Modified the anfc_write_page_hwecc as per the write_page
> >   prototype
> > Changes in v4:
> > - Added support for onfi timing mode configuration
> > - Added clock supppport
> > - Added support for multiple chipselects
> > Changes in v3:
> > - Removed unused variables
> > - Avoided busy loop and used jifies based implementation
> > - Fixed compiler warnings "right shift count >= width of type"
> > - Removed unneeded codei and improved error reporting
> > - Added onfi version check to ensure reading the valid address cycles
> > Changes in v2:
> > - Added missing of.h to avoid kbuild system report erro
> > ---
> >  drivers/mtd/nand/Kconfig       |   8 +
> >  drivers/mtd/nand/Makefile      |   1 +
> >  drivers/mtd/nand/arasan_nand.c | 974
> +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 983 insertions(+)
> >  create mode 100644 drivers/mtd/nand/arasan_nand.c
> >
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index 7b7a887..e831f4e 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -569,4 +569,12 @@ config MTD_NAND_MTK
> >  	  Enables support for NAND controller on MTK SoCs.
> >  	  This controller is found on mt27xx, mt81xx, mt65xx SoCs.
> >
> > +config MTD_NAND_ARASAN
> > +	tristate "Support for Arasan Nand Flash controller"
> > +	depends on HAS_IOMEM
> > +	depends on HAS_DMA
> > +	help
> > +	  Enables the driver for the Arasan Nand Flash controller on
> > +	  Zynq UltraScale+ MPSoC.
> > +
> >  endif # MTD_NAND
> > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
> > index cafde6f..44b8b00 100644
> > --- a/drivers/mtd/nand/Makefile
> > +++ b/drivers/mtd/nand/Makefile
> > @@ -58,5 +58,6 @@ obj-$(CONFIG_MTD_NAND_HISI504)	        +=
> hisi504_nand.o
> >  obj-$(CONFIG_MTD_NAND_BRCMNAND)		+= brcmnand/
> >  obj-$(CONFIG_MTD_NAND_QCOM)		+= qcom_nandc.o
> >  obj-$(CONFIG_MTD_NAND_MTK)		+= mtk_nand.o mtk_ecc.o
> > +obj-$(CONFIG_MTD_NAND_ARASAN)		+= arasan_nand.o
> 
> Keep the list at least reasonably sorted.

Ok

> 
> >  nand-objs := nand_base.o nand_bbt.o nand_timings.o
> > diff --git a/drivers/mtd/nand/arasan_nand.c
> b/drivers/mtd/nand/arasan_nand.c
> > new file mode 100644
> > index 0000000..6b0670e
> > --- /dev/null
> > +++ b/drivers/mtd/nand/arasan_nand.c
> > @@ -0,0 +1,974 @@
> > +/*
> > + * Arasan Nand Flash Controller Driver
> 
> NAND
> 
> > + * Copyright (C) 2014 - 2015 Xilinx, Inc.
> 
> It's 2016 now ...

Sorry :). Yes, I will update.

> 
> > + * This program is free software; you can redistribute it and/or modify it
> under
> > + * the terms of the GNU General Public License version 2 as published by
> the
> > + * Free Software Foundation; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io-64-nonatomic-lo-hi.h>
> > +#include <linux/module.h>
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/mtd/nand.h>
> > +#include <linux/mtd/partitions.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define DRIVER_NAME			"arasan_nand"
> > +#define EVNT_TIMEOUT			1000
> 
> Rename to EVENT_TIMEOUT_<units> to make this less cryptic

Ok.

> 
> > +#define STATUS_TIMEOUT			2000
> 
> DTTO

Thanks. Just realized that it is unused. I will remove this.

> 
> > +#define PKT_OFST			0x00
> > +#define MEM_ADDR1_OFST			0x04
> > +#define MEM_ADDR2_OFST			0x08
> > +#define CMD_OFST			0x0C
> > +#define PROG_OFST			0x10
> > +#define INTR_STS_EN_OFST		0x14
> > +#define INTR_SIG_EN_OFST		0x18
> > +#define INTR_STS_OFST			0x1C
> > +#define READY_STS_OFST			0x20
> > +#define DMA_ADDR1_OFST			0x24
> > +#define FLASH_STS_OFST			0x28
> > +#define DATA_PORT_OFST			0x30
> > +#define ECC_OFST			0x34
> > +#define ECC_ERR_CNT_OFST		0x38
> > +#define ECC_SPR_CMD_OFST		0x3C
> > +#define ECC_ERR_CNT_1BIT_OFST		0x40
> > +#define ECC_ERR_CNT_2BIT_OFST		0x44
> > +#define DMA_ADDR0_OFST			0x50
> > +#define DATA_INTERFACE_REG		0x6C
> 
> Why are some things suffixed with _OFST and some with _REG ? Consistency
> please. Using ARASAN_ prefix, ie. #define ARASAN_FOO 0xbar to define
> regs would be nice.
> 

Ok. I will maintain the consistency. Since all these definitions for arasan controller
and I feel adding the prefix is no value and some places it will go beyond 80 col limit.
Different reviewers have different opinions here. I am following the above convention.  

> > +#define PKT_CNT_SHIFT			12
> > +
> > +#define ECC_ENABLE			BIT(31)
> > +#define DMA_EN_MASK			GENMASK(27, 26)
> > +#define DMA_ENABLE			0x2
> > +#define DMA_EN_SHIFT			26
> > +#define REG_PAGE_SIZE_MASK		GENMASK(25, 23)
> > +#define REG_PAGE_SIZE_SHIFT		23
> > +#define REG_PAGE_SIZE_512		0
> > +#define REG_PAGE_SIZE_1K		5
> > +#define REG_PAGE_SIZE_2K		1
> > +#define REG_PAGE_SIZE_4K		2
> > +#define REG_PAGE_SIZE_8K		3
> > +#define REG_PAGE_SIZE_16K		4
> > +#define CMD2_SHIFT			8
> > +#define ADDR_CYCLES_SHIFT		28
> > +
> > +#define XFER_COMPLETE			BIT(2)
> > +#define READ_READY			BIT(1)
> > +#define WRITE_READY			BIT(0)
> > +#define MBIT_ERROR			BIT(3)
> > +#define ERR_INTRPT			BIT(4)
> > +
> > +#define PROG_PGRD			BIT(0)
> > +#define PROG_ERASE			BIT(2)
> > +#define PROG_STATUS			BIT(3)
> > +#define PROG_PGPROG			BIT(4)
> > +#define PROG_RDID			BIT(6)
> > +#define PROG_RDPARAM			BIT(7)
> > +#define PROG_RST			BIT(8)
> > +#define PROG_GET_FEATURE		BIT(9)
> > +#define PROG_SET_FEATURE		BIT(10)
> > +
> > +#define ONFI_STATUS_FAIL		BIT(0)
> > +#define ONFI_STATUS_READY		BIT(6)
> > +
> > +#define PG_ADDR_SHIFT			16
> > +#define BCH_MODE_SHIFT			25
> > +#define BCH_EN_SHIFT			27
> > +#define ECC_SIZE_SHIFT			16
> > +
> > +#define MEM_ADDR_MASK			GENMASK(7, 0)
> > +#define BCH_MODE_MASK			GENMASK(27, 25)
> > +
> > +#define CS_MASK				GENMASK(31, 30)
> > +#define CS_SHIFT			30
> > +
> > +#define PAGE_ERR_CNT_MASK		GENMASK(16, 8)
> > +#define PKT_ERR_CNT_MASK		GENMASK(7, 0)
> > +
> > +#define NVDDR_MODE			BIT(9)
> > +#define NVDDR_TIMING_MODE_SHIFT		3
> > +
> > +#define ONFI_ID_LEN			8
> > +#define TEMP_BUF_SIZE			512
> > +#define NVDDR_MODE_PACKET_SIZE		8
> > +#define SDR_MODE_PACKET_SIZE		4
> > +
> > +#define ONFI_DATA_INTERFACE_NVDDR      (1 << 4)
> 
> BIT() ?

Sure.

> 
> 
> [...]
> 
> > +struct anfc {
> > +	struct nand_hw_control controller;
> > +	struct list_head chips;
> > +	struct device *dev;
> > +	void __iomem *base;
> > +	int curr_cmd;
> > +	struct clk *clk_sys;
> > +	struct clk *clk_flash;
> > +	bool dma;
> > +	bool err;
> > +	bool iswriteoob;
> > +	u8 buf[TEMP_BUF_SIZE];
> > +	int irq;
> > +	u32 bufshift;
> > +	int csnum;
> > +	struct completion evnt;
> 
> event ?

Ok

> 
> > +};
> 
> [...]
> 
> > +static void anfc_prepare_cmd(struct anfc *nfc, u8 cmd1, u8 cmd2, u8
> dmamode,
> > +			     u32 pagesize, u8 addrcycles)
> > +{
> > +	u32 regval;
> > +
> > +	regval = cmd1 | (cmd2 << CMD2_SHIFT);
> > +	if (dmamode && nfc->dma)
> > +		regval |= DMA_ENABLE << DMA_EN_SHIFT;
> > +	if (addrcycles)
> > +		regval |= addrcycles << ADDR_CYCLES_SHIFT;
> > +	if (pagesize)
> > +		regval |= anfc_page(pagesize) << REG_PAGE_SIZE_SHIFT;
> 
> Drop the if (foo), if it's zero, the regval would be OR'd with zero.

Ok.
> 
> > +	writel(regval, nfc->base + CMD_OFST);
> > +}
> > +
> > +static int anfc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> > +			  int page)
> > +{
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +
> > +	nfc->iswriteoob = true;
> > +	chip->cmdfunc(mtd, NAND_CMD_SEQIN, mtd->writesize, page);
> > +	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> > +	nfc->iswriteoob = false;
> > +
> > +	return 0;
> > +}
> > +
> > +static void anfc_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> > +{
> > +	u32 pktcount, pktsize, eccintr = 0;
> > +	unsigned int buf_rd_cnt = 0;
> > +	u32 *bufptr = (u32 *)buf;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	dma_addr_t paddr;
> > +
> > +	if (nfc->curr_cmd == NAND_CMD_READ0) {
> > +		pktsize = achip->pktsize;
> > +		pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
> > +		if (!achip->bch)
> > +			eccintr = MBIT_ERROR;
> > +	} else {
> > +		pktsize = len;
> > +		pktcount = 1;
> > +	}
> > +
> > +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > +	if (nfc->dma) {
> > +		paddr = dma_map_single(nfc->dev, buf, len,
> DMA_FROM_DEVICE);
> > +		if (dma_mapping_error(nfc->dev, paddr)) {
> > +			dev_err(nfc->dev, "Read buffer mapping error");
> > +			return;
> > +		}
> > +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > +		anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
> > +		writel(PROG_PGRD, nfc->base + PROG_OFST);
> > +		anfc_wait_for_event(nfc);
> > +		dma_unmap_single(nfc->dev, paddr, len,
> DMA_FROM_DEVICE);
> 
> Split this function into anfc_read_buf() and then anfc_read_buf_pio()
> and anfc_read_buf_dma() to avoid this ugliness. Also, does this handle
> any errors in any way ? Looks like it ignores all errors, so please fix.
> 

Ok. Regarding errors, it handles the errors except checking for contiguous
Address region  for the given address. I will add this in next version.

> > +		return;
> > +	}
> > +
> > +	anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > +	writel(PROG_PGRD, nfc->base + PROG_OFST);
> > +
> > +	while (buf_rd_cnt < pktcount) {
> > +		anfc_wait_for_event(nfc);
> > +		buf_rd_cnt++;
> > +
> > +		if (buf_rd_cnt == pktcount)
> > +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > +		readsl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > +		bufptr += (pktsize / 4);
> > +
> > +		if (buf_rd_cnt < pktcount)
> > +			anfc_enable_intrs(nfc, (READ_READY | eccintr));
> > +	}
> > +
> > +	anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_write_buf(struct mtd_info *mtd, const uint8_t *buf, int
> len)
> > +{
> > +	u32 pktcount, pktsize;
> > +	unsigned int buf_wr_cnt = 0;
> > +	u32 *bufptr = (u32 *)buf;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	dma_addr_t paddr;
> > +
> > +	if (nfc->iswriteoob) {
> > +		pktsize = len;
> > +		pktcount = 1;
> > +	} else {
> > +		pktsize = achip->pktsize;
> > +		pktcount = mtd->writesize / pktsize;
> > +	}
> 
> This looks like a copy of the read path. Can these two functions be
> parametrized and merged ?
> 
Ok.

> > +	anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > +	if (nfc->dma) {
> > +		paddr = dma_map_single(nfc->dev, (void *)buf, len,
> > +				       DMA_TO_DEVICE);
> > +		if (dma_mapping_error(nfc->dev, paddr)) {
> > +			dev_err(nfc->dev, "Write buffer mapping error");
> > +			return;
> > +		}
> > +		lo_hi_writeq(paddr, nfc->base + DMA_ADDR0_OFST);
> > +		anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +		writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > +		anfc_wait_for_event(nfc);
> > +		dma_unmap_single(nfc->dev, paddr, len, DMA_TO_DEVICE);
> > +		return;
> > +	}
> > +
> > +	anfc_enable_intrs(nfc, WRITE_READY);
> > +	writel(PROG_PGPROG, nfc->base + PROG_OFST);
> > +
> > +	while (buf_wr_cnt < pktcount) {
> > +		anfc_wait_for_event(nfc);
> > +		buf_wr_cnt++;
> > +		if (buf_wr_cnt == pktcount)
> > +			anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +
> > +		writesl(nfc->base + DATA_PORT_OFST, bufptr, pktsize/4);
> > +		bufptr += (pktsize / 4);
> > +
> > +		if (buf_wr_cnt < pktcount)
> > +			anfc_enable_intrs(nfc, WRITE_READY);
> > +	}
> > +
> > +	anfc_wait_for_event(nfc);
> > +}
> 
> 
> [...]
> 
> > +static void anfc_writefifo(struct anfc *nfc, u32 prog, u32 size, u8 *buf)
> > +{
> > +	u32 *bufptr = (u32 *)buf;
> > +
> > +	anfc_enable_intrs(nfc, WRITE_READY);
> > +
> > +	writel(prog, nfc->base + PROG_OFST);
> > +	anfc_wait_for_event(nfc);
> > +
> > +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +	writesl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
> 
> use ioread32_rep and iowrite32_rep , otherwise this won't compile on x86
> with COMPILE_TEST.
> 

Ok. I have just got the kbuild notification for x86 system. I will fix this.


> > +	anfc_wait_for_event(nfc);
> > +}
> > +
> > +static void anfc_readfifo(struct anfc *nfc, u32 prog, u32 size)
> > +{
> > +	u32 *bufptr = (u32 *)nfc->buf;
> > +
> > +	anfc_enable_intrs(nfc, READ_READY);
> > +
> > +	writel(prog, nfc->base + PROG_OFST);
> > +	anfc_wait_for_event(nfc);
> > +
> > +	anfc_enable_intrs(nfc, XFER_COMPLETE);
> > +	readsl(nfc->base + DATA_PORT_OFST, bufptr, size/4);
> 
> See above
> 
> > +	anfc_wait_for_event(nfc);
> > +}
> 
> 
> [...]
> 
> > +static void anfc_select_chip(struct mtd_info *mtd, int num)
> > +{
> > +	u32 val;
> > +	struct nand_chip *chip = mtd_to_nand(mtd);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +
> > +	if (num == -1)
> > +		return;
> > +
> > +	val = readl(nfc->base + MEM_ADDR2_OFST);
> > +	val = (val & ~(CS_MASK)) | (achip->csnum << CS_SHIFT);
> > +	val = (val & ~(BCH_MODE_MASK)) | (achip->bchmode <<
> BCH_MODE_SHIFT);
> 
> Just rewrite this as a series of val &= ~(foo | bar); val |= baz | quux;
> for clarity sake.
>

 
Ok.

> > +	writel(val, nfc->base + MEM_ADDR2_OFST);
> > +	nfc->csnum = achip->csnum;
> > +	writel(achip->eccval, nfc->base + ECC_OFST);
> > +	writel(achip->inftimeval, nfc->base + DATA_INTERFACE_REG);
> > +}
> > +
> > +static irqreturn_t anfc_irq_handler(int irq, void *ptr)
> > +{
> > +	struct anfc *nfc = ptr;
> > +	u32 regval = 0, status;
> > +
> > +	status = readl(nfc->base + INTR_STS_OFST);
> > +	if (status & XFER_COMPLETE) {
> > +		complete(&nfc->evnt);
> > +		regval |= XFER_COMPLETE;
> 
> Can the complete() be invoked multiple times ? That seems a bit odd.
> 

I will check and fix this.

> > +	}
> > +
> > +	if (status & READ_READY) {
> > +		complete(&nfc->evnt);
> > +		regval |= READ_READY;
> > +	}
> > +
> > +	if (status & WRITE_READY) {
> > +		complete(&nfc->evnt);
> > +		regval |= WRITE_READY;
> > +	}
> > +
> > +	if (status & MBIT_ERROR) {
> > +		nfc->err = true;
> > +		complete(&nfc->evnt);
> > +		regval |= MBIT_ERROR;
> > +	}
> > +
> > +	if (regval) {
> > +		writel(regval, nfc->base + INTR_STS_OFST);
> > +		writel(0, nfc->base + INTR_STS_EN_OFST);
> > +		writel(0, nfc->base + INTR_SIG_EN_OFST);
> > +
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> > +}
> > +
> > +static int anfc_onfi_set_features(struct mtd_info *mtd, struct nand_chip
> *chip,
> > +				int addr, uint8_t *subfeature_param)
> > +{
> > +	struct anfc *nfc = to_anfc(chip->controller);
> > +	struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > +	int status;
> > +
> > +	if (!chip->onfi_version || !(le16_to_cpu(chip-
> >onfi_params.opt_cmd)
> > +		& ONFI_OPT_CMD_SET_GET_FEATURES))
> 
> Split this into two conditions to improve readability.
> 

Ok.
> > +		return -EINVAL;
> > +
> > +	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
> > +	anfc_writefifo(nfc, PROG_SET_FEATURE, achip->spktsize,
> > +			subfeature_param);
> > +
> > +	status = chip->waitfunc(mtd, chip);
> > +	if (status & NAND_STATUS_FAIL)
> > +		return -EIO;
> > +
> > +	return 0;
> > +}
> > +
> > +static int anfc_init_timing_mode(struct anfc *nfc,
> > +				 struct anfc_nand_chip *achip)
> > +{
> > +	int mode, err;
> > +	unsigned int feature[2];
> > +	u32 inftimeval;
> > +	struct nand_chip *chip = &achip->chip;
> > +	struct mtd_info *mtd = nand_to_mtd(chip);
> > +
> > +	memset(feature, 0, NVDDR_MODE_PACKET_SIZE);
> > +	/* Get nvddr timing modes */
> > +	mode = onfi_get_sync_timing_mode(chip) & 0xff;
> > +	if (!mode) {
> > +		mode = fls(onfi_get_async_timing_mode(chip)) - 1;
> > +		inftimeval = mode;
> > +	} else {
> > +		mode = fls(mode) - 1;
> > +		inftimeval = NVDDR_MODE | (mode <<
> NVDDR_TIMING_MODE_SHIFT);
> > +		mode |= ONFI_DATA_INTERFACE_NVDDR;
> > +	}
> > +
> > +	feature[0] = mode;
> > +	chip->select_chip(mtd, achip->csnum);
> > +	err = chip->onfi_set_features(mtd, chip,
> ONFI_FEATURE_ADDR_TIMING_MODE,
> > +				      (uint8_t *)feature);
> > +	chip->select_chip(mtd, -1);
> > +	if (err)
> > +		return err;
> > +
> > +	achip->inftimeval = inftimeval;
> > +
> > +	if (mode & ONFI_DATA_INTERFACE_NVDDR)
> > +		achip->spktsize = NVDDR_MODE_PACKET_SIZE;
> > +
> > +	return 0;
> > +}
> 
> [...]
> 
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Xilinx, Inc");
> 
> There should be a contact with email address here.
> 
I think there is an alias for Xilinx, Inc in maintainers list.
If not I will update it.

Regards,
Punnaiah

> > +MODULE_DESCRIPTION("Arasan NAND Flash Controller Driver");
> >
> 
> 
> --
> Best regards,
> Marek Vasut
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply

* RE: [PATCH v6 1/2] mtd: arasan: Add device tree binding documentation
From: Punnaiah Choudary Kalluri @ 2016-12-05 13:56 UTC (permalink / raw)
  To: Boris Brezillon, Marek Vasut
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, richard@nod.at,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	linux-mtd@lists.infradead.org, kalluripunnaiahchoudary@gmail.com,
	kpc528@gmail.com, Michal Simek, cyrille.pitchen@atmel.com,
	computersforpeace@gmail.com, dwmw2@infradead.org
In-Reply-To: <20161205093630.650451d7@bbrezillon>



> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com]
> Sent: Monday, December 05, 2016 2:07 PM
> To: Marek Vasut <marek.vasut@gmail.com>
> Cc: Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> dwmw2@infradead.org; computersforpeace@gmail.com; richard@nod.at;
> cyrille.pitchen@atmel.com; robh+dt@kernel.org; mark.rutland@arm.com;
> linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> devicetree@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> kalluripunnaiahchoudary@gmail.com; kpc528@gmail.com; Punnaiah
> Choudary Kalluri <punnaia@xilinx.com>
> Subject: Re: [PATCH v6 1/2] mtd: arasan: Add device tree binding
> documentation
> 
> On Mon, 5 Dec 2016 05:25:54 +0100
> Marek Vasut <marek.vasut@gmail.com> wrote:
> 
> > On 12/05/2016 05:11 AM, Punnaiah Choudary Kalluri wrote:
> > > This patch adds the dts binding document for arasan nand flash
> > > controller.
> > >
> > > Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> > > ---
> > > changes in v6:
> > > - Removed num-cs property
> > > - Separated nandchip from nand controller changes in v5:
> > > - None
> > > Changes in v4:
> > > - Added num-cs property
> > > - Added clock support
> > > Changes in v3:
> > > - None
> > > Changes in v2:
> > > - None
> > > ---
> > >  .../devicetree/bindings/mtd/arasan_nfc.txt         | 38
> ++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > new file mode 100644
> > > index 0000000..dcbe7ad
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nfc.txt
> > > @@ -0,0 +1,38 @@
> > > +Arasan Nand Flash Controller with ONFI 3.1 support
> >
> > Arasan NAND Flash ...
> >
> > > +Required properties:
> > > +- compatible: Should be "arasan,nfc-v3p10"
> >
> > This v3p10 looks like version 3 patchlevel 10, but shouldn't we have
> > some fallback option which doesn't encode IP version in the compat
> > string ?
> 
> Not necessarily. Usually you define a generic compatible when you have
> other reliable means to detect the IP version (a version register for
> example).
> If you can't detect that at runtime, then providing only specific compatible
> strings is a good solution to avoid breaking the DT ABI.

Yes. I am also thinking the same. Arasan controller doesn't have the register
to indicate the IP version number.

> 
> >
> > Also, shouldn't quirks be handled by DT props instead of effectively
> > encoding them into the compatible string ?
> 
> Well, from my experience, it's better to hide as much as possible behind the
> compatible. This way, if new quirks are needed for a specific revision, you can
> update the driver without having to change the DT.
> 

Agree.

Regards,
Punnaiah

> >
> > > +- reg: Memory map for module access
> > > +- interrupt-parent: Interrupt controller the interrupt is routed
> > > +through
> > > +- interrupts: Should contain the interrupt for the device
> > > +- clock-name: List of input clocks - "clk_sys", "clk_flash"
> > > +	      (See clock bindings for details)
> > > +- clocks: Clock phandles (see clock bindings for details)
> > > +
> > > +Optional properties:
> > > +- arasan,has-mdma: Enables Dma support
> >
> > 'Enables DMA support' , with DMA in caps.
> >
> > > +for nand partition information please refer the below file
> >
> > For NAND ...
> >
> > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > +
> > > +Example:
> > > +	nand0: nand@ff100000 {
> > > +		compatible = "arasan,nfc-v3p10"
> > > +		reg = <0x0 0xff100000 0x1000>;
> > > +		clock-name = "clk_sys", "clk_flash"
> > > +		clocks = <&misc_clk &misc_clk>;
> > > +		interrupt-parent = <&gic>;
> > > +		interrupts = <0 14 4>;
> > > +		arasan,has-mdma;
> > > +		#address-cells = <1>;
> > > +		#size-cells = <0>
> > > +
> > > +		nand@0 {
> > > +			reg = <0>
> > > +			partition@0 {
> > > +				label = "filesystem";
> > > +				reg = <0x0 0x0 0x1000000>;
> > > +			};
> > > +			(...)
> > > +		};
> > > +	};
> > >
> >
> >


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply

* [PATCH v2] of/irq: improve error report on irq discovery process failure
From: Guilherme G. Piccoli @ 2016-12-05 13:59 UTC (permalink / raw)
  To: linux-pci, devicetree, linuxppc-dev
  Cc: mark.rutland, gpiccoli, marc.zyngier, robh+dt, frowand.list

On PowerPC machines some PCI slots might not have level triggered
interrupts capability (also know as level signaled interrupts),
leading of_irq_parse_pci() to complain by presenting error messages
on the kernel log - in this case, the properties "interrupt-map" and
"interrupt-map-mask" are not present on device's node in the device
tree.

This patch introduces a different message for this specific case,
and also reduces its level from error to warning. Besides, we warn
(once) that possibly some PCI slots on the system have no level
triggered interrupts available.
We changed some error return codes too on function of_irq_parse_raw()
in order other failure's cases can be presented in a more precise way.

Before this patch, when an adapter was plugged in a slot without level
interrupts capabilitiy on PowerPC, we saw a generic error message
like this:

    [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22

Now, with this applied, we see the following specific message:

    [16.154] pci 0014:60:00.1: of_irq_parse_pci: no interrupt-map found,
    INTx interrupts not available

Finally, we standardize the error path in of_irq_parse_raw() by always
taking the fail path instead of returning directly from the loop.

Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---

v2:
  * Changed function return code to always return negative values;
  * Improved/simplified warning outputs;
  * Changed some return codes and some error paths in of_irq_parse_raw()
in order to be more precise/consistent;

 drivers/of/irq.c        | 19 ++++++++++++-------
 drivers/of/of_pci_irq.c | 10 +++++++++-
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 393fea8..9deee86 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -104,7 +104,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	const __be32 *match_array = initial_match_array;
 	const __be32 *tmp, *imap, *imask, dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
 	u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
-	int imaplen, match, i;
+	int imaplen, match, i, rc = -EINVAL;
 
 #ifdef DEBUG
 	of_print_phandle_args("of_irq_parse_raw: ", out_irq);
@@ -134,7 +134,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	pr_debug("of_irq_parse_raw: ipar=%s, size=%d\n", of_node_full_name(ipar), intsize);
 
 	if (out_irq->args_count != intsize)
-		return -EINVAL;
+		goto fail;
 
 	/* Look for this #address-cells. We have to implement the old linux
 	 * trick of looking for the parent here as some device-trees rely on it
@@ -153,8 +153,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	pr_debug(" -> addrsize=%d\n", addrsize);
 
 	/* Range check so that the temporary buffer doesn't overflow */
-	if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS))
+	if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS)) {
+		rc = -EFAULT;
 		goto fail;
+	}
 
 	/* Precalculate the match array - this simplifies match loop */
 	for (i = 0; i < addrsize; i++)
@@ -240,10 +242,11 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 			    newintsize, newaddrsize);
 
 			/* Check for malformed properties */
-			if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS))
-				goto fail;
-			if (imaplen < (newaddrsize + newintsize))
+			if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS)
+			    || (imaplen < (newaddrsize + newintsize))) {
+				rc = -EFAULT;
 				goto fail;
+			}
 
 			imap += newaddrsize + newintsize;
 			imaplen -= newaddrsize + newintsize;
@@ -271,11 +274,13 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 		ipar = newpar;
 		newpar = NULL;
 	}
+	rc = -ENOENT; /* No interrupt-map found */
+
  fail:
 	of_node_put(ipar);
 	of_node_put(newpar);
 
-	return -EINVAL;
+	return rc;
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_raw);
 
diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 2306313..c175d9c 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -93,7 +93,15 @@ int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq
 		goto err;
 	return 0;
 err:
-	dev_err(&pdev->dev, "of_irq_parse_pci() failed with rc=%d\n", rc);
+	if (rc == -ENOENT) {
+		dev_warn(&pdev->dev,
+			"%s: no interrupt-map found, INTx interrupts not available\n",
+			__func__);
+		pr_warn_once("%s: possibly some PCI slots don't have level triggered interrupts capability\n",
+			__func__);
+	} else {
+		dev_err(&pdev->dev, "%s: failed with rc=%d\n", __func__, rc);
+	}
 	return rc;
 }
 EXPORT_SYMBOL_GPL(of_irq_parse_pci);
-- 
2.1.0

^ permalink raw reply related

* Re: [RFC] New Device Tree property - "bonding"
From: Rob Herring @ 2016-12-05 14:18 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram
  Cc: frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org,
	Chris Paterson, Geert Uytterhoeven,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <KL1PR06MB1031872E5DC2C65F2A490D6BC3890-k6wCOA2IOKSdmgae6ZAqr20DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

On Fri, Nov 25, 2016 at 10:55 AM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> Hello DT maintainers,
>
> In one of the Renesas SoCs we have a device called DRIF (Digital Radio Interface) controller. A DRIF channel contains 4 external pins - SCK, SYNC, Data pins D0 & D1.
>
> Internally a DRIF channel is made up of two SPI slave devices (also called sub-channels here) that share common CLK & SYNC signals but have their own resource set. The DRIF channel can have either one of the sub-channel active at a time or both. When both sub-channels are active, they need to be managed together as one device as they share same CLK & SYNC. We plan to tie these two sub-channels together with a new property called "renesas,bonding".

Is there no need to describe the master device? No GPIOs, regulators
or other sideband controls needed? If that's never needed (which seems
doubtful), then I would do something different here probably with the
master device as a child of one DRIF and then phandles to master from
the other DRIFs. Otherwise, this looks fine to me.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V7 00/10] PM / OPP: Multiple regulator support
From: Viresh Kumar @ 2016-12-05 14:21 UTC (permalink / raw)
  To: Rafael Wysocki
  Cc: Lists linaro-kernel, linux-pm@vger.kernel.org, Stephen Boyd,
	Nishanth Menon, Vincent Guittot, Rob Herring, Dave Gerlach,
	Mark Brown, devicetree@vger.kernel.org, Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar@linaro.org>

On 1 December 2016 at 16:28, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi,
>
> Some platforms (like TI) have complex DVFS configuration for CPU
> devices, where multiple regulators are required to be configured to
> change DVFS state of the device. This was explained well by Nishanth
> earlier [1].
>
> One of the major complaints around multiple regulators case was that the
> DT isn't responsible in any way to represent the order in which multiple
> supplies need to be programmed, before or after a frequency change. It
> was considered in this patch and such information is left for the
> platform specific OPP driver now, which can register its own
> opp_set_rate() callback with the OPP core and the OPP core will then
> call it during DVFS.
>
> The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
> and code to pass values for multiple regulators and verified that they
> are all properly read by the kernel (using debugfs interface).
>
> Dave Gerlach has already tested [2] it on the real TI platforms and it
> works well for him.
>
> This is rebased over: linux-next branch in the PM tree.
>
> V6->V7:
> - Added RBY from Stephen for the 8th patch as well.
> - Rebased over pm/bleeding-edge as the dependency patch is already
>   applied there.
> - s/dev_pm_opp_set_regulator()/dev_pm_opp_set_regulators() in a comment.
> - Removed the local 'names' array in cpufreq-dt and used "&name"
>   instead.
> - Only the 6th patch doesn't have a Reviewed-by Tag now..

Hi Rafael,

V6 of this series received some minor comments [1] and I have
resolved them all here. Now that the merge window is so close,
I think we should be merge it now so that it gets a chance to live
in linux-next for few days.

--
viresh

[1] https://marc.info/?l=linux-pm&m=148054543630253&w=2

^ permalink raw reply

* Re: [PATCH v2] of/irq: improve error report on irq discovery process failure
From: Rob Herring @ 2016-12-05 14:28 UTC (permalink / raw)
  To: Guilherme G. Piccoli
  Cc: linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linuxppc-dev, Mark Rutland, Benjamin Herrenschmidt, Frank Rowand,
	Marc Zyngier
In-Reply-To: <1480946356-16778-1-git-send-email-gpiccoli@linux.vnet.ibm.com>

On Mon, Dec 5, 2016 at 7:59 AM, Guilherme G. Piccoli
<gpiccoli@linux.vnet.ibm.com> wrote:
> On PowerPC machines some PCI slots might not have level triggered
> interrupts capability (also know as level signaled interrupts),
> leading of_irq_parse_pci() to complain by presenting error messages
> on the kernel log - in this case, the properties "interrupt-map" and
> "interrupt-map-mask" are not present on device's node in the device
> tree.
>
> This patch introduces a different message for this specific case,
> and also reduces its level from error to warning. Besides, we warn
> (once) that possibly some PCI slots on the system have no level
> triggered interrupts available.
> We changed some error return codes too on function of_irq_parse_raw()
> in order other failure's cases can be presented in a more precise way.
>
> Before this patch, when an adapter was plugged in a slot without level
> interrupts capabilitiy on PowerPC, we saw a generic error message
> like this:
>
>     [54.239] pci 002d:70:00.0: of_irq_parse_pci() failed with rc=-22
>
> Now, with this applied, we see the following specific message:
>
>     [16.154] pci 0014:60:00.1: of_irq_parse_pci: no interrupt-map found,
>     INTx interrupts not available
>
> Finally, we standardize the error path in of_irq_parse_raw() by always
> taking the fail path instead of returning directly from the loop.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> ---
>
> v2:
>   * Changed function return code to always return negative values;

Are you sure this is safe? This is tricky because of differing values
of NO_IRQ (0 or -1).

>   * Improved/simplified warning outputs;
>   * Changed some return codes and some error paths in of_irq_parse_raw()
> in order to be more precise/consistent;

This too could have some side effects on callers.

Not saying don't do these changes, just need some assurances this has
been considered.

Rob

^ permalink raw reply

* Re: [PATCH v3 1/2] ARM: dts: da850-lcdk: add the dumb-vga-dac node
From: Rob Herring @ 2016-12-05 14:32 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Mark Rutland, linux-devicetree, Kevin Hilman, Michael Turquette,
	Sekhar Nori, Russell King, linux-drm, LKML, Peter Ujfalusi,
	Tomi Valkeinen, Jyri Sarha, Frank Rowand, arm-soc,
	Laurent Pinchart
In-Reply-To: <1480420624-23544-2-git-send-email-bgolaszewski@baylibre.com>

On Tue, Nov 29, 2016 at 5:57 AM, Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
> Add the dumb-vga-dac node to the board DT together with corresponding
> ports and vga connector. This allows to retrieve the edid info from
> the display automatically.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  arch/arm/boot/dts/da850-lcdk.dts | 58 ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/da850.dtsi     | 17 ++++++++++++
>  2 files changed, 75 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> index 711b9ad..d864f11 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -50,6 +50,53 @@
>                         system-clock-frequency = <24576000>;
>                 };
>         };
> +
> +       vga_bridge {

s/_/-/

> +               compatible = "dumb-vga-dac";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&lcd_pins>;

Are these pins from the LCD controller? They should be part of the LCD
controller node.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH v3 1/3] ARM: dts: vf610-zii-dev-rev-b: Remove leftover PWM pingroup
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
	Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
	cphealy

Remove pwm0grp since it is:

	a) Not referenced anywhere in the DTS file (unlike Tower board it
	is based on, this board does not use/expose FTM0)

	b) Configures PTB2 and PTB3 in a way that contradicts
	pinctrl-mdio-mux

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v2:

	- None

 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index fa19cfd..2210811 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -677,15 +677,6 @@
 		>;
 	};
 
-	pinctrl_pwm0: pwm0grp {
-		fsl,pins = <
-			VF610_PAD_PTB0__FTM0_CH0	0x1582
-			VF610_PAD_PTB1__FTM0_CH1	0x1582
-			VF610_PAD_PTB2__FTM0_CH2	0x1582
-			VF610_PAD_PTB3__FTM0_CH3	0x1582
-		>;
-	};
-
 	pinctrl_qspi0: qspi0grp {
 		fsl,pins = <
 			VF610_PAD_PTD7__QSPI0_B_QSCK	0x31c3
-- 
2.5.5

^ permalink raw reply related

* [PATCH v3 2/3] ARM: dts: vf610-zii-dev: Add .dts file for rev. C
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
	Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
	cphealy
In-Reply-To: <1480948716-17744-1-git-send-email-andrew.smirnov@gmail.com>

Add .dts file for rev. C of the board by factoring out commonalities
into a shared include file (vf610-zii-dev-rev-b-c.dtsi) and deriving
revision specific file from it (vf610-zii-dev-rev-b.dts and
vf610-zii-dev-reb-c.dts).

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v2:

	- Removed unnecessary "ethernet-phy-id0022.1550" string from
          ethernet-phy@0 node

	- Correctecd IRQ_TYPE_EDGE_FALLING to IRQ_TYPE_LEVEL_LOW in
          ethernet-phy@0, since that's what KS8091 corresponding to
          that node uses

	- Changed PTB28's pinmux configuration to include a 47kOhm
          pullup to pull aforementioned PHY IRQ line to unasserted
          level by default


 arch/arm/boot/dts/Makefile                |   3 +-
 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 300 +----------------------
 arch/arm/boot/dts/vf610-zii-dev-rev-c.dts | 394 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/vf610-zii-dev.dtsi      | 383 +++++++++++++++++++++++++++++
 4 files changed, 780 insertions(+), 300 deletions(-)
 create mode 100644 arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
 create mode 100644 arch/arm/boot/dts/vf610-zii-dev.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index befcd26..9f0d2a1 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -442,7 +442,8 @@ dtb-$(CONFIG_SOC_VF610) += \
 	vf610-cosmic.dtb \
 	vf610m4-cosmic.dtb \
 	vf610-twr.dtb \
-	vf610-zii-dev-rev-b.dtb
+	vf610-zii-dev-rev-b.dtb \
+	vf610-zii-dev-rev-c.dtb
 dtb-$(CONFIG_ARCH_MXS) += \
 	imx23-evk.dtb \
 	imx23-olinuxino.dtb \
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index 2210811..c0fc3f2 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -43,32 +43,12 @@
  */
 
 /dts-v1/;
-#include "vf610.dtsi"
+#include "vf610-zii-dev.dtsi"
 
 / {
 	model = "ZII VF610 Development Board, Rev B";
 	compatible = "zii,vf610dev-b", "zii,vf610dev", "fsl,vf610";
 
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-
-	memory {
-		reg = <0x80000000 0x20000000>;
-	};
-
-	gpio-leds {
-		compatible = "gpio-leds";
-		pinctrl-0 = <&pinctrl_leds_debug>;
-		pinctrl-names = "default";
-
-		debug {
-			label = "zii:green:debug1";
-			gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
-			linux,default-trigger = "heartbeat";
-		};
-	};
-
 	mdio-mux {
 		compatible = "mdio-mux-gpio";
 		pinctrl-0 = <&pinctrl_mdio_mux>;
@@ -281,25 +261,6 @@
 		};
 	};
 
-	reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu {
-		compatible = "regulator-fixed";
-		regulator-name = "vcc_3v3_mcu";
-		regulator-min-microvolt = <3300000>;
-		regulator-max-microvolt = <3300000>;
-	};
-
-	usb0_vbus: regulator-usb0-vbus {
-		compatible = "regulator-fixed";
-		pinctrl-0 = <&pinctrl_usb_vbus>;
-		regulator-name = "usb_vbus";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-		enable-active-high;
-		regulator-always-on;
-		regulator-boot-on;
-		gpio = <&gpio0 6 0>;
-	};
-
 	spi0 {
 		compatible = "spi-gpio";
 		pinctrl-0 = <&pinctrl_gpio_spi0>;
@@ -336,49 +297,6 @@
 	};
 };
 
-&adc0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_adc0_ad5>;
-	vref-supply = <&reg_vcc_3v3_mcu>;
-	status = "okay";
-};
-
-&edma0 {
-	status = "okay";
-};
-
-&esdhc1 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_esdhc1>;
-	bus-width = <4>;
-	status = "okay";
-};
-
-&fec0 {
-	phy-mode = "rmii";
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec0>;
-	status = "okay";
-};
-
-&fec1 {
-	phy-mode = "rmii";
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec1>;
-	status = "okay";
-
-	fixed-link {
-		   speed = <100>;
-		   full-duplex;
-	};
-
-	mdio1: mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		status = "okay";
-	};
-};
-
 &i2c0 {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
@@ -403,33 +321,6 @@
 		interrupt-parent = <&gpio2>;
 		interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
 	};
-
-	lm75@48 {
-		compatible = "national,lm75";
-		reg = <0x48>;
-	};
-
-	at24c04@50 {
-		compatible = "atmel,24c04";
-		reg = <0x50>;
-	};
-
-	at24c04@52 {
-		compatible = "atmel,24c04";
-		reg = <0x52>;
-	};
-
-	ds1682@6b {
-		compatible = "dallas,ds1682";
-		reg = <0x6b>;
-	};
-};
-
-&i2c1 {
-	clock-frequency = <100000>;
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_i2c1>;
-	status = "okay";
 };
 
 &i2c2 {
@@ -499,120 +390,8 @@
 	};
 };
 
-&uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart0>;
-	status = "okay";
-};
-
-&uart1 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart1>;
-	status = "okay";
-};
-
-&uart2 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart2>;
-	status = "okay";
-};
-
-&usbdev0 {
-	disable-over-current;
-	vbus-supply = <&usb0_vbus>;
-	dr_mode = "host";
-	status = "okay";
-};
-
-&usbh1 {
-	disable-over-current;
-	status = "okay";
-};
-
-&usbmisc0 {
-	status = "okay";
-};
-
-&usbmisc1 {
-	status = "okay";
-};
-
-&usbphy0 {
-	status = "okay";
-};
-
-&usbphy1 {
-	status = "okay";
-};
 
 &iomuxc {
-	pinctrl_adc0_ad5: adc0ad5grp {
-		fsl,pins = <
-			VF610_PAD_PTC30__ADC0_SE5	0x00a1
-		>;
-	};
-
-	pinctrl_dspi0: dspi0grp {
-		fsl,pins = <
-			VF610_PAD_PTB18__DSPI0_CS1	0x1182
-			VF610_PAD_PTB19__DSPI0_CS0	0x1182
-			VF610_PAD_PTB20__DSPI0_SIN	0x1181
-			VF610_PAD_PTB21__DSPI0_SOUT	0x1182
-			VF610_PAD_PTB22__DSPI0_SCK	0x1182
-		>;
-	};
-
-	pinctrl_dspi2: dspi2grp {
-		fsl,pins = <
-			VF610_PAD_PTD31__DSPI2_CS1	0x1182
-			VF610_PAD_PTD30__DSPI2_CS0	0x1182
-			VF610_PAD_PTD29__DSPI2_SIN	0x1181
-			VF610_PAD_PTD28__DSPI2_SOUT	0x1182
-			VF610_PAD_PTD27__DSPI2_SCK	0x1182
-		>;
-	};
-
-	pinctrl_esdhc1: esdhc1grp {
-		fsl,pins = <
-			VF610_PAD_PTA24__ESDHC1_CLK	0x31ef
-			VF610_PAD_PTA25__ESDHC1_CMD	0x31ef
-			VF610_PAD_PTA26__ESDHC1_DAT0	0x31ef
-			VF610_PAD_PTA27__ESDHC1_DAT1	0x31ef
-			VF610_PAD_PTA28__ESDHC1_DATA2	0x31ef
-			VF610_PAD_PTA29__ESDHC1_DAT3	0x31ef
-			VF610_PAD_PTA7__GPIO_134	0x219d
-		>;
-	};
-
-	pinctrl_fec0: fec0grp {
-		fsl,pins = <
-			VF610_PAD_PTC0__ENET_RMII0_MDC	0x30d2
-			VF610_PAD_PTC1__ENET_RMII0_MDIO	0x30d3
-			VF610_PAD_PTC2__ENET_RMII0_CRS	0x30d1
-			VF610_PAD_PTC3__ENET_RMII0_RXD1	0x30d1
-			VF610_PAD_PTC4__ENET_RMII0_RXD0	0x30d1
-			VF610_PAD_PTC5__ENET_RMII0_RXER	0x30d1
-			VF610_PAD_PTC6__ENET_RMII0_TXD1	0x30d2
-			VF610_PAD_PTC7__ENET_RMII0_TXD0	0x30d2
-			VF610_PAD_PTC8__ENET_RMII0_TXEN	0x30d2
-		>;
-	};
-
-	pinctrl_fec1: fec1grp {
-		fsl,pins = <
-			VF610_PAD_PTA6__RMII_CLKIN		0x30d1
-			VF610_PAD_PTC9__ENET_RMII1_MDC		0x30d2
-			VF610_PAD_PTC10__ENET_RMII1_MDIO	0x30d3
-			VF610_PAD_PTC11__ENET_RMII1_CRS		0x30d1
-			VF610_PAD_PTC12__ENET_RMII1_RXD1	0x30d1
-			VF610_PAD_PTC13__ENET_RMII1_RXD0	0x30d1
-			VF610_PAD_PTC14__ENET_RMII1_RXER	0x30d1
-			VF610_PAD_PTC15__ENET_RMII1_TXD1	0x30d2
-			VF610_PAD_PTC16__ENET_RMII1_TXD0	0x30d2
-			VF610_PAD_PTC17__ENET_RMII1_TXEN	0x30d2
-		>;
-	};
-
 	pinctrl_gpio_e6185_eeprom_sel: pinctrl-gpio-e6185-eeprom-spi0 {
 		fsl,pins = <
 			VF610_PAD_PTE27__GPIO_132	0x33e2
@@ -629,39 +408,6 @@
 		>;
 	};
 
-	pinctrl_i2c_mux_reset: pinctrl-i2c-mux-reset {
-		fsl,pins = <
-			 VF610_PAD_PTE14__GPIO_119	0x31c2
-			 >;
-	};
-
-	pinctrl_i2c0: i2c0grp {
-		fsl,pins = <
-			VF610_PAD_PTB14__I2C0_SCL	0x37ff
-			VF610_PAD_PTB15__I2C0_SDA	0x37ff
-		>;
-	};
-
-	pinctrl_i2c1: i2c1grp {
-		fsl,pins = <
-			VF610_PAD_PTB16__I2C1_SCL	0x37ff
-			VF610_PAD_PTB17__I2C1_SDA	0x37ff
-		>;
-	};
-
-	pinctrl_i2c2: i2c2grp {
-		fsl,pins = <
-			VF610_PAD_PTA22__I2C2_SCL	0x37ff
-			VF610_PAD_PTA23__I2C2_SDA	0x37ff
-		>;
-	};
-
-	pinctrl_leds_debug: pinctrl-leds-debug {
-		fsl,pins = <
-			 VF610_PAD_PTD20__GPIO_74	0x31c2
-			 >;
-	};
-
 	pinctrl_mdio_mux: pinctrl-mdio-mux {
 		fsl,pins = <
 			VF610_PAD_PTA18__GPIO_8		0x31c2
@@ -676,48 +422,4 @@
 			VF610_PAD_PTB28__GPIO_98	0x219d
 		>;
 	};
-
-	pinctrl_qspi0: qspi0grp {
-		fsl,pins = <
-			VF610_PAD_PTD7__QSPI0_B_QSCK	0x31c3
-			VF610_PAD_PTD8__QSPI0_B_CS0	0x31ff
-			VF610_PAD_PTD9__QSPI0_B_DATA3	0x31c3
-			VF610_PAD_PTD10__QSPI0_B_DATA2	0x31c3
-			VF610_PAD_PTD11__QSPI0_B_DATA1	0x31c3
-			VF610_PAD_PTD12__QSPI0_B_DATA0	0x31c3
-		>;
-	};
-
-	pinctrl_uart0: uart0grp {
-		fsl,pins = <
-			VF610_PAD_PTB10__UART0_TX	0x21a2
-			VF610_PAD_PTB11__UART0_RX	0x21a1
-		>;
-	};
-
-	pinctrl_uart1: uart1grp {
-		fsl,pins = <
-			VF610_PAD_PTB23__UART1_TX	0x21a2
-			VF610_PAD_PTB24__UART1_RX	0x21a1
-		>;
-	};
-
-	pinctrl_uart2: uart2grp {
-		fsl,pins = <
-			VF610_PAD_PTD0__UART2_TX	0x21a2
-			VF610_PAD_PTD1__UART2_RX	0x21a1
-		>;
-	};
-
-	pinctrl_usb_vbus: pinctrl-usb-vbus {
-		fsl,pins = <
-			VF610_PAD_PTA16__GPIO_6	0x31c2
-		>;
-	};
-
-	pinctrl_usb0_host: usb0-host-grp {
-		fsl,pins = <
-			VF610_PAD_PTD6__GPIO_85		0x0062
-		>;
-	};
 };
diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
new file mode 100644
index 0000000..4d38f41
--- /dev/null
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts
@@ -0,0 +1,394 @@
+/*
+ * Copyright (C) 2015, 2016 Zodiac Inflight Innovations
+ *
+ * Based on an original 'vf610-twr.dts' which is Copyright 2015,
+ * Freescale Semiconductor, Inc.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License
+ *     version 2 as published by the Free Software Foundation.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "vf610-zii-dev.dtsi"
+
+/ {
+	model = "ZII VF610 Development Board, Rev C";
+	compatible = "zii,vf610dev-c", "zii,vf610dev", "fsl,vf610";
+
+	mdio-mux {
+		compatible = "mdio-mux-gpio";
+		pinctrl-0 = <&pinctrl_mdio_mux>;
+		pinctrl-names = "default";
+		gpios = <&gpio0 8  GPIO_ACTIVE_HIGH
+			 &gpio0 9  GPIO_ACTIVE_HIGH
+			 &gpio0 25 GPIO_ACTIVE_HIGH>;
+		mdio-parent-bus = <&mdio1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mdio_mux_1: mdio@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			switch0: switch0@0 {
+				compatible = "marvell,mv88e6190";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+				dsa,member = <0 0>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+						label = "cpu";
+						ethernet = <&fec1>;
+						fixed-link {
+							speed = <100>;
+							full-duplex;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+						label = "lan1";
+					};
+
+					port@2 {
+						reg = <2>;
+						label = "lan2";
+					};
+
+					port@3 {
+						reg = <3>;
+						label = "lan3";
+					};
+
+					port@4 {
+						reg = <4>;
+						label = "lan4";
+					};
+
+					switch0port10: port@10 {
+						reg = <10>;
+						label = "dsa";
+						phy-mode = "xgmii";
+						link = <&switch1port10>;
+					};
+				};
+			};
+		};
+
+		mdio_mux_2: mdio@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			switch1: switch1@0 {
+				compatible = "marvell,mv88e6190";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+				dsa,member = <0 1>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@1 {
+						reg = <1>;
+						label = "lan5";
+					};
+
+					port@2 {
+						reg = <2>;
+						label = "lan6";
+					};
+
+					port@3 {
+						reg = <3>;
+						label = "lan7";
+					};
+
+					port@4 {
+						reg = <4>;
+						label = "lan8";
+					};
+
+
+					switch1port10: port@10 {
+						reg = <10>;
+						label = "dsa";
+						phy-mode = "xgmii";
+						link = <&switch0port10>;
+					};
+				};
+			};
+		};
+
+		mdio_mux_4: mdio@4 {
+			reg = <4>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+	};
+};
+
+&dspi0 {
+	bus-num = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_dspi0>;
+	status = "okay";
+	spi-num-chipselects = <2>;
+
+	m25p128@0 {
+		compatible = "m25p128", "jedec,spi-nor";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0>;
+		spi-max-frequency = <1000000>;
+	};
+};
+
+&i2c0 {
+	/*
+	 * U712
+	 *
+	 * Exposed signals:
+	 *    P1 - WE2_CMD
+	 *    P2 - WE2_CLK
+	 */
+	gpio5: pca9557@18 {
+		compatible = "nxp,pca9557";
+		reg = <0x18>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	/*
+	 * U121
+	 *
+	 * Exposed signals:
+	 *    I/O0  - ENET_SWR_EN
+	 *    I/O1  - ESW1_RESETn
+	 *    I/O2  - ARINC_RESET
+	 *    I/O3  - DD1_IO_RESET
+	 *    I/O4  - ESW2_RESETn
+	 *    I/O5  - ESW3_RESETn
+	 *    I/O6  - ESW4_RESETn
+	 *    I/O8  - TP909
+	 *    I/O9  - FEM_SEL
+	 *    I/O10 - WIFI_RESETn
+	 *    I/O11 - PHY_RSTn
+	 *    I/O12 - OPT1_SD
+	 *    I/O13 - OPT2_SD
+	 *    I/O14 - OPT1_TX_DIS
+	 *    I/O15 - OPT2_TX_DIS
+	 */
+	gpio6: sx1503@20 {
+		compatible = "semtech,sx1503q";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_sx1503_20>;
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+		gpio-controller;
+		interrupt-controller;
+
+		enet_swr_en {
+			gpio-hog;
+			gpios = <0 GPIO_ACTIVE_HIGH>;
+			output-high;
+			line-name = "enet-swr-en";
+		};
+	};
+
+	/*
+	 * U715
+	 *
+	 * Exposed signals:
+	 *     IO0 - WE1_CLK
+	 *     IO1 - WE1_CMD
+	 */
+	gpio7: pca9554@22 {
+		compatible = "nxp,pca9554";
+		reg = <0x22>;
+		gpio-controller;
+		#gpio-cells = <2>;
+
+	};
+};
+
+&i2c1 {
+	at24mac602@00 {
+		compatible = "atmel,24c02";
+		reg = <0x50>;
+		read-only;
+	};
+};
+
+&i2c2 {
+	tca9548@70 {
+		compatible = "nxp,pca9548";
+		pinctrl-0 = <&pinctrl_i2c_mux_reset>;
+		pinctrl-names = "default";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x70>;
+		reset-gpios = <&gpio3 23 GPIO_ACTIVE_LOW>;
+
+		i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+		};
+
+		i2c@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			sfp2: at24c04@50 {
+				compatible = "atmel,24c02";
+				reg = <0x50>;
+			};
+		};
+
+		i2c@2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+
+			sfp3: at24c04@50 {
+				compatible = "atmel,24c02";
+				reg = <0x50>;
+			};
+		};
+
+		i2c@3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+		};
+	};
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart3>;
+	status = "okay";
+};
+
+&gpio0 {
+	eth0_intrp {
+		gpio-hog;
+		gpios = <23 GPIO_ACTIVE_HIGH>;
+		input;
+		line-name = "sx1503-irq";
+	};
+};
+
+&gpio3 {
+	eth0_intrp {
+		gpio-hog;
+		gpios = <2 GPIO_ACTIVE_HIGH>;
+		input;
+		line-name = "eth0-intrp";
+	};
+};
+
+&fec0 {
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "okay";
+
+		ethernet-phy@0 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_fec0_phy_int>;
+
+			interrupt-parent = <&gpio3>;
+			interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+			reg = <0>;
+		};
+	};
+};
+
+&iomuxc {
+	pinctr_atzb_rf_233: pinctrl-atzb-rf-233 {
+		fsl,pins = <
+			VF610_PAD_PTB2__GPIO_24		0x31c2
+			VF610_PAD_PTE27__GPIO_132	0x33e2
+		>;
+	};
+
+
+	pinctrl_sx1503_20: pinctrl-sx1503-20 {
+		fsl,pins = <
+			VF610_PAD_PTB1__GPIO_23		0x219d
+		>;
+	};
+
+	pinctrl_uart3: uart3grp {
+		fsl,pins = <
+			VF610_PAD_PTA20__UART3_TX	0x21a2
+			VF610_PAD_PTA21__UART3_RX	0x21a1
+		>;
+	};
+
+	pinctrl_mdio_mux: pinctrl-mdio-mux {
+		fsl,pins = <
+			VF610_PAD_PTA18__GPIO_8		0x31c2
+			VF610_PAD_PTA19__GPIO_9		0x31c2
+			VF610_PAD_PTB3__GPIO_25		0x31c2
+		>;
+	};
+
+	pinctrl_fec0_phy_int: pinctrl-fec0-phy-int {
+		fsl,pins = <
+			VF610_PAD_PTB28__GPIO_98	0x219d
+		>;
+	};
+};
diff --git a/arch/arm/boot/dts/vf610-zii-dev.dtsi b/arch/arm/boot/dts/vf610-zii-dev.dtsi
new file mode 100644
index 0000000..9f5e2e7
--- /dev/null
+++ b/arch/arm/boot/dts/vf610-zii-dev.dtsi
@@ -0,0 +1,383 @@
+/*
+ * Copyright (C) 2015, 2016 Zodiac Inflight Innovations
+ *
+ * Based on an original 'vf610-twr.dts' which is Copyright 2015,
+ * Freescale Semiconductor, Inc.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License
+ *     version 2 as published by the Free Software Foundation.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+n *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include "vf610.dtsi"
+
+/ {
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0x80000000 0x20000000>;
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-0 = <&pinctrl_leds_debug>;
+		pinctrl-names = "default";
+
+		debug {
+			label = "zii:green:debug1";
+			gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "heartbeat";
+		};
+	};
+
+	reg_vcc_3v3_mcu: regulator-vcc-3v3-mcu {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc_3v3_mcu";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+	};
+
+	usb0_vbus: regulator-usb0-vbus {
+		compatible = "regulator-fixed";
+		pinctrl-0 = <&pinctrl_usb_vbus>;
+		regulator-name = "usb_vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		regulator-boot-on;
+		gpio = <&gpio0 6 0>;
+	};
+};
+
+&adc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_adc0_ad5>;
+	vref-supply = <&reg_vcc_3v3_mcu>;
+	status = "okay";
+};
+
+&edma0 {
+	status = "okay";
+};
+
+&esdhc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_esdhc1>;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&fec0 {
+	phy-mode = "rmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fec0>;
+	status = "okay";
+};
+
+&fec1 {
+	phy-mode = "rmii";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_fec1>;
+	status = "okay";
+
+	fixed-link {
+		   speed = <100>;
+		   full-duplex;
+	};
+
+	mdio1: mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "okay";
+	};
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default", "gpio";
+	pinctrl-0 = <&pinctrl_i2c0>;
+	pinctrl-1 = <&pinctrl_i2c0_gpio>;
+	scl-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
+	sda-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+	status = "okay";
+
+	lm75@48 {
+		compatible = "national,lm75";
+		reg = <0x48>;
+	};
+
+	at24c04@50 {
+		compatible = "atmel,24c04";
+		reg = <0x50>;
+	};
+
+	at24c04@52 {
+		compatible = "atmel,24c04";
+		reg = <0x52>;
+	};
+
+	ds1682@6b {
+		compatible = "dallas,ds1682";
+		reg = <0x6b>;
+	};
+};
+
+&i2c1 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	status = "okay";
+};
+
+&i2c2 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c2>;
+	status = "okay";
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart0>;
+	status = "okay";
+};
+
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart1>;
+	status = "okay";
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_uart2>;
+	status = "okay";
+};
+
+&usbdev0 {
+	disable-over-current;
+	vbus-supply = <&usb0_vbus>;
+	dr_mode = "host";
+	status = "okay";
+};
+
+&usbh1 {
+	disable-over-current;
+	status = "okay";
+};
+
+&usbmisc0 {
+	status = "okay";
+};
+
+&usbmisc1 {
+	status = "okay";
+};
+
+&usbphy0 {
+	status = "okay";
+};
+
+&usbphy1 {
+	status = "okay";
+};
+
+&iomuxc {
+	pinctrl_adc0_ad5: adc0ad5grp {
+		fsl,pins = <
+			VF610_PAD_PTC30__ADC0_SE5	0x00a1
+		>;
+	};
+
+	pinctrl_dspi0: dspi0grp {
+		fsl,pins = <
+			VF610_PAD_PTB18__DSPI0_CS1	0x1182
+			VF610_PAD_PTB19__DSPI0_CS0	0x1182
+			VF610_PAD_PTB20__DSPI0_SIN	0x1181
+			VF610_PAD_PTB21__DSPI0_SOUT	0x1182
+			VF610_PAD_PTB22__DSPI0_SCK	0x1182
+		>;
+	};
+
+	pinctrl_dspi2: dspi2grp {
+		fsl,pins = <
+			VF610_PAD_PTD31__DSPI2_CS1	0x1182
+			VF610_PAD_PTD30__DSPI2_CS0	0x1182
+			VF610_PAD_PTD29__DSPI2_SIN	0x1181
+			VF610_PAD_PTD28__DSPI2_SOUT	0x1182
+			VF610_PAD_PTD27__DSPI2_SCK	0x1182
+		>;
+	};
+
+	pinctrl_esdhc1: esdhc1grp {
+		fsl,pins = <
+			VF610_PAD_PTA24__ESDHC1_CLK	0x31ef
+			VF610_PAD_PTA25__ESDHC1_CMD	0x31ef
+			VF610_PAD_PTA26__ESDHC1_DAT0	0x31ef
+			VF610_PAD_PTA27__ESDHC1_DAT1	0x31ef
+			VF610_PAD_PTA28__ESDHC1_DATA2	0x31ef
+			VF610_PAD_PTA29__ESDHC1_DAT3	0x31ef
+			VF610_PAD_PTA7__GPIO_134	0x219d
+		>;
+	};
+
+	pinctrl_fec0: fec0grp {
+		fsl,pins = <
+			VF610_PAD_PTC0__ENET_RMII0_MDC	0x30d2
+			VF610_PAD_PTC1__ENET_RMII0_MDIO	0x30d3
+			VF610_PAD_PTC2__ENET_RMII0_CRS	0x30d1
+			VF610_PAD_PTC3__ENET_RMII0_RXD1	0x30d1
+			VF610_PAD_PTC4__ENET_RMII0_RXD0	0x30d1
+			VF610_PAD_PTC5__ENET_RMII0_RXER	0x30d1
+			VF610_PAD_PTC6__ENET_RMII0_TXD1	0x30d2
+			VF610_PAD_PTC7__ENET_RMII0_TXD0	0x30d2
+			VF610_PAD_PTC8__ENET_RMII0_TXEN	0x30d2
+		>;
+	};
+
+	pinctrl_fec1: fec1grp {
+		fsl,pins = <
+			VF610_PAD_PTA6__RMII_CLKIN		0x30d1
+			VF610_PAD_PTC9__ENET_RMII1_MDC		0x30d2
+			VF610_PAD_PTC10__ENET_RMII1_MDIO	0x30d3
+			VF610_PAD_PTC11__ENET_RMII1_CRS		0x30d1
+			VF610_PAD_PTC12__ENET_RMII1_RXD1	0x30d1
+			VF610_PAD_PTC13__ENET_RMII1_RXD0	0x30d1
+			VF610_PAD_PTC14__ENET_RMII1_RXER	0x30d1
+			VF610_PAD_PTC15__ENET_RMII1_TXD1	0x30d2
+			VF610_PAD_PTC16__ENET_RMII1_TXD0	0x30d2
+			VF610_PAD_PTC17__ENET_RMII1_TXEN	0x30d2
+		>;
+	};
+
+	pinctrl_gpio_spi0: pinctrl-gpio-spi0 {
+		fsl,pins = <
+			VF610_PAD_PTB22__GPIO_44	0x33e2
+			VF610_PAD_PTB21__GPIO_43	0x33e2
+			VF610_PAD_PTB20__GPIO_42	0x33e1
+			VF610_PAD_PTB19__GPIO_41	0x33e2
+			VF610_PAD_PTB18__GPIO_40	0x33e2
+		>;
+	};
+
+	pinctrl_i2c_mux_reset: pinctrl-i2c-mux-reset {
+		fsl,pins = <
+			 VF610_PAD_PTE14__GPIO_119	0x31c2
+			 >;
+	};
+
+	pinctrl_i2c0: i2c0grp {
+		fsl,pins = <
+			VF610_PAD_PTB14__I2C0_SCL	0x37ff
+			VF610_PAD_PTB15__I2C0_SDA	0x37ff
+		>;
+	};
+
+	pinctrl_i2c0_gpio: i2c0grp-gpio {
+		fsl,pins = <
+			VF610_PAD_PTB14__GPIO_36	0x31c2
+			VF610_PAD_PTB15__GPIO_37	0x31c2
+		>;
+	};
+
+	
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			VF610_PAD_PTB16__I2C1_SCL	0x37ff
+			VF610_PAD_PTB17__I2C1_SDA	0x37ff
+		>;
+	};
+
+	pinctrl_i2c2: i2c2grp {
+		fsl,pins = <
+			VF610_PAD_PTA22__I2C2_SCL	0x37ff
+			VF610_PAD_PTA23__I2C2_SDA	0x37ff
+		>;
+	};
+
+	pinctrl_leds_debug: pinctrl-leds-debug {
+		fsl,pins = <
+			 VF610_PAD_PTD20__GPIO_74	0x31c2
+			 >;
+	};
+
+	pinctrl_qspi0: qspi0grp {
+		fsl,pins = <
+			VF610_PAD_PTD7__QSPI0_B_QSCK	0x31c3
+			VF610_PAD_PTD8__QSPI0_B_CS0	0x31ff
+			VF610_PAD_PTD9__QSPI0_B_DATA3	0x31c3
+			VF610_PAD_PTD10__QSPI0_B_DATA2	0x31c3
+			VF610_PAD_PTD11__QSPI0_B_DATA1	0x31c3
+			VF610_PAD_PTD12__QSPI0_B_DATA0	0x31c3
+		>;
+	};
+
+	pinctrl_uart0: uart0grp {
+		fsl,pins = <
+			VF610_PAD_PTB10__UART0_TX	0x21a2
+			VF610_PAD_PTB11__UART0_RX	0x21a1
+		>;
+	};
+
+	pinctrl_uart1: uart1grp {
+		fsl,pins = <
+			VF610_PAD_PTB23__UART1_TX	0x21a2
+			VF610_PAD_PTB24__UART1_RX	0x21a1
+		>;
+	};
+
+	pinctrl_uart2: uart2grp {
+		fsl,pins = <
+			VF610_PAD_PTD0__UART2_TX	0x21a2
+			VF610_PAD_PTD1__UART2_RX	0x21a1
+		>;
+	};
+
+	pinctrl_usb_vbus: pinctrl-usb-vbus {
+		fsl,pins = <
+			VF610_PAD_PTA16__GPIO_6	0x31c2
+		>;
+	};
+
+	pinctrl_usb0_host: usb0-host-grp {
+		fsl,pins = <
+			VF610_PAD_PTD6__GPIO_85		0x0062
+		>;
+	};
+};
-- 
2.5.5

^ permalink raw reply related

* [PATCH v3 3/3] ARM: dts: vf610-zii-dev-rev-b: Remove 'fixed-link' from DSA ports
From: Andrey Smirnov @ 2016-12-05 14:38 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, devicetree, andrew, Andrey Smirnov, Russell King,
	Stefan Agner, linux-kernel, Rob Herring, Sascha Hauer, Shawn Guo,
	cphealy
In-Reply-To: <1480948716-17744-1-git-send-email-andrew.smirnov@gmail.com>

Remove 'fixed-link' nodes from DSA ports since they are not needed (they
are not limiting link's speed and the ports will be configured to their
maximux speed as a default)

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v2:

	- None


 arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
index c0fc3f2..646c90c 100644
--- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
+++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts
@@ -97,10 +97,6 @@
 						phy-mode = "rgmii-txid";
 						link = <&switch1port6
 							&switch2port9>;
-						fixed-link {
-							speed = <1000>;
-							full-duplex;
-						};
 					};
 
 					port@6 {
@@ -165,10 +161,6 @@
 						label = "dsa";
 						phy-mode = "rgmii-txid";
 						link = <&switch0port5>;
-						fixed-link {
-							speed = <1000>;
-							full-duplex;
-						};
 					};
 				};
 				mdio {
-- 
2.5.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox