Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 2/4] dma: fsl-qdma: add devicetree documentation for qDMA driver.
From: Rob Herring @ 2017-12-20 18:43 UTC (permalink / raw)
  To: Wen He; +Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171219064157.29586-1-wen.he_1-3arQi8VN3Tc@public.gmane.org>

On Tue, Dec 19, 2017 at 02:41:57PM +0800, Wen He wrote:

Need a commit message.

> Signed-off-by: Wen He <wen.he_1-3arQi8VN3Tc@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/fsl-qdma.txt | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/fsl-qdma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/fsl-qdma.txt b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
> new file mode 100644
> index 000000000000..b076177b4863
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
> @@ -0,0 +1,42 @@
> +* Freescale queue Direct Memory Access Controller(qDMA) Controller
> +
> +  The qDMA controller transfers blocks of data between one source and one or more

Why the indentation?

> +destinations. The blocks of data transferred can be represented in memory as contiguous
> +or non-contiguous using scatter/gather table(s). Channel virtualization is supported
> +through enqueuing of DMA jobs to, or dequeuing DMA jobs from, different work
> +queues.
> +
> +* qDMA Controller
> +Required properties:
> +- compatible :

Add "Should be one of:"

> +	- "fsl,ls1021a-qdma",
> +	Or "fsl,ls1043a-qdma" followed by "fsl,ls1021a-qdma",

Then remove the "Or" and replace " followed by" with a comma (like dts 
source).

> +- reg : Specifies base physical address(s) and size of the qDMA registers.
> +	The region is qDMA control register's address and size.
> +- interrupts : A list of interrupt-specifiers, one for each entry in
> +	interrupt-names.
> +- interrupt-names : Should contain:
> +	"qdma-error" - the error interrupt
> +	"qdma-queue" - the queue interrupt
> +- channels : Number of channels supported by the controller

dma-channels is the standard name.

> +- queues : Number of queues supported by driver

Needs a vendor prefix.

> +
> +Optional properties:
> +- big-endian: If present registers and hardware scatter/gather descriptors
> +	of the qDMA are implemented in big endian mode, otherwise in little
> +	mode.
> +
> +
> +Examples:
> +
> +	qdma: qdma@8390000 {

Use standard node names:

dma-controller@...

> +		compatible = "fsl,ls1021a-qdma";
> +		reg = <0x0 0x8398000 0x0 0x2000 /* Controller registers */
> +		       0x0 0x839a000 0x0 0x2000>; /* Block registers */
> +		interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>,
> +				<GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "qdma-error", "qdma-queue";
> +		channels = <8>;
> +		queues = <2>;
> +		big-endian;
> +	};
> -- 
> 2.14.1
> 
> --
> 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
--
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 v2] Input: mms114 - add support for mms152
From: Dmitry Torokhov @ 2017-12-20 18:34 UTC (permalink / raw)
  To: Simon Shields; +Cc: linux-input@vger.kernel.org, devicetree, Andi Shyti
In-Reply-To: <20171220153810.1961-1-simon@lineageos.org>

[resending as plain text]

Hi Simon,

On Wed, Dec 20, 2017 at 9:38 AM, Simon Shields <simon@lineageos.org> wrote:
> MMS152 has no configuration registers, but the packet format used in
> interrupts is identical to mms114.

Since there seems to be renewed interest in mms114 driver, I would like for
it to get a facelift and switch to using the standard touchscreen
properties rather than the custom ones (we still need to keep compatibility
though). Also, we do not seem to have any in-tree users of the
mms114_platform_data structure. Can you please see if you  can:

- switch the driver to use the generic device properties
(device_property_*() API)
- use touchscreen_parse_properties() to parse the standard touchscreen
properties
- use touchscreen_report_pos() to report coordinates
- drop mms114_platform_data

Thanks!

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/6] dt-bindings: soc: qcom: Add label for GLINK bindings
From: Rob Herring @ 2017-12-20 18:30 UTC (permalink / raw)
  To: Chris Lew
  Cc: bjorn.andersson, andy.gross, david.brown, aneela, linux-arm-msm,
	linux-remoteproc, linux-soc, devicetree, linux-kernel
In-Reply-To: <1513634534-22861-2-git-send-email-clew@codeaurora.org>

On Mon, Dec 18, 2017 at 02:02:09PM -0800, Chris Lew wrote:
> Add a label property to identify the edge this node represents.

Why does a user need to know this?

> 
> Signed-off-by: Chris Lew <clew@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> index 9663cab52246..0b8cc533ca83 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,glink.txt
> @@ -10,6 +10,11 @@ edge.
>  	Value type: <stringlist>
>  	Definition: must be "qcom,glink-rpm"
>  
> +- label:
> +	Usage: optional
> +	Value type: <string>
> +	Definition: should specify the subsystem name this edge corresponds to.
> +
>  - interrupts:
>  	Usage: required
>  	Value type: <prop-encoded-array>
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply

* Re: [PATCH v5 4/6] dt: bindings: lp8860: Add trigger binding to the lp8860
From: Rob Herring @ 2017-12-20 18:29 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <20171218202307.4913-4-dmurphy@ti.com>

On Mon, Dec 18, 2017 at 02:23:05PM -0600, Dan Murphy wrote:
> Add a default trigger optional node to the child node.
> This will allow the driver to set the trigger for a backlight.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v5 - No changes
> 
> v4 - No changes
> v3 - Removed optional and rebased - https://patchwork.kernel.org/patch/10093755/
> v2 - Moved binding changes to first patch in the series.
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v5 2/6] dt: bindings: lp8860: Update DT label binding
From: Rob Herring @ 2017-12-20 18:28 UTC (permalink / raw)
  To: Dan Murphy
  Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree,
	linux-kernel, linux-leds
In-Reply-To: <20171218202307.4913-2-dmurphy@ti.com>

On Mon, Dec 18, 2017 at 02:23:03PM -0600, Dan Murphy wrote:
> Update the lp8860 label binding to the LED
> standard as documented in
> 
> Documentation/devicetree/bindings/leds/common.txt
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v5 - Renamed label to just white:backlight - https://patchwork.kernel.org/patch/10108361
> Comment was made on patch 4 https://patchwork.kernel.org/patch/10108351/
> 
> v4 - No changes
> v3 - Added address and size cells, updated label with color and inserted spaces
> around the reg node - https://patchwork.kernel.org/patch/10093749/
> v2 - Added reg to child node and made it required
> 
>  Documentation/devicetree/bindings/leds/leds-lp8860.txt | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v6] i2c: Add support for NXP PCA984x family.
From: Rob Herring @ 2017-12-20 18:27 UTC (permalink / raw)
  To: Adrian Fiergolski; +Cc: linux-i2c, peda, devicetree
In-Reply-To: <20171218174506.20543-1-adrian.fiergolski@cern.ch>

On Mon, Dec 18, 2017 at 06:45:06PM +0100, Adrian Fiergolski wrote:
> This patch extends the current i2c-mux-pca954x driver and adds support for
> a newer PCA984x family of the I2C switches and multiplexers from NXP.
> 
> Signed-off-by: Adrian Fiergolski <adrian.fiergolski@cern.ch>
> ---
>  .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  5 ++-
>  drivers/i2c/muxes/Kconfig                          |  6 ++--
>  drivers/i2c/muxes/i2c-mux-pca954x.c                | 38 +++++++++++++++++++---
>  3 files changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index aa097045a10e..b428bc0d81b1 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -1,10 +1,13 @@
>  * NXP PCA954x I2C bus switch
>  
> +The driver supports NXP PCA954x and PCA984x I2C mux/switch devices.
> +
>  Required Properties:
>  
>    - compatible: Must contain one of the following.
>      "nxp,pca9540", "nxp,pca9542", "nxp,pca9543", "nxp,pca9544",
> -    "nxp,pca9545", "nxp,pca9546", "nxp,pca9547", "nxp,pca9548"
> +    "nxp,pca9545", "nxp,pca9546", "nxp,pca9547", "nxp,pca9548",
> +    "nxp,pca9846", "nxp,pca9847", "nxp,pca9848", "nxp,pca9849"

The format here should be 1 valid combination of compatibles per line. 
So it should be clear what are valid fallbacks as Peter suggested.

>  
>    - reg: The I2C address of the device.
>  

^ permalink raw reply

* Re: [PATCH V2 4/9] devicetree: bindings: stm32: add support of STM32MP157
From: Rob Herring @ 2017-12-20 18:24 UTC (permalink / raw)
  To: Ludovic Barre
  Cc: Russell King, Linus Walleij, Arnd Bergmann, Maxime Coquelin,
	Alexandre Torgue, Gerald Baeza,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1513610272-7824-5-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>

On Mon, Dec 18, 2017 at 04:17:47PM +0100, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> 
> This patch adds STM32MP157 SoC bindings.
> 
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/stm32.txt | 1 +
>  1 file changed, 1 insertion(+)

"dt-bindings: ..." is the preferred subject prefix. Otherwise,

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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] touchscreen: mms114: add support for mms152
From: Rob Herring @ 2017-12-20 18:21 UTC (permalink / raw)
  To: Simon Shields
  Cc: Dmitry Torokhov, linux-input-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171218124933.1803-1-simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>

On Mon, Dec 18, 2017 at 11:49:33PM +1100, Simon Shields wrote:
> MMS152 has no configuration registers, but the packet format used in
> interrupts is identical to mms114.
> 
> Signed-off-by: Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
> ---
>  .../bindings/input/touchscreen/mms114.txt          |  8 ++--
>  drivers/input/touchscreen/mms114.c                 | 55 +++++++++++++++++++---
>  include/linux/platform_data/mms114.h               |  6 +++
>  3 files changed, 58 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> index 89d4c56c5671..733411020ced 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> @@ -1,15 +1,15 @@
> -* MELFAS MMS114 touchscreen controller
> +* MELFAS MMS114/MMS152 touchscreen controller
>  
>  Required properties:
> -- compatible: must be "melfas,mms114"
> +- compatible: "melfas,mms114" for MMS114, or "melfas,mms152" for MMS152

Please reformat to 1 per line.

>  - reg: I2C address of the chip
>  - interrupts: interrupt to which the chip is connected
>  - x-size: horizontal resolution of touchscreen
>  - y-size: vertical resolution of touchscreen
>  
>  Optional properties:
> -- contact-threshold:
> -- moving-threshold:
> +- contact-threshold: only with "melfas,mms114"
> +- moving-threshold: only with "melfas,mms114"
>  - x-invert: invert X axis
>  - y-invert: invert Y axis
>  
--
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 2/4] ARM: dts: split trats2 DTS in preparation for midas boards
From: Rob Herring @ 2017-12-20 18:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Simon Shields, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Kukjin Kim, devicetree-u79uwXL29TY76Z2rM5mHXA, Marek Szyprowski,
	Bartłomiej Żołnierkiewicz
In-Reply-To: <CAJKOXPdzAvsaAOo6P+UA8VWYd7YjmtSqtkyKtNaz7jNdUXUH4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Dec 20, 2017 at 03:05:18PM +0100, Krzysztof Kozlowski wrote:
> On Mon, Dec 18, 2017 at 1:38 PM, Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org> wrote:
> > The midas boards share a lot with trats2. Split the common parts
> > out of trats2 into a common midas dtsi and a common "galaxy s3" dts.
> >
> > Signed-off-by: Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi        |  145 ++
> >  ...exynos4412-trats2.dts => exynos4412-midas.dtsi} |  117 +-
> >  arch/arm/boot/dts/exynos4412-trats2.dts            | 1446 +-------------------
> >  3 files changed, 184 insertions(+), 1524 deletions(-)
> >  create mode 100644 arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> >  copy arch/arm/boot/dts/{exynos4412-trats2.dts => exynos4412-midas.dtsi} (92%)
> >  rewrite arch/arm/boot/dts/exynos4412-trats2.dts (97%)
> >
> > diff --git a/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > new file mode 100644
> > index 000000000000..2806236484a6
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/exynos4412-galaxy-s3.dtsi
> > @@ -0,0 +1,145 @@
> > +/*
> > + * Samsung's Exynos4412 based Galaxy S3 board device tree source
> > + *
> > + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> > + *             http://www.samsung.com
> > + *
> > + * Device tree source file for Samsung's Galaxy S3 boards which are based on
> > + * Samsung's Exynos4412 SoC.
> > + *
> > + * 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.
> > + */
> 
> One new line to make it consistent with others.

If you're going to change it, use an SPDX tag.

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 v3 1/4] dt-bindings: samsung: document bindings for Midas family boards
From: Rob Herring @ 2017-12-20 18:17 UTC (permalink / raw)
  To: Simon Shields
  Cc: linux-samsung-soc, Kukjin Kim, Krzysztof Kozlowski, devicetree,
	Marek Szyprowski, Bartłomiej Żołnierkiewicz
In-Reply-To: <20171218123805.26345-2-simon@lineageos.org>

On Mon, Dec 18, 2017 at 11:38:02PM +1100, Simon Shields wrote:
> Document GT-I9300, GT-I9305, GT-N7100, and GT-N7105 bindings, along
> with the shared "midas" binding.
> 
> Signed-off-by: Simon Shields <simon@lineageos.org>
> ---
>  Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt | 4 ++++
>  1 file changed, 4 insertions(+)

My comment on v2 remains.

^ permalink raw reply

* Re: [PATCH v4 11/16] dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
From: Rob Herring @ 2017-12-20 18:16 UTC (permalink / raw)
  To: Jacob Chen
  Cc: linux-rockchip, linux-kernel, linux-arm-kernel, mchehab,
	linux-media, sakari.ailus, hans.verkuil, tfiga, zhengsq,
	laurent.pinchart, zyc, eddie.cai.linux, jeffy.chen, allon.huang,
	devicetree, heiko, Joao.Pinto, Luis.Oliveira, Jose.Abreu,
	Jacob Chen
In-Reply-To: <20171218121445.6086-8-jacob-chen@iotwrt.com>

On Mon, Dec 18, 2017 at 08:14:40PM +0800, Jacob Chen wrote:
> From: Jacob Chen <jacob2.chen@rock-chips.com>
> 
> Add DT bindings documentation for Rockchip MIPI D-PHY RX
> 
> Signed-off-by: Jacob Chen <jacob2.chen@rock-chips.com>
> ---
>  .../bindings/media/rockchip-mipi-dphy.txt          | 88 ++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v4 10/16] dt-bindings: Document the Rockchip ISP1 bindings
From: Rob Herring @ 2017-12-20 18:14 UTC (permalink / raw)
  To: Jacob Chen
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
	hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w, tfiga-F7+t8E8rja9g9hUCZPvPmw,
	zhengsq-TNX95d0MmH7DzftRWevZcw,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	zyc-TNX95d0MmH7DzftRWevZcw,
	eddie.cai.linux-Re5JQEeQqe8AvxtiuMwx3w,
	jeffy.chen-TNX95d0MmH7DzftRWevZcw,
	allon.huang-TNX95d0MmH7DzftRWevZcw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, heiko-4mtYJXux2i+zQB+pC5nmwQ,
	Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
	Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w,
	Jose.Abreu-HKixBCOQz3hWk0Htik3J/w, Jacob Chen
In-Reply-To: <20171218121445.6086-7-jacob-chen-fyOeoxGR3m/QT0dZR+AlfA@public.gmane.org>

On Mon, Dec 18, 2017 at 08:14:39PM +0800, Jacob Chen wrote:
> From: Jacob Chen <jacob2.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> Add DT bindings documentation for Rockchip ISP1
> 
> Signed-off-by: Jacob Chen <jacob2.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>  .../devicetree/bindings/media/rockchip-isp1.txt    | 69 ++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

--
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 v4 1/4] dt-bindings: rtc: add bindings for i.MX53 SRTC
From: Rob Herring @ 2017-12-20 18:13 UTC (permalink / raw)
  To: linux-kernel-dev-QonKdJ6Bx35Wk0Htik3J/w
  Cc: Patrick Bruenn, Alessandro Zummo, Alexandre Belloni, Mark Rutland,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, Fabio Estevam, Juergen Borleis, Noel Vellemans,
	Shawn Guo, Sascha Hauer, Russell King,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Philippe Ombredanne
In-Reply-To: <20171218115133.16371-2-linux-kernel-dev-QonKdJ6Bx35Wk0Htik3J/w@public.gmane.org>

On Mon, Dec 18, 2017 at 12:51:30PM +0100, linux-kernel-dev-QonKdJ6Bx35Wk0Htik3J/w@public.gmane.org wrote:
> From: Patrick Bruenn <p.bruenn-QonKdJ6Bx35Wk0Htik3J/w@public.gmane.org>
> 
> Document the binding for i.MX53 SRTC implemented by rtc-mxc_v2
> 
> Signed-off-by: Patrick Bruenn <p.bruenn-QonKdJ6Bx35Wk0Htik3J/w@public.gmane.org>

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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 1/9] dt-bindings: ti-sysc: Update binding for timers and capabilities
From: Rob Herring @ 2017-12-20 18:10 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-omap, Nishanth Menon, devicetree, Paul Walmsley,
	Dave Gerlach, Tomi Valkeinen, Matthijs van Duin, linux-kernel,
	Liam Girdwood, Tero Kristo, Mark Brown, Sakari Ailus,
	Laurent Pinchart, Benoît Cousson, Mark Rutland,
	Mauro Carvalho Chehab, linux-arm-kernel
In-Reply-To: <20171216192222.GH3875@atomide.com>

On Sat, Dec 16, 2017 at 11:22:22AM -0800, Tony Lindgren wrote:
> * Rob Herring <robh@kernel.org> [171216 18:33]:
> > >  Optional properties:
> > >  
> > > +- ti,sysc-mask	shall contain mask of supported register bits for the
> > > +		SYSCONFIG register as documented in the Technical Reference
> > > +		Manual (TRM) for the interconnect target module
> > > +
> > > +- ti,sysc-midle	list of master idle modes supported by the interconnect
> > > +		target module as documented in the TRM for SYSCONFIG
> > > +		register MIDLEMODE bits
> > > +
> > > +- ti,sysc-sidle	list of slave idle modes supported by the interconnect
> > > +		target module as documented in the TRM for SYSCONFIG
> > > +		register SIDLEMODE bits
> > > +
> > > +- ti,sysc-delay-us	delay needed after OCP softreset before accssing
> > > +			SYSCONFIG register again
> > > +
> > > +- ti,syss-mask	optional mask of reset done status bits as described in the
> > > +		TRM for SYSSTATUS registers, typically 1 with some devices
> > > +		having separate reset done bits for children like OHCI and
> > > +		EHCI
> > > +
> > 
> > Seems like a lot of this should be implied by specific compatible 
> > strings.
> 
> Unfortunately that would still explode the permutations to almost
> one compatible per module especially for types "ti,sysc-omap2" and
> "ti,sysc-omap4". And the features and idle modes supported by the
> module are all over the place for "ti,sysc-mask", "ti,sysc-midle",
> "ti,sysc-sidle" and "ti,syss-mask"..

Okay.

> I was planning to have "ti,sysc-delay-us" only in the driver, but
> the same IP needs it set on dm814x while not on omap4 for OTG
> for example. I could add SoC specific quirks to the driver
> for that one if you prefer that instead?

No, I don't have a preference.

> I do have a patch also I'm testing to use the revision register
> value for handling further quirks, but unfortunately that
> register is not populated or updated for many modules. And it's
> only usable after the module is already configured to accessible :)
> 
> > Are the bits you've defined all of them or there's more?
> 
> That's it, with this binding I've allocated the data from dts
> for the tests I've done. So that should allow us to replace the
> static data to start with as seen with the following command:
> 
> $ git grep -A10 "struct omap_hwmod_class_sysconfig" \
> 	arch/arm/*hwmod*data*.c
> ...
> 
> So that's to configure a big pile of different module
> configurations we currently have as can be seen with:
> 
> $ git grep "struct omap_hwmod_class_sysconfig" \
> 	arch/arm/*hwmod*data*.c | wc -l
> 194
> 
> I'm sure there's still few duplicates there though..

Okay, then I guess I'm okay with this.

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 5/7] dt-bindings: i3c: Document core bindings
From: Rob Herring @ 2017-12-20 18:06 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Wolfram Sang, linux-i2c, Jonathan Corbet, linux-doc,
	Greg Kroah-Hartman, Arnd Bergmann, Przemyslaw Sroka,
	Arkadiusz Golec, Alan Douglas, Bartosz Folta, Damian Kos,
	Alicja Jurasik-Urbaniak, Cyprian Wronka, Suresh Punnoose,
	Thomas Petazzoni, Nishanth Menon, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree
In-Reply-To: <20171216193537.0aef2e97@bbrezillon>

On Sat, Dec 16, 2017 at 07:35:37PM +0100, Boris Brezillon wrote:
> On Sat, 16 Dec 2017 11:20:40 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Thu, Dec 14, 2017 at 04:16:08PM +0100, Boris Brezillon wrote:
> > > A new I3C subsystem has been added and a generic description has been
> > > created to represent the I3C bus and the devices connected on it.
> > > 
> > > Document this generic representation.

[...]

> > So please define the node 
> > name to be "i3c-controller". That's more inline with other node names 
> > than i3c-master that you used below.
> 
> Hm, not sure i3c-controller is appropriate though, because you can have
> slave controllers. Maybe i3c-host, but I'd prefer to keep the term
> master since it's employed everywhere in the spec. I can also be
> i3c-master-controller if you prefer.

Okay, i3c-master is fine. Just make it explicit.

> > > +I3C devices
> > > +===========
> > > +
> > > +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
> > > +are thus discoverable. So, by default, I3C devices do not have to be described
> > > +in the device tree.
> > > +This being said, one might want to attach extra resources to these devices,
> > > +and those resources may have to be described in the device tree, which in turn
> > > +means we have to describe I3C devices.
> > > +
> > > +Another use case for describing an I3C device in the device tree is when this
> > > +I3C device has a static address and we want to assign it a specific dynamic
> > > +address before the DAA takes place (so that other devices on the bus can't
> > > +take this dynamic address).
> > > +
> > > +Required properties
> > > +-------------------
> > > +- i3c-pid: PID (Provisional ID). 64-bit property which is used to match a
> > > +	   device discovered during DAA with its device tree definition. The
> > > +	   PID is supposed to be unique on a given bus, which guarantees a 1:1
> > > +	   match. This property becomes optional if a reg property is defined,
> > > +	   meaning that the device has a static address.  
> > 
> > What determines this number?
> 
> Part of it is fixed (manufacturer and part id) and the last few bits
> represent the device instance on the bus (so you can have several
> identical devices on the same bus). The manufacturer and part ids
> should be statically assigned during production, instance id is usually
> configurable through extra pins that you drive high or low at reset
> time.

Sounds like an I2C address at least for the pin strapping part...

> > > +
> > > +Optional properties
> > > +-------------------
> > > +- reg: static address. Only valid is the device has a static address.
> > > +- i3c-dynamic-address: dynamic address to be assigned to this device. This
> > > +		       property depends on the reg property.  
> > 
> > Perhaps "assigned-address" property would be appropriate. I'm not all 
> > that familiar with it though.
> 
> Again, the spec use the term "dynamic address" everywhere, and I'd like
> to stay as close as possible to the spec.

I looked at assigned-addresses a bit more and that won't really fit 
because it should be the same format as reg. So I think reg should 
always be the PID as that is fixed and always present. Then the DAA 
address is separate and can be the i3c-dynamic-address property.

However, there's still part I don't understand...

> > > +		/* I3C device with a static address. */
> > > +		thermal_sensor: sensor@68 {
> > > +			reg = <0x68>;
> > > +			i3c-dynamic-address = <0xa>;

I'm confused as to how/why you have both reg and dynamic address?

> > > +		};
> > > +
> > > +		/*
> > > +		 * I3C device without a static address but requiring resources
> > > +		 * described in the DT.
> > > +		 */
> > > +		sensor2 {  
> > 
> > It's not great that we can't follow using generic node names. Maybe the 
> > PID can be used as the address? In USB for example, we use hub ports for 
> > DT addresses rather than USB addresses since those are dynamic.
> 
> Hm, the problem is, we may have 2 numbering schemes here: one where reg
> is used (reg representing the I2C/static address), and another one
> where the PID is used.
> If you're okay with mixing those 2 schemes, then I'm fine with that too.

Mixing I2C devices and I3C devices, yes. But you need to mix in a single 
device? IOW, do I3C devices also have an I2C address?

Rob

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: Add memory node unit name
From: Fabio Estevam @ 2017-12-20 18:05 UTC (permalink / raw)
  To: Marco Franchi
  Cc: Shawn Guo, Rob Herring, linux-arm-kernel, linux-kernel,
	devicetree
In-Reply-To: <1512575989-15627-1-git-send-email-marcofrk@gmail.com>

On Wed, Dec 6, 2017 at 1:59 PM, Marco Franchi <marcofrk@gmail.com> wrote:
> Fix the following warnings from dtc by adding the unit name to memory
> nodes:
>
> Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
>
> Converted using the following command:
>
> perl -p0777i -e 's/memory \{\n\t\treg = \<0x+([0-9a-f])/memory\@$1$\0000000 \{\n\t\treg = <0x$1/m' `find ./arch/arm/boot/dts -name "imx*"`
>
> The files below were manually fixed:
> -imx1-ads.dts
> -imx1-apf9328.dts
>
> Signed-off-by: Marco Franchi <marcofrk@gmail.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply

* Re: [PATCH v7 5/6] arm: dts: mt2712: Add clock controller device nodes
From: Matthias Brugger @ 2017-12-20 18:03 UTC (permalink / raw)
  To: Weiyi Lu, Stephen Boyd, Mike Turquette, Rob Herring
  Cc: James Liao, Fan Chen, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-clk, srv_heupstream
In-Reply-To: <1511854102-23195-7-git-send-email-weiyi.lu@mediatek.com>



On 11/28/2017 08:28 AM, Weiyi Lu wrote:
> Add clock controller nodes for MT2712, include topckgen, infracfg,
> pericfg, mcucfg and apmixedsys. This patch also add six oscillators that
> provide clocks for MT2712.
> 
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 115 ++++++++++++++++++++++++++++++
>  1 file changed, 115 insertions(+)

I fixed the subject line for you, but the next time please take care to start
the line with "arm64" instead of "arm"

Thanks,
Matthias

> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> index 5d4e406..5703793 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> @@ -5,6 +5,7 @@
>   * SPDX-License-Identifier: (GPL-2.0 OR MIT)
>   */
>  
> +#include <dt-bindings/clock/mt2712-clk.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
> @@ -98,6 +99,48 @@
>  		#clock-cells = <0>;
>  	};
>  
> +	clk26m: oscillator@0 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <26000000>;
> +		clock-output-names = "clk26m";
> +	};
> +
> +	clk32k: oscillator@1 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "clk32k";
> +	};
> +
> +	clkfpc: oscillator@2 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <50000000>;
> +		clock-output-names = "clkfpc";
> +	};
> +
> +	clkaud_ext_i_0: oscillator@3 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <6500000>;
> +		clock-output-names = "clkaud_ext_i_0";
> +	};
> +
> +	clkaud_ext_i_1: oscillator@4 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <196608000>;
> +		clock-output-names = "clkaud_ext_i_1";
> +	};
> +
> +	clkaud_ext_i_2: oscillator@5 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <180633600>;
> +		clock-output-names = "clkaud_ext_i_2";
> +	};
> +
>  	timer {
>  		compatible = "arm,armv8-timer";
>  		interrupt-parent = <&gic>;
> @@ -111,6 +154,24 @@
>  			      (GIC_CPU_MASK_RAW(0x13) | IRQ_TYPE_LEVEL_LOW)>;
>  	};
>  
> +	topckgen: syscon@10000000 {
> +		compatible = "mediatek,mt2712-topckgen", "syscon";
> +		reg = <0 0x10000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	infracfg: syscon@10001000 {
> +		compatible = "mediatek,mt2712-infracfg", "syscon";
> +		reg = <0 0x10001000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	pericfg: syscon@10003000 {
> +		compatible = "mediatek,mt2712-pericfg", "syscon";
> +		reg = <0 0x10003000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
>  	uart5: serial@1000f000 {
>  		compatible = "mediatek,mt2712-uart",
>  			     "mediatek,mt6577-uart";
> @@ -121,6 +182,18 @@
>  		status = "disabled";
>  	};
>  
> +	apmixedsys: syscon@10209000 {
> +		compatible = "mediatek,mt2712-apmixedsys", "syscon";
> +		reg = <0 0x10209000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	mcucfg: syscon@10220000 {
> +		compatible = "mediatek,mt2712-mcucfg", "syscon";
> +		reg = <0 0x10220000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
>  	sysirq: interrupt-controller@10220a80 {
>  		compatible = "mediatek,mt2712-sysirq",
>  			     "mediatek,mt6577-sysirq";
> @@ -192,5 +265,47 @@
>  		clock-names = "baud", "bus";
>  		status = "disabled";
>  	};
> +
> +	mfgcfg: syscon@13000000 {
> +		compatible = "mediatek,mt2712-mfgcfg", "syscon";
> +		reg = <0 0x13000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	mmsys: syscon@14000000 {
> +		compatible = "mediatek,mt2712-mmsys", "syscon";
> +		reg = <0 0x14000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	imgsys: syscon@15000000 {
> +		compatible = "mediatek,mt2712-imgsys", "syscon";
> +		reg = <0 0x15000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	bdpsys: syscon@15010000 {
> +		compatible = "mediatek,mt2712-bdpsys", "syscon";
> +		reg = <0 0x15010000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	vdecsys: syscon@16000000 {
> +		compatible = "mediatek,mt2712-vdecsys", "syscon";
> +		reg = <0 0x16000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	vencsys: syscon@18000000 {
> +		compatible = "mediatek,mt2712-vencsys", "syscon";
> +		reg = <0 0x18000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	jpgdecsys: syscon@19000000 {
> +		compatible = "mediatek,mt2712-jpgdecsys", "syscon";
> +		reg = <0 0x19000000 0 0x1000>;
> +		#clock-cells = <1>;
> +	};
>  };
>  
> 

^ permalink raw reply

* Re: [PATCH v7 0/6] Mediatek MT2712 clock and scpsys support
From: Matthias Brugger @ 2017-12-20 18:02 UTC (permalink / raw)
  To: Weiyi Lu
  Cc: Stephen Boyd, Mike Turquette, Rob Herring, James Liao, Fan Chen,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek,
	linux-clk, srv_heupstream
In-Reply-To: <1513317052.30745.1.camel@mtksdaap41>



On 12/15/2017 06:50 AM, Weiyi Lu wrote:
> On Tue, 2017-11-28 at 15:28 +0800, Weiyi Lu wrote:
> 
> Hi Matthias,
> Just gentle ping. Many thanks.
> 

Now pushed to v4.15-next thanks


>> This series is based on v4.15-rc1 and composed of
>> scpsys control (PATCH 1-4) and device tree (PATCH 5-6)
>>
>> changes since v6:
>> - Rebase to v4.15-rc1.
>>
>> changes since v5:
>> - Refine bus protection with proper variable name
>>   and better implementation for the if statement.
>>
>> changes since v4:
>> - Refine scpsys and infracfg for bus protection by passing
>>   a boolean flag to determine the register update method
>>
>> changes since v3:
>> - Rebase to v4.14-rc1.
>>
>> changes since v2:
>> - ensure the clocks used by clocksource driver are registered
>>   before clocksource init() by using CLK_OF_DECLARE()
>> - correct the frequency of clk32k/clkrtc_ext/clkrtc_int
>>
>> changes since v1:
>> - Rebase to v4.13-next-soc.
>> - Refine scpsys and infracfg for bus protection.
>>
>> *** BLURB HERE ***
>>
>> Weiyi Lu (6):
>>   dt-bindings: soc: add MT2712 power dt-bindings
>>   soc: mediatek: extend bus protection API
>>   soc: mediatek: add dependent clock jpgdec/audio for scpsys
>>   soc: mediatek: add MT2712 scpsys support
>>   arm: dts: mt2712: Add clock controller device nodes
>>   arm: dts: Add power controller device node of MT2712
>>
>>  .../devicetree/bindings/soc/mediatek/scpsys.txt    |   3 +
>>  arch/arm64/boot/dts/mediatek/mt2712e.dtsi          | 131 +++++++++++++++++++
>>  drivers/soc/mediatek/mtk-infracfg.c                |  26 +++-
>>  drivers/soc/mediatek/mtk-scpsys.c                  | 140 ++++++++++++++++++---
>>  include/dt-bindings/power/mt2712-power.h           |  26 ++++
>>  include/linux/soc/mediatek/infracfg.h              |   7 +-
>>  6 files changed, 311 insertions(+), 22 deletions(-)
>>  create mode 100644 include/dt-bindings/power/mt2712-power.h
>>
> 
> 

^ permalink raw reply

* Re: [PATCH] devicetree: Add video bus switch
From: Laurent Pinchart @ 2017-12-20 17:54 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Sakari Ailus, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	sre-DgEjT+Ai2ygdnm+yROfE0A, pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
	linux-media-u79uwXL29TY76Z2rM5mHXA, galak-sgV2jX0FEOL9JmXXK+q4OQ,
	mchehab-JPH+aEBZ4P+UEJcrhfAQsw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170204215610.GA9243@amd>

Hi Pavel,

On Saturday, 4 February 2017 23:56:10 EET Pavel Machek wrote:
> Hi!
> 
> >>>> +Required properties
> >>>> +===================
> >>>> +
> >>>> +compatible	: must contain "video-bus-switch"
> >>> 
> >>> How generic is this? Should we have e.g. nokia,video-bus-switch? And
> >>> if so, change the file name accordingly.
> >> 
> >> Generic for "single GPIO controls the switch", AFAICT. But that should
> >> be common enough...
> > 
> > Um, yes. Then... how about: video-bus-switch-gpio? No Nokia prefix.
> 
> Ok, done. I also fixed the english a bit.
> 
> >>>> +reg		: The interface:
> >>>> +		  0 - port for image signal processor
> >>>> +		  1 - port for first camera sensor
> >>>> +		  2 - port for second camera sensor
> >>> 
> >>> I'd say this must be pretty much specific to the one in N900. You
> >>> could have more ports. Or you could say that ports beyond 0 are
> >>> camera sensors. I guess this is good enough for now though, it can be
> >>> changed later on with the source if a need arises.
> >> 
> >> Well, I'd say that selecting between two sensors is going to be the
> >> common case. If someone needs more than two, it will no longer be
> >> simple GPIO, so we'll have some fixing to do.
> > 
> > It could be two GPIOs --- that's how the GPIO I2C mux works.
> > 
> > But I'd be surprised if someone ever uses something like that
> > again. ;-)
> 
> I'd say.. lets handle that when we see hardware like that.
> 
> >>> Btw. was it still considered a problem that the endpoint properties
> >>> for the sensors can be different? With the g_routing() pad op which is
> >>> to be added, the ISP driver (should actually go to a framework
> >>> somewhere) could parse the graph and find the proper endpoint there.
> >> 
> >> I don't know about g_routing. I added g_endpoint_config method that
> >> passes the configuration, and that seems to work for me.
> >> 
> >> I don't see g_routing in next-20170201 . Is there place to look?
> > 
> > I think there was a patch by Laurent to LMML quite some time ago. I
> > suppose that set will be repicked soonish.
> > 
> > I don't really object using g_endpoint_config() as a temporary solution;
> > I'd like to have Laurent's opinion on that though. Another option is to
> > wait, but we've already waited a looong time (as in total).
> 
> Laurent, do you have some input here? We have simple "2 cameras
> connected to one signal processor" situation here. We need some way of
> passing endpoint configuration from the sensors through the switch. I
> did this:

Could you give me a bit more information about the platform you're targeting: 
how the switch is connected, what kind of switch it is, and what endpoint 
configuration data you need ?

> >> @@ -415,6 +416,8 @@ struct v4l2_subdev_video_ops {
> >>                          const struct v4l2_mbus_config *cfg);
> >>     int (*s_rx_buffer)(struct v4l2_subdev *sd, void *buf,
> >>                        unsigned int *size);
> >> +   int (*g_endpoint_config)(struct v4l2_subdev *sd,
> >> +                       struct v4l2_of_endpoint *cfg);
> 
> Google of g_routing tells me:
> 
> 9) Highly reconfigurable hardware - Julien Beraud
> 
> - 44 sub-devices connected with an interconnect.
> - As long as formats match, any sub-device could be connected to any
> - other sub-device through a link.
> - The result is 44 * 44 links at worst.
> - A switch sub-device proposed as the solution to model the
> - interconnect. The sub-devices are connected to the switch
> - sub-devices through the hardware links that connect to the
> - interconnect.
> - The switch would be controlled through new IOCTLs S_ROUTING and
> - G_ROUTING.
> - Patches available:
>  http://git.linuxtv.org/cgit.cgi/pinchartl/media.git/log/?h=xilinx-wip
> 
> but the patches are from 2005. So I guess I'll need some guidance here...

You made me feel very old for a moment. The patches are from 2015 :-)

> > I'll reply to the other patch containing the code.

-- 
Regards,

Laurent Pinchart

--
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 v6 00/18] dwc MSI fixes, ARTPEC-6 EP mode support, ARTPEC-7 SoC support
From: Lorenzo Pieralisi @ 2017-12-20 17:34 UTC (permalink / raw)
  To: Niklas Cassel, Jingoo Han, Joao Pinto
  Cc: linux-arm-kernel, linux-pci, linux-omap, Niklas Cassel,
	devicetree, linux-kernel
In-Reply-To: <20171219232940.659-1-niklas.cassel@axis.com>

On Wed, Dec 20, 2017 at 12:29:21AM +0100, Niklas Cassel wrote:
> This is a series that adds:
> - PCI endpoint mode support in the ARTPEC-6 driver.
> - ARTPEC-7 SoC support in the ARTPEC-6 driver (the SoCs are very similar).
> - Small fixes for MSI in designware-ep and designware-host,
>   needed to get endpoint mode support working for ARTPEC-6.
> - Cleanups in pci-dra7xx to better prepare for endpoint mode in other
>   DWC based PCIe drivers.

Joao, Jingoo,

Gustavo tested the series and Kishon ACK'ed the relevant patches,
I need your ACKs on designware patches to queue this series for
v4.16.

I am away from tomorrow (noon) till beginning of January which means
that either I queue this series tomorrow or at -rc6, please do
chime in if you can.

Thanks,
Lorenzo

> Changes since V5:
> -Dropped GFP_DMA32 from "PCI: dwc: Use the DMA-API to get the MSI address"
>  so that we use the exact same GFP flags as before.
> -Rewrote commit message for "PCI: dwc: Make cpu_addr_fixup take struct
> dw_pcie as argument" to be more detailed.
> 
> Niklas Cassel (18):
>   PCI: dwc: Use the DMA-API to get the MSI address
>   PCI: designware-ep: dw_pcie_ep_set_msi() should only set MMC bits
>   PCI: designware-ep: Read-only registers need DBI_RO_WR_EN to be
>     writable
>   PCI: designware-ep: Pre-allocate memory for MSI in dw_pcie_ep_init
>   PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()
>   PCI: designware-ep: Add generic function for raising MSI irq
>   PCI: dwc: dra7xx: Refactor Kconfig and Makefile handling for host/ep
>     mode
>   PCI: dwc: dra7xx: Assign pp->ops in dra7xx_add_pcie_port() rather than
>     in probe
>   PCI: dwc: dra7xx: Help compiler to remove unused code
>   PCI: dwc: artpec6: Remove unused defines
>   PCI: dwc: artpec6: Use BIT and GENMASK macros
>   PCI: dwc: artpec6: Split artpec6_pcie_establish_link() into smaller
>     functions
>   bindings: PCI: artpec: Add support for endpoint mode
>   PCI: dwc: artpec6: Add support for endpoint mode
>   PCI: dwc: Make cpu_addr_fixup take struct dw_pcie as argument
>   PCI: dwc: artpec6: Deassert the core before waiting for PHY
>   bindings: PCI: artpec: Add support for the ARTPEC-7 SoC
>   PCI: dwc: artpec6: Add support for the ARTPEC-7 SoC
> 
>  .../devicetree/bindings/pci/axis,artpec6-pcie.txt  |   5 +-
>  drivers/pci/dwc/Kconfig                            |  68 +--
>  drivers/pci/dwc/Makefile                           |   4 +-
>  drivers/pci/dwc/pci-dra7xx.c                       |  27 +-
>  drivers/pci/dwc/pcie-artpec6.c                     | 470 ++++++++++++++++++---
>  drivers/pci/dwc/pcie-designware-ep.c               |  59 ++-
>  drivers/pci/dwc/pcie-designware-host.c             |  15 +-
>  drivers/pci/dwc/pcie-designware.c                  |   2 +-
>  drivers/pci/dwc/pcie-designware.h                  |  22 +-
>  9 files changed, 554 insertions(+), 118 deletions(-)
> 
> -- 
> 2.14.2
> 

^ permalink raw reply

* Re: [PATCH v2 2/2] eeprom: at24: remove temporary fix for at24mac402 size.
From: Bartosz Golaszewski @ 2017-12-20 17:18 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas,
	Divagar Mohandass, devicetree, Linux Kernel Mailing List,
	linux-i2c
In-Reply-To: <1512768306-14816-3-git-send-email-svendev@arcx.com>

2017-12-08 22:25 GMT+01:00 Sven Van Asbroeck <svendev@arcx.com>:
> The chip size passed via devicetree, i2c, or acpi device ids is
> now no longer limited to a power of two. So the temporary
> fix can be removed.
>
> Signed-off-by: Sven Van Asbroeck <svendev@arcx.com>
> ---
>  drivers/misc/eeprom/at24.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index c3759cb..e522350 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -569,16 +569,6 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>                 dev_warn(&client->dev,
>                         "page_size looks suspicious (no power of 2)!\n");
>
> -       /*
> -        * REVISIT: the size of the EUI-48 byte array is 6 in at24mac402, while
> -        * the call to ilog2() in AT24_DEVICE_MAGIC() rounds it down to 4.
> -        *
> -        * Eventually we'll get rid of the magic values altoghether in favor of
> -        * real structs, but for now just manually set the right size.
> -        */
> -       if (chip.flags & AT24_FLAG_MAC && chip.byte_len == 4)
> -               chip.byte_len = 6;
> -
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C) &&
>             !i2c_check_functionality(client->adapter,
>                                      I2C_FUNC_SMBUS_WRITE_I2C_BLOCK))
> --
> 1.9.1
>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH v5 1/1] eeprom: at24: convert magic numbers to structs.
From: Bartosz Golaszewski @ 2017-12-20 17:16 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Wolfram Sang, nsekhar, Sakari Ailus, Javier Martinez Canillas,
	Divagar Mohandass, devicetree, Linux Kernel Mailing List,
	linux-i2c
In-Reply-To: <1513788536-7672-2-git-send-email-svendev@arcx.com>

2017-12-20 17:48 GMT+01:00 Sven Van Asbroeck <svendev@arcx.com>:
> Fundamental properties such as capacity and page size differ
> among at24-type chips. But these chips do not have an id register,
> so this can't be discovered at runtime.
>
> Traditionally, at24-type eeprom properties were determined in two ways:
> - by passing a 'struct at24_platform_data' via platform_data, or
> - by naming the chip type in the devicetree, which passes a
>         'magic number' to probe(), which is then converted to
>         a 'struct at24_platform_data'.
>
> Recently a bug was discovered because the magic number rounds down
> all chip sizes to the lowest power of two. This was addressed by
> a work-around, with the wish that magic numbers should over time
> be converted to structs.
> commit 5478e478eee3 ("eeprom: at24: correctly set the size for at24mac402")
>
> This patch replaces the magic numbers with 'struct at24_chip_data'.
>
> Signed-off-by: Sven Van Asbroeck <svendev@arcx.com>
> ---
>  drivers/misc/eeprom/at24.c | 221 ++++++++++++++++++++-------------------------
>  1 file changed, 100 insertions(+), 121 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index b44a3d2b..b1f78b9 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -105,16 +105,6 @@ struct at24_data {
>  module_param_named(write_timeout, at24_write_timeout, uint, 0);
>  MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to try writes (default 25)");
>
> -#define AT24_SIZE_BYTELEN 5
> -#define AT24_SIZE_FLAGS 8
> -
> -#define AT24_BITMASK(x) (BIT(x) - 1)
> -
> -/* create non-zero magic value for given eeprom parameters */
> -#define AT24_DEVICE_MAGIC(_len, _flags)                        \
> -       ((1 << AT24_SIZE_FLAGS | (_flags))              \
> -           << AT24_SIZE_BYTELEN | ilog2(_len))
> -
>  /*
>   * Both reads and writes fail if the previous write didn't complete yet. This
>   * macro loops a few times waiting at least long enough for one entire page
> @@ -132,113 +122,108 @@ struct at24_data {
>              op_time ? time_before(op_time, tout) : true;               \
>              usleep_range(1000, 1500), op_time = jiffies)
>
> +struct at24_chip_data {
> +       /*
> +        * these fields mirror their equivalents in
> +        * struct at24_platform_data
> +        */
> +       u32 byte_len;
> +       u8 flags;
> +};
> +
> +#define AT24_CHIP_DATA(_name, _len, _flags)                            \
> +       static const struct at24_chip_data _name = {                    \
> +               .byte_len = _len, .flags = _flags,                      \
> +       }
> +
> +/* needs 8 addresses as A0-A2 are ignored */
> +AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR);
> +/* old variants can't be handled with this generic entry! */
> +AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24cs01, 16,
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24cs02, 16,
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24mac402, 48 / 8,
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24mac602, 64 / 8,
> +       AT24_FLAG_MAC | AT24_FLAG_READONLY);
> +/* spd is a 24c02 in memory DIMMs */
> +AT24_CHIP_DATA(at24_data_spd, 2048 / 8,
> +       AT24_FLAG_READONLY | AT24_FLAG_IRUGO);
> +AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24cs04, 16,
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +/* 24rf08 quirk is handled at i2c-core */
> +AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24cs08, 16,
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0);
> +AT24_CHIP_DATA(at24_data_24cs16, 16,
> +       AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24cs32, 16,
> +       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24cs64, 16,
> +       AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
> +AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16);
> +AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16);
> +/* identical to 24c08 ? */
> +AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0);
> +
>  static const struct i2c_device_id at24_ids[] = {
> -       /* needs 8 addresses as A0-A2 are ignored */
> -       { "24c00",      AT24_DEVICE_MAGIC(128 / 8,      AT24_FLAG_TAKE8ADDR) },
> -       /* old variants can't be handled with this generic entry! */
> -       { "24c01",      AT24_DEVICE_MAGIC(1024 / 8,     0) },
> -       { "24cs01",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
> -       { "24c02",      AT24_DEVICE_MAGIC(2048 / 8,     0) },
> -       { "24cs02",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
> -       { "24mac402",   AT24_DEVICE_MAGIC(48 / 8,
> -                               AT24_FLAG_MAC | AT24_FLAG_READONLY) },
> -       { "24mac602",   AT24_DEVICE_MAGIC(64 / 8,
> -                               AT24_FLAG_MAC | AT24_FLAG_READONLY) },
> -       /* spd is a 24c02 in memory DIMMs */
> -       { "spd",        AT24_DEVICE_MAGIC(2048 / 8,
> -                               AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
> -       { "24c04",      AT24_DEVICE_MAGIC(4096 / 8,     0) },
> -       { "24cs04",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
> -       /* 24rf08 quirk is handled at i2c-core */
> -       { "24c08",      AT24_DEVICE_MAGIC(8192 / 8,     0) },
> -       { "24cs08",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
> -       { "24c16",      AT24_DEVICE_MAGIC(16384 / 8,    0) },
> -       { "24cs16",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
> -       { "24c32",      AT24_DEVICE_MAGIC(32768 / 8,    AT24_FLAG_ADDR16) },
> -       { "24cs32",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_ADDR16 |
> -                               AT24_FLAG_SERIAL |
> -                               AT24_FLAG_READONLY) },
> -       { "24c64",      AT24_DEVICE_MAGIC(65536 / 8,    AT24_FLAG_ADDR16) },
> -       { "24cs64",     AT24_DEVICE_MAGIC(16,
> -                               AT24_FLAG_ADDR16 |
> -                               AT24_FLAG_SERIAL |
> -                               AT24_FLAG_READONLY) },
> -       { "24c128",     AT24_DEVICE_MAGIC(131072 / 8,   AT24_FLAG_ADDR16) },
> -       { "24c256",     AT24_DEVICE_MAGIC(262144 / 8,   AT24_FLAG_ADDR16) },
> -       { "24c512",     AT24_DEVICE_MAGIC(524288 / 8,   AT24_FLAG_ADDR16) },
> -       { "24c1024",    AT24_DEVICE_MAGIC(1048576 / 8,  AT24_FLAG_ADDR16) },
> -       { "at24", 0 },
> +       { "24c00",      (kernel_ulong_t)&at24_data_24c00 },
> +       { "24c01",      (kernel_ulong_t)&at24_data_24c01 },
> +       { "24cs01",     (kernel_ulong_t)&at24_data_24cs01 },
> +       { "24c02",      (kernel_ulong_t)&at24_data_24c02 },
> +       { "24cs02",     (kernel_ulong_t)&at24_data_24cs02 },
> +       { "24mac402",   (kernel_ulong_t)&at24_data_24mac402 },
> +       { "24mac602",   (kernel_ulong_t)&at24_data_24mac602 },
> +       { "spd",        (kernel_ulong_t)&at24_data_spd },
> +       { "24c04",      (kernel_ulong_t)&at24_data_24c04 },
> +       { "24cs04",     (kernel_ulong_t)&at24_data_24cs04 },
> +       { "24c08",      (kernel_ulong_t)&at24_data_24c08 },
> +       { "24cs08",     (kernel_ulong_t)&at24_data_24cs08 },
> +       { "24c16",      (kernel_ulong_t)&at24_data_24c16 },
> +       { "24cs16",     (kernel_ulong_t)&at24_data_24cs16 },
> +       { "24c32",      (kernel_ulong_t)&at24_data_24c32 },
> +       { "24cs32",     (kernel_ulong_t)&at24_data_24cs32 },
> +       { "24c64",      (kernel_ulong_t)&at24_data_24c64 },
> +       { "24cs64",     (kernel_ulong_t)&at24_data_24cs64 },
> +       { "24c128",     (kernel_ulong_t)&at24_data_24c128 },
> +       { "24c256",     (kernel_ulong_t)&at24_data_24c256 },
> +       { "24c512",     (kernel_ulong_t)&at24_data_24c512 },
> +       { "24c1024",    (kernel_ulong_t)&at24_data_24c1024 },
> +       { "at24",       0 },
>         { /* END OF LIST */ }
>  };
>  MODULE_DEVICE_TABLE(i2c, at24_ids);
>
>  static const struct of_device_id at24_of_match[] = {
> -       {
> -               .compatible = "atmel,24c00",
> -               .data = (void *)AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR)
> -       },
> -       {
> -               .compatible = "atmel,24c01",
> -               .data = (void *)AT24_DEVICE_MAGIC(1024 / 8, 0)
> -       },
> -       {
> -               .compatible = "atmel,24c02",
> -               .data = (void *)AT24_DEVICE_MAGIC(2048 / 8, 0)
> -       },
> -       {
> -               .compatible = "atmel,spd",
> -               .data = (void *)AT24_DEVICE_MAGIC(2048 / 8,
> -                               AT24_FLAG_READONLY | AT24_FLAG_IRUGO)
> -       },
> -       {
> -               .compatible = "atmel,24c04",
> -               .data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0)
> -       },
> -       {
> -               .compatible = "atmel,24c08",
> -               .data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0)
> -       },
> -       {
> -               .compatible = "atmel,24c16",
> -               .data = (void *)AT24_DEVICE_MAGIC(16384 / 8, 0)
> -       },
> -       {
> -               .compatible = "atmel,24c32",
> -               .data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
> -       },
> -       {
> -               .compatible = "atmel,24c64",
> -               .data = (void *)AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16)
> -       },
> -       {
> -               .compatible = "atmel,24c128",
> -               .data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16)
> -       },
> -       {
> -               .compatible = "atmel,24c256",
> -               .data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
> -       },
> -       {
> -               .compatible = "atmel,24c512",
> -               .data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16)
> -       },
> -       {
> -               .compatible = "atmel,24c1024",
> -               .data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16)
> -       },
> -       { },
> +       { .compatible = "atmel,24c00",          .data = &at24_data_24c00 },
> +       { .compatible = "atmel,24c01",          .data = &at24_data_24c01 },
> +       { .compatible = "atmel,24c02",          .data = &at24_data_24c02 },
> +       { .compatible = "atmel,spd",            .data = &at24_data_spd },
> +       { .compatible = "atmel,24c04",          .data = &at24_data_24c04 },
> +       { .compatible = "atmel,24c08",          .data = &at24_data_24c08 },
> +       { .compatible = "atmel,24c16",          .data = &at24_data_24c16 },
> +       { .compatible = "atmel,24c32",          .data = &at24_data_24c32 },
> +       { .compatible = "atmel,24c64",          .data = &at24_data_24c64 },
> +       { .compatible = "atmel,24c128",         .data = &at24_data_24c128 },
> +       { .compatible = "atmel,24c256",         .data = &at24_data_24c256 },
> +       { .compatible = "atmel,24c512",         .data = &at24_data_24c512 },
> +       { .compatible = "atmel,24c1024",        .data = &at24_data_24c1024 },
> +       { /* END OF LIST */ },
>  };
>  MODULE_DEVICE_TABLE(of, at24_of_match);
>
>  static const struct acpi_device_id at24_acpi_ids[] = {
> -       { "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) },
> -       { }
> +       { "INT3499",    (kernel_ulong_t)&at24_data_INT3499 },
> +       { /* END OF LIST */ }
>  };
>  MODULE_DEVICE_TABLE(acpi, at24_acpi_ids);
>
> @@ -516,8 +501,8 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
>
>  static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  {
> -       struct at24_platform_data chip;
> -       kernel_ulong_t magic = 0;
> +       struct at24_platform_data chip = { 0 };
> +       const struct at24_chip_data *cd = NULL;
>         bool writable;
>         struct at24_data *at24;
>         int err;
> @@ -535,28 +520,22 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
>                  */
>                 if (client->dev.of_node &&
>                     of_match_device(at24_of_match, &client->dev)) {
> -                       magic = (kernel_ulong_t)
> -                               of_device_get_match_data(&client->dev);
> +                       cd = of_device_get_match_data(&client->dev);
>                 } else if (id) {
> -                       magic = id->driver_data;
> +                       cd = (void *)id->driver_data;
>                 } else {
>                         const struct acpi_device_id *aid;
>
>                         aid = acpi_match_device(at24_acpi_ids, &client->dev);
>                         if (aid)
> -                               magic = aid->driver_data;
> +                               cd = (void *)aid->driver_data;
>                 }
> -               if (!magic)
> +               if (!cd)
>                         return -ENODEV;
>
> -               chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
> -               magic >>= AT24_SIZE_BYTELEN;
> -               chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
> -
> +               chip.byte_len = cd->byte_len;
> +               chip.flags = cd->flags;
>                 at24_get_pdata(&client->dev, &chip);
> -
> -               chip.setup = NULL;
> -               chip.context = NULL;
>         }
>
>         if (!is_power_of_2(chip.byte_len))
> --
> 1.9.1
>

Patch applied, thanks!

In the future: always resend the entire series even when fixing issues
in one patch only.

Best regards,
Bartosz Golaszewski

^ permalink raw reply

* Re: [PATCH 1/3] dt-bindings: ARM: Mediatek: Fix ethsys documentation
From: Matthias Brugger @ 2017-12-20 17:13 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Michael Turquette, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	sean.wang-NuS5LvNUpcJWk0Htik3J/w,
	chen.zhong-NuS5LvNUpcJWk0Htik3J/w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171219013222.GZ7997-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>



On 12/19/2017 02:32 AM, Stephen Boyd wrote:
> On 12/14, Matthias Brugger wrote:
>> Hi Stephen, Michael,
>>
>> On 12/01/2017 01:07 PM, Matthias Brugger wrote:
>>> The ethsys registers a reset controller, so we need to specify a
>>> reset cell. This patch fixes the documentation.
>>>
>>> Signed-off-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  Documentation/devicetree/bindings/arm/mediatek/mediatek,ethsys.txt | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,ethsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,ethsys.txt
>>> index 7aa3fa167668..6cc7840ff37a 100644
>>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,ethsys.txt
>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,ethsys.txt
>>> @@ -20,4 +20,5 @@ ethsys: clock-controller@1b000000 {
>>>  	compatible = "mediatek,mt2701-ethsys", "syscon";
>>>  	reg = <0 0x1b000000 0 0x1000>;
>>>  	#clock-cells = <1>;
>>> +	#reset-cells = <1>;
>>>  };
>>>
>>
>> Will you take this patch through the clk tree, or shall I take it through my SoC
>> tree?
>>
> 
> It's resets, we are clk maintainers. I'm clkfused.
> 
> You can take it, along with my
> 
> Acked-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> 
> if you like/expect conflicts.
> 

These are resets in the clock IP-block. I'll take it through my branch, I don't
expect any conflicts.

Regards,
Matthias
--
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 v5 2/3] PCI: mobiveil: Add Mobiveil PCIe Host Bridge IP driver
From: Lorenzo Pieralisi @ 2017-12-20 17:03 UTC (permalink / raw)
  To: Subrahmanya Lingappa
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
	marc.zyngier-5wv7dgnIgG8, robh-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, mingkai.hu-3arQi8VN3Tc,
	peter.newton-3arQi8VN3Tc, minghuan.lian-3arQi8VN3Tc,
	rajesh.raina-3arQi8VN3Tc, rajan.kapoor-3arQi8VN3Tc,
	prabhjot.singh-3arQi8VN3Tc
In-Reply-To: <1513181317-19914-1-git-send-email-l.subrahmanya-DTHOJn6Rh8lhmhkoCovsdw@public.gmane.org>

On Wed, Dec 13, 2017 at 11:08:37AM -0500, Subrahmanya Lingappa wrote:
> Adds driver for Mobiveil AXI PCIe Host Bridge Soft IP -
> GPEX 4.0, a PCIe gen4 IP. This IP has upto 8
> outbound and inbound windows for the address translation.
> 
> Signed-off-by: Subrahmanya Lingappa <l.subrahmanya-DTHOJn6Rh8lhmhkoCovsdw@public.gmane.org>
> Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> Cc: Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>
> Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/pci/host/Kconfig         |   8 +
>  drivers/pci/host/Makefile        |   1 +
>  drivers/pci/host/pcie-mobiveil.c | 653 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 662 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-mobiveil.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 38d1298..09bf1d9 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -27,6 +27,14 @@ config PCIE_XILINX_NWL
>  	 or End Point. The current option selection will only
>  	 support root port enabling.
>  
> +config PCIE_MOBIVEIL
> +        bool "Mobiveil AXI PCIe host bridge support"
> +        depends on ARCH_ZYNQMP || COMPILE_TEST
> +        depends on PCI_MSI_IRQ_DOMAIN

There is no PCI_MSI_IRQ_DOMAIN dependency in this patch.

> +        help
> +          Say 'Y' here if you want kernel to support the Mobiveil AXI PCIe
> +          Host Bridge driver.
> +
>  config PCI_FTPCI100
>  	bool "Faraday Technology FTPCI100 PCI controller"
>  	depends on OF
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 34ec1d8..d745d68 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
>  obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
>  obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
>  obj-$(CONFIG_VMD) += vmd.o
> +obj-$(CONFIG_PCIE_MOBIVEIL) += pcie-mobiveil.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/host/pcie-mobiveil.c b/drivers/pci/host/pcie-mobiveil.c
> new file mode 100644
> index 0000000..8611aaa
> --- /dev/null
> +++ b/drivers/pci/host/pcie-mobiveil.c
> @@ -0,0 +1,653 @@
> +/*
> + * PCIe host controller driver for Mobiveil PCIe Host controller
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Mobiveil Inc.
> + * Author: Subrahmanya Lingappa <l.subrahmanya-DTHOJn6Rh8lhmhkoCovsdw@public.gmane.org>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_pci.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/* register offsets and bit positions */
> +
> +/*
> + * translation tables are grouped into windows, each window registers are
> + * grouped into blocks of 4 or 16 registers each
> + */
> +#define PAB_REG_BLOCK_SIZE_4	4
> +#define PAB_REG_BLOCK_SIZE_16	16

What I wanted to say is that you can tag the block with a name
instead of using a number.

I can say that block size 4 is only for PAB_EXT_*, you can tag
it as such.

eg PAB_EXT_REG_BLOCK_SIZE

> +#define PAB_REG_ADDR_4(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_4))
> +#define PAB_REG_ADDR_16(offset, win) (offset + (win * PAB_REG_BLOCK_SIZE_16))
> +
> +#define LTSSM_STATUS		0x0404
> +#define  LTSSM_STATUS_L0_MASK	0x3f
> +#define  LTSSM_STATUS_L0	0x2d
> +
> +#define PAB_CTRL		0x0808
> +#define  AMBA_PIO_ENABLE_SHIFT	0
> +#define  PEX_PIO_ENABLE_SHIFT	1
> +#define  PAGE_SEL_SHIFT		13
> +#define  PAGE_SEL_MASK		0x3f
> +#define  PAGE_LO_MASK		0x3ff
> +#define  PAGE_SEL_EN		0xc00
> +#define  PAGE_SEL_OFFSET_SHIFT	10
> +
> +#define PAB_AXI_PIO_CTRL	0x0840
> +#define  APIO_EN_MASK		0xf
> +
> +#define PAB_PEX_PIO_CTRL	0x08c0
> +#define  PIO_ENABLE_SHIFT	0
> +
> +#define PAB_INTP_AMBA_MISC_ENB	0x0b0c
> +#define PAB_INTP_AMBA_MISC_STAT	0x0b1c
> +#define  PAB_INTP_INTX_MASK	0x1e0
> +
> +#define PAB_AXI_AMAP_CTRL(win)	PAB_REG_ADDR_16(0x0ba0, win)
> +#define  WIN_ENABLE_SHIFT	0
> +#define  WIN_TYPE_SHIFT		1
> +#define  WIN_SIZE_SHIFT		10
> +
> +#define PAB_EXT_AXI_AMAP_SIZE(win)	PAB_REG_ADDR_4(0xbaf0, win)
> +
> +#define PAB_AXI_AMAP_AXI_WIN(win)	PAB_REG_ADDR_16(0x0ba4, win)
> +#define  AXI_WINDOW_BASE_SHIFT		2
> +
> +#define PAB_AXI_AMAP_PEX_WIN_L(win)	PAB_REG_ADDR_16(0x0ba8, win)
> +#define  PAB_BUS_SHIFT			24
> +#define  PAB_DEVICE_SHIFT		19
> +#define  PAB_FUNCTION_SHIFT		16
> +
> +#define PAB_AXI_AMAP_PEX_WIN_H(win)	PAB_REG_ADDR_16(0x0bac, win)
> +#define PAB_INTP_AXI_PIO_CLASS		0x474

This one is a bit odd here, move it up in the file.

> +#define PAB_PEX_AMAP_CTRL(win)		PAB_REG_ADDR_16(0x4ba0, win)
> +#define  AMAP_CTRL_EN_SHIFT		0
> +#define  AMAP_CTRL_TYPE_SHIFT		1
> +
> +#define PAB_EXT_PEX_AMAP_SIZEN(win)	PAB_REG_ADDR_4(0xbef0, win)
> +#define PAB_PEX_AMAP_AXI_WIN(win)	PAB_REG_ADDR_16(0x4ba4, win)
> +#define PAB_PEX_AMAP_PEX_WIN_L(win)	PAB_REG_ADDR_16(0x4ba8, win)
> +#define PAB_PEX_AMAP_PEX_WIN_H(win)	PAB_REG_ADDR_16(0x4bac, win)

Here you have three base offsets:

0xb00
0x4000
0xb000

you can create a macro for each of them, according to what they
represent and then add an offset into each.

> +
> +/* supported number of interrupts */
> +#define PCI_NUM_INTX		4

It is already defined in linux/pci.h

> +#define PAB_INTA_POS		5
> +
> +/* outbound and inbound window definitions */
> +#define WIN_NUM_0		0
> +#define WIN_NUM_1		1
> +#define CFG_WINDOW_TYPE		0
> +#define IO_WINDOW_TYPE		1
> +#define MEM_WINDOW_TYPE		2
> +#define IB_WIN_SIZE		(256 * 1024 * 1024)
> +#define MAX_PIO_WINDOWS		8
> +
> +/* Parameters for the waiting for link up routine */
> +#define LINK_WAIT_MAX_RETRIES	10
> +#define LINK_WAIT_MIN		90000
> +#define LINK_WAIT_MAX		100000
> +
> +#ifndef UINT64_MAX
> +#define UINT64_MAX		(u64)(~((u64)0))
> +#endif
> +
> +struct mobiveil_pcie {
> +	struct platform_device *pdev;
> +	struct list_head resources;
> +	void __iomem *config_axi_slave_base;	/* endpoint config base */
> +	void __iomem *csr_axi_slave_base;	/* root port config base */
> +	struct irq_domain *intx_domain;
> +	int irq;
> +	int apio_wins;
> +	int ppio_wins;
> +	int ob_wins_configured;	/*  configured outbound windows */
> +	int ib_wins_configured;	/*  configured inbound windows */
> +	struct resource *ob_io_res;
> +	char root_bus_nr;
> +};
> +
> +static inline void csr_writel(struct mobiveil_pcie *pcie, const u32 value,
> +		const u32 reg)
> +{
> +	writel_relaxed(value, pcie->csr_axi_slave_base + reg);
> +}
> +
> +static inline u32 csr_readl(struct mobiveil_pcie *pcie,	const u32 reg)
> +{
> +	return readl_relaxed(pcie->csr_axi_slave_base + reg);
> +}
> +
> +static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> +{
> +	return (csr_readl(pcie, LTSSM_STATUS) &
> +		LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> +}

https://marc.info/?l=linux-pci&m=151329230814614&w=2

Bjorn would like to remove it.

> +static bool mobiveil_pcie_valid_device(struct pci_bus *bus, unsigned int devfn)
> +{
> +	struct mobiveil_pcie *pcie = bus->sysdata;
> +
> +	/* Check if link is up when trying to access downstream ports */
> +	if (bus->number != pcie->root_bus_nr)
> +		if (!mobiveil_pcie_link_up(pcie))
> +			return false;
> +
> +	/* Only one device down on each root port */
> +	if ((bus->number == pcie->root_bus_nr) && (devfn > 0))
> +		return false;
> +
> +	/*
> +	 * Do not read more than one device on the bus directly
> +	 * attached to RC
> +	 */
> +	if ((bus->primary == pcie->root_bus_nr) && (devfn > 0))
> +		return false;
> +
> +	return true;
> +}
> +
> +/*
> + * mobiveil_pcie_map_bus - routine to get the configuration base of either
> + * root port or endpoint
> + */
> +static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> +					unsigned int devfn, int where)
> +{
> +	struct mobiveil_pcie *pcie = bus->sysdata;
> +
> +	if (!mobiveil_pcie_valid_device(bus, devfn))
> +		return NULL;
> +
> +	if (bus->number == pcie->root_bus_nr) {
> +		/* RC config access */
> +		return pcie->csr_axi_slave_base + where;
> +	} else {
> +		/*
> +		 * EP config access (in Config/APIO space)
> +		 * Program PEX Address base (31..16 bits) with appropriate value
> +		 * (BDF) in PAB_AXI_AMAP_PEX_WIN_L0 Register.
> +		 * Relies on pci_lock serialization
> +		 */
> +		csr_writel(pcie,
> +			bus->number << PAB_BUS_SHIFT |
> +			PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> +			PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT,
> +			PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> +		return pcie->config_axi_slave_base + where;
> +	}
> +}
> +
> +static struct pci_ops mobiveil_pcie_ops = {
> +	.map_bus = mobiveil_pcie_map_bus,
> +	.read = pci_generic_config_read,
> +	.write = pci_generic_config_write,
> +};
> +
> +static void mobiveil_pcie_isr(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct mobiveil_pcie *pcie = irq_desc_get_handler_data(desc);
> +	struct device *dev = &pcie->pdev->dev;
> +	u32 intr_status;
> +	unsigned long shifted_status;

Why not u32 ?

> +	u32 bit, virq;
> +	u32 val, mask;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	/* read INTx status */
> +	val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> +	mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> +	intr_status = val & mask;
> +
> +	/* Handle INTx */
> +	if (intr_status & PAB_INTP_INTX_MASK) {
> +		shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);

Why can't you reuse val to read the status and create shifted_status
from it ?

eg

shifted_status = val << PAB_INTA_POS;
csr_writel(pcie, shifted_status, PAB_INTP_AMBA_MISC_STAT);

Just a hint to make the loop more readable. BTW, I do not think that
writing shifted_status into that register is OK, see below.

> +		do {
> +			for_each_set_bit(bit, &shifted_status, PCI_NUM_INTX) {
> +
> +				/* clear interrupts */
> +				csr_writel(pcie, shifted_status << PAB_INTA_POS,
> +						PAB_INTP_AMBA_MISC_STAT);

Should not you clear just the interrupt you are about to handle ?

> +				virq = irq_find_mapping(pcie->intx_domain,
> +					bit + 1);
> +				if (virq)
> +					generic_handle_irq(virq);
> +				else
> +					dev_err_ratelimited(dev,
> +						"unexpected IRQ, INT%d\n",
> +						bit);
> +
> +			}
> +
> +			shifted_status = csr_readl(pcie,
> +						PAB_INTP_AMBA_MISC_STAT);
> +
> +		} while ((shifted_status >>  PAB_INTA_POS) != 0);

I do not understand this. Can you explain to me how the
register at:

PAB_INTP_AMBA_MISC_STAT

works ?

A patch for mediatek has been posted:

https://patchwork.ozlabs.org/patch/851181/

It looks like this loop may suffer from the same issue, so please do
have a look.

On top of that, most of the operations you are carrying out here
can be done automatically by making them part of the struct irq_chip
methods (ie acking IRQs, masking IRQs, etc, see below).

> +	}
> +
> +	csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);

> +	chained_irq_exit(chip, desc);
> +}
> +
> +static int mobiveil_pcie_parse_dt(struct mobiveil_pcie *pcie)
> +{
> +	struct device *dev = &pcie->pdev->dev;
> +	struct platform_device *pdev = pcie->pdev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	const char *type;
> +
> +	type = of_get_property(node, "device_type", NULL);
> +	if (!type || strcmp(type, "pci")) {
> +		dev_err(dev, "invalid \"device_type\" %s\n", type);
> +		return -EINVAL;
> +	}
> +
> +	/* map config resource */
> +	res = platform_get_resource_byname(pdev,
> +		IORESOURCE_MEM, "config_axi_slave");
> +	pcie->config_axi_slave_base = devm_pci_remap_cfg_resource(dev, res);
> +	if (IS_ERR(pcie->config_axi_slave_base))
> +		return PTR_ERR(pcie->config_axi_slave_base);
> +	pcie->ob_io_res = res;
> +
> +	/* map csr resource */
> +	res = platform_get_resource_byname(pdev,
> +		IORESOURCE_MEM, "csr_axi_slave");
> +	pcie->csr_axi_slave_base = devm_pci_remap_cfg_resource(dev, res);
> +	if (IS_ERR(pcie->csr_axi_slave_base))
> +		return PTR_ERR(pcie->csr_axi_slave_base);
> +
> +	/* read the number of windows requested */
> +	if (!pcie->apio_wins &&
> +		of_property_read_u32(node, "apio-wins", &pcie->apio_wins)) {
> +		pcie->apio_wins = MAX_PIO_WINDOWS;
> +	}
> +
> +	if (!pcie->ppio_wins &&

What I wanted to say in my previous review is that apio_wins and
ppio_wins are zero here why would they be any different and why
do you need to check that.

> +		of_property_read_u32(node, "ppio-wins", &pcie->ppio_wins)) {
> +		pcie->ppio_wins = MAX_PIO_WINDOWS;
> +	}
> +
> +	pcie->irq = platform_get_irq(pdev, 0);
> +	if (pcie->irq <= 0) {
> +		dev_err(dev, "failed to map IRQ: %d\n", pcie->irq);
> +		return -ENODEV;
> +	}
> +
> +	irq_set_chained_handler_and_data(pcie->irq, mobiveil_pcie_isr, pcie);
> +
> +	return 0;
> +}
> +
> +/*
> + * select_paged_register - routine to access paged register of root complex
> + *
> + * registers of RC are paged, for this scheme to work
> + * extracted higher 6 bits of the offset will be written to pg_sel
> + * field of PAB_CTRL register and rest of the lower 10 bits enabled with
> + * PAGE_SEL_EN are used as offset of the register.
> + *
> + */
> +static void select_paged_register(struct mobiveil_pcie *pcie, u32 offset)
> +{
> +	int pab_ctrl_dw, pg_sel;
> +
> +	/* clear pg_sel field */
> +	pab_ctrl_dw = csr_readl(pcie, PAB_CTRL);
> +	pab_ctrl_dw = (pab_ctrl_dw & ~(PAGE_SEL_MASK << PAGE_SEL_SHIFT));
> +
> +	/* set pg_sel field */
> +	pg_sel = (offset >> PAGE_SEL_OFFSET_SHIFT) & PAGE_SEL_MASK;
> +	pab_ctrl_dw |= ((pg_sel << PAGE_SEL_SHIFT));
> +	csr_writel(pcie, pab_ctrl_dw, PAB_CTRL);
> +}
> +
> +static void write_paged_register(struct mobiveil_pcie *pcie,
> +		u32 val, u32 offset)
> +{
> +	u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN;
> +
> +	select_paged_register(pcie, offset);
> +	csr_writel(pcie, val, off);
> +}
> +
> +static u32 read_paged_register(struct mobiveil_pcie *pcie, u32 offset)
> +{
> +	u32 off = (offset & PAGE_LO_MASK) | PAGE_SEL_EN;
> +
> +	select_paged_register(pcie, offset);
> +	return csr_readl(pcie, off);
> +}
> +
> +static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
> +		int pci_addr, int type,	int size_kb)
> +{
> +	int pio_ctrl_val;
> +	int amap_ctrl_dw;
> +	u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb)));
> +
> +	if ((pcie->ib_wins_configured + 1) > pcie->ppio_wins) {
> +		dev_err(&pcie->pdev->dev,
> +			"ERROR: max inbound windows reached !\n");
> +		return;
> +	}
> +
> +	pio_ctrl_val = csr_readl(pcie, PAB_PEX_PIO_CTRL);
> +	csr_writel(pcie,
> +		pio_ctrl_val | (1 << PIO_ENABLE_SHIFT), PAB_PEX_PIO_CTRL);
> +	amap_ctrl_dw =	read_paged_register(pcie, PAB_PEX_AMAP_CTRL(win_num));
> +	amap_ctrl_dw = (amap_ctrl_dw | (type << AMAP_CTRL_TYPE_SHIFT));
> +	amap_ctrl_dw = (amap_ctrl_dw | (1 << AMAP_CTRL_EN_SHIFT));
> +
> +	write_paged_register(pcie, amap_ctrl_dw | lower_32_bits(size64),
> +				PAB_PEX_AMAP_CTRL(win_num));
> +
> +	write_paged_register(pcie, upper_32_bits(size64),
> +				PAB_EXT_PEX_AMAP_SIZEN(win_num));
> +
> +	write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_AXI_WIN(win_num));
> +	write_paged_register(pcie, pci_addr, PAB_PEX_AMAP_PEX_WIN_L(win_num));
> +	write_paged_register(pcie, 0, PAB_PEX_AMAP_PEX_WIN_H(win_num));
> +}
> +
> +static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
> +		u64 cpu_addr, u64 pci_addr, int config_io_bit,
> +		int size_kb)
> +{
> +
> +	u32 value, type;
> +	u64 size64 = (UINT64_MAX << (WIN_SIZE_SHIFT + ilog2(size_kb)));

Can you explain to me the sizing mechanism please ? It is just to make
sure I got it right.

> +	if ((pcie->ob_wins_configured + 1) > pcie->apio_wins) {
> +		dev_err(&pcie->pdev->dev,
> +			"ERROR: max outbound windows reached !\n");
> +		return;
> +	}
> +
> +	/*
> +	 * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
> +	 * to 4 KB in PAB_AXI_AMAP_CTRL register
> +	 */
> +	type = config_io_bit;
> +	value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> +	csr_writel(pcie,
> +			1 << WIN_ENABLE_SHIFT |
> +			type << WIN_TYPE_SHIFT |
> +			lower_32_bits(size64),
> +			PAB_AXI_AMAP_CTRL(win_num));
> +
> +	write_paged_register(pcie, upper_32_bits(size64),
> +				PAB_EXT_AXI_AMAP_SIZE(win_num));
> +
> +	/*
> +	 * program AXI window base with appropriate value in
> +	 * PAB_AXI_AMAP_AXI_WIN0 register
> +	 */
> +	value = csr_readl(pcie, PAB_AXI_AMAP_AXI_WIN(win_num));
> +	csr_writel(pcie,
> +			cpu_addr >> AXI_WINDOW_BASE_SHIFT <<
> +			AXI_WINDOW_BASE_SHIFT, PAB_AXI_AMAP_AXI_WIN(win_num));
> +
> +	value = csr_readl(pcie, PAB_AXI_AMAP_PEX_WIN_H(win_num));

What's "value" used for in this function ?

> +	csr_writel(pcie, lower_32_bits(pci_addr),
> +			PAB_AXI_AMAP_PEX_WIN_L(win_num));
> +	csr_writel(pcie, upper_32_bits(pci_addr),
> +			PAB_AXI_AMAP_PEX_WIN_H(win_num));
> +
> +	pcie->ob_wins_configured++;
> +}
> +
> +static int mobiveil_bringup_link(struct mobiveil_pcie *pcie)
> +{
> +	int retries;
> +
> +	/* check if the link is up or not */
> +	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> +		if (mobiveil_pcie_link_up(pcie))
> +			return 0;
> +
> +		usleep_range(LINK_WAIT_MIN, LINK_WAIT_MAX);
> +	}
> +	dev_err(&pcie->pdev->dev, "link never came up\n");
> +	return -ETIMEDOUT;
> +}
> +
> +static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> +{
> +	u32 value;
> +	int type = 0;

You reinitialize it later and it should not be an int but an unsigned
type.

> +	u32 pab_ctrl;
> +	int err;
> +	struct resource_entry *win, *tmp;
> +
> +	/* setup the PCIe slot link power*/

Comment is useless, remove it.

> +	err = mobiveil_bringup_link(pcie);
> +	if (err) {
> +		dev_info(&pcie->pdev->dev, "link bring-up failed\n");
> +		return err;
> +	}
> +
> +	/*
> +	 * program Bus Master Enable Bit in Command Register in PAB Config
> +	 * Space
> +	 */
> +	value = csr_readl(pcie, PCI_COMMAND);
> +	csr_writel(pcie,
> +		value |

Why is this on a separate line ?

> +		PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER,
> +		PCI_COMMAND);
> +
> +	/*
> +	 * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
> +	 * register
> +	 */
> +	pab_ctrl = csr_readl(pcie, PAB_CTRL);
> +	csr_writel(pcie, pab_ctrl | (1 << AMBA_PIO_ENABLE_SHIFT) |
> +		(1 << PEX_PIO_ENABLE_SHIFT),
> +		PAB_CTRL);
> +
> +	csr_writel(pcie, PAB_INTP_INTX_MASK, PAB_INTP_AMBA_MISC_ENB);
> +
> +	/* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> +	 * PAB_AXI_PIO_CTRL Register
> +	 */
> +	value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
> +	csr_writel(pcie, value | APIO_EN_MASK, PAB_AXI_PIO_CTRL);
> +
> +	/*
> +	 * we'll program one outbound window for config reads and
> +	 * another default inbound window for all the upstream traffic
> +	 * rest of the outbound windows will be configured according to
> +	 * the "ranges" field defined in device tree
> +	 */
> +
> +	/* config outbound translation window */
> +	program_ob_windows(pcie, pcie->ob_wins_configured,
> +			pcie->ob_io_res->start,	0, CFG_WINDOW_TYPE,
> +			resource_size(pcie->ob_io_res)/1024);
> +
> +	/* memory inbound  translation window */
> +	program_ib_windows(pcie, WIN_NUM_1, 0, MEM_WINDOW_TYPE, IB_WIN_SIZE);
> +
> +	/* Get the I/O and memory ranges from DT */
> +	resource_list_for_each_entry_safe(win, tmp, &pcie->resources) {
> +		type = 0;
> +		if (resource_type(win->res) == IORESOURCE_MEM)
> +			type = MEM_WINDOW_TYPE;
> +		if (resource_type(win->res) == IORESOURCE_IO)
> +			type = IO_WINDOW_TYPE;
> +		if (type) {
> +			/* configure outbound translation window */
> +			program_ob_windows(pcie, pcie->ob_wins_configured,
> +				win->res->start, 0, type,
> +				resource_size(win->res)/1024);
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +/* routine to setup the INTx related data */
> +static int mobiveil_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> +		irq_hw_number_t hwirq)
> +{
> +	irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);

IIUC, the host bridge acts as an INTX/MSI controller/multiplexer.

So, instead of relying on dummy_irq_chip, you should create your own
irq_chip with the methods that you require (eg irq_ack(), irq_mask(),
irq_compose_msi_msg()) and use that as bottom irq_chip for both
INTX and MSI domains.

That's why I asked you to have a look at pcie-tango.c (except that there
INTX aren't supported but the basic idea does not change) and implement
IRQ domains handling like that same driver.

> +	irq_set_chip_data(irq, domain->host_data);
> +	return 0;
> +}
> +
> +/* INTx domain opeartions structure */

s/opeartions/operations

> +static const struct irq_domain_ops intx_domain_ops = {
> +	.map = mobiveil_pcie_intx_map,
> +};
> +
> +static int mobiveil_pcie_init_irq_domain(struct mobiveil_pcie *pcie)
> +{
> +	struct device *dev = &pcie->pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	int ret;

ret is unused

> +	/* setup INTx */
> +	pcie->intx_domain = irq_domain_add_linear(node,
> +				PCI_NUM_INTX + 1, &intx_domain_ops, pcie);

You should use PCI_NUM_INTX and add pci_irqd_intx_xlate() as the
domain ops .xlate.

> +	if (!pcie->intx_domain) {
> +		dev_err(dev, "Failed to get a INTx IRQ domain\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mobiveil_pcie_probe(struct platform_device *pdev)
> +{
> +	struct mobiveil_pcie *pcie;
> +	struct pci_bus *bus;
> +	struct pci_bus *child;
> +	struct pci_host_bridge *bridge;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	resource_size_t iobase;
> +	int ret;
> +
> +	/* allocate the PCIe port */
> +	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
> +	if (!bridge)
> +		return -ENODEV;
> +
> +	pcie = pci_host_bridge_priv(bridge);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	pcie->pdev = pdev;
> +
> +	/* parse the device tree */

Remove this comment, it does not add anything.

> +	ret = mobiveil_pcie_parse_dt(pcie);
> +	if (ret) {
> +		dev_err(dev, "Parsing DT failed, ret: %x\n", ret);
> +		return ret;
> +	}
> +
> +	INIT_LIST_HEAD(&pcie->resources);
> +
> +	/* parse the host bridge base addresses from the device tree file */
> +	ret = of_pci_get_host_bridge_resources(node,
> +			0, 0xff, &pcie->resources, &iobase);
> +	if (ret) {
> +		dev_err(dev, "Getting bridge resources failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	/*
> +	 * configure all inbound and outbound windows and prepare the RC for
> +	 * config access
> +	 */
> +	ret = mobiveil_host_init(pcie);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize host\n");
> +		goto error;
> +	}
> +
> +	/* fixup for PCIe class register */
> +	csr_writel(pcie, 0x060402ab, PAB_INTP_AXI_PIO_CLASS);
> +
> +	/* initialize the IRQ domains */
> +	ret = mobiveil_pcie_init_irq_domain(pcie);
> +	if (ret) {
> +		dev_err(dev, "Failed creating IRQ Domain\n");
> +		goto error;
> +	}
> +
> +

Two empty lines, should be one.

> +	ret = devm_request_pci_bus_resources(dev, &pcie->resources);
> +	if (ret)
> +		goto error;
> +
> +	/* Initialize bridge */
> +	list_splice_init(&pcie->resources, &bridge->windows);
> +	bridge->dev.parent = dev;
> +	bridge->sysdata = pcie;
> +	bridge->busnr = pcie->root_bus_nr;
> +	bridge->ops = &mobiveil_pcie_ops;
> +	bridge->map_irq = of_irq_parse_and_map_pci;
> +	bridge->swizzle_irq = pci_common_swizzle;
> +
> +	/* setup the kernel resources for the newly added PCIe root bus */
> +	ret = pci_scan_root_bus_bridge(bridge);
> +	if (ret)
> +		goto error;
> +
> +	bus = bridge->bus;
> +
> +	pci_assign_unassigned_bus_resources(bus);
> +	list_for_each_entry(child, &bus->children, node)
> +		pcie_bus_configure_settings(child);
> +	pci_bus_add_devices(bus);
> +
> +	platform_set_drvdata(pdev, pcie);

Is this really needed ?

Thanks,
Lorenzo

> +
> +	return 0;
> +error:
> +	pci_free_resource_list(&pcie->resources);
> +	return ret;
> +}
> +
> +static const struct of_device_id mobiveil_pcie_of_match[] = {
> +	{.compatible = "mbvl,gpex40-pcie",},
> +};
> +
> +MODULE_DEVICE_TABLE(of, mobiveil_pcie_of_match);
> +
> +static struct platform_driver mobiveil_pcie_driver = {
> +	.probe = mobiveil_pcie_probe,
> +	.driver = {
> +			.name = "mobiveil-pcie",
> +			.of_match_table = mobiveil_pcie_of_match,
> +			.suppress_bind_attrs = true,
> +		},
> +};
> +
> +builtin_platform_driver(mobiveil_pcie_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Mobiveil PCIe host controller driver");
> +MODULE_AUTHOR("Subrahmanya Lingappa <l.subrahmanya-DTHOJn6Rh8lhmhkoCovsdw@public.gmane.org>");
> -- 
> 1.8.3.1
> 
--
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

* [PATCH v5 1/1] eeprom: at24: convert magic numbers to structs.
From: Sven Van Asbroeck @ 2017-12-20 16:48 UTC (permalink / raw)
  To: svendev, wsa, brgl, nsekhar, sakari.ailus, javier,
	divagar.mohandass
  Cc: devicetree, linux-kernel, linux-i2c
In-Reply-To: <1513788536-7672-1-git-send-email-svendev@arcx.com>

Fundamental properties such as capacity and page size differ
among at24-type chips. But these chips do not have an id register,
so this can't be discovered at runtime.

Traditionally, at24-type eeprom properties were determined in two ways:
- by passing a 'struct at24_platform_data' via platform_data, or
- by naming the chip type in the devicetree, which passes a
	'magic number' to probe(), which is then converted to
	a 'struct at24_platform_data'.

Recently a bug was discovered because the magic number rounds down
all chip sizes to the lowest power of two. This was addressed by
a work-around, with the wish that magic numbers should over time
be converted to structs.
commit 5478e478eee3 ("eeprom: at24: correctly set the size for at24mac402")

This patch replaces the magic numbers with 'struct at24_chip_data'.

Signed-off-by: Sven Van Asbroeck <svendev@arcx.com>
---
 drivers/misc/eeprom/at24.c | 221 ++++++++++++++++++++-------------------------
 1 file changed, 100 insertions(+), 121 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index b44a3d2b..b1f78b9 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -105,16 +105,6 @@ struct at24_data {
 module_param_named(write_timeout, at24_write_timeout, uint, 0);
 MODULE_PARM_DESC(at24_write_timeout, "Time (in ms) to try writes (default 25)");
 
-#define AT24_SIZE_BYTELEN 5
-#define AT24_SIZE_FLAGS 8
-
-#define AT24_BITMASK(x) (BIT(x) - 1)
-
-/* create non-zero magic value for given eeprom parameters */
-#define AT24_DEVICE_MAGIC(_len, _flags)			\
-	((1 << AT24_SIZE_FLAGS | (_flags))		\
-	    << AT24_SIZE_BYTELEN | ilog2(_len))
-
 /*
  * Both reads and writes fail if the previous write didn't complete yet. This
  * macro loops a few times waiting at least long enough for one entire page
@@ -132,113 +122,108 @@ struct at24_data {
 	     op_time ? time_before(op_time, tout) : true;		\
 	     usleep_range(1000, 1500), op_time = jiffies)
 
+struct at24_chip_data {
+	/*
+	 * these fields mirror their equivalents in
+	 * struct at24_platform_data
+	 */
+	u32 byte_len;
+	u8 flags;
+};
+
+#define AT24_CHIP_DATA(_name, _len, _flags)				\
+	static const struct at24_chip_data _name = {			\
+		.byte_len = _len, .flags = _flags,			\
+	}
+
+/* needs 8 addresses as A0-A2 are ignored */
+AT24_CHIP_DATA(at24_data_24c00, 128 / 8, AT24_FLAG_TAKE8ADDR);
+/* old variants can't be handled with this generic entry! */
+AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0);
+AT24_CHIP_DATA(at24_data_24cs01, 16,
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, 0);
+AT24_CHIP_DATA(at24_data_24cs02, 16,
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24mac402, 48 / 8,
+	AT24_FLAG_MAC | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24mac602, 64 / 8,
+	AT24_FLAG_MAC | AT24_FLAG_READONLY);
+/* spd is a 24c02 in memory DIMMs */
+AT24_CHIP_DATA(at24_data_spd, 2048 / 8,
+	AT24_FLAG_READONLY | AT24_FLAG_IRUGO);
+AT24_CHIP_DATA(at24_data_24c04, 4096 / 8, 0);
+AT24_CHIP_DATA(at24_data_24cs04, 16,
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+/* 24rf08 quirk is handled at i2c-core */
+AT24_CHIP_DATA(at24_data_24c08, 8192 / 8, 0);
+AT24_CHIP_DATA(at24_data_24cs08, 16,
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24c16, 16384 / 8, 0);
+AT24_CHIP_DATA(at24_data_24cs16, 16,
+	AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24c32, 32768 / 8, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24cs32, 16,
+	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24c64, 65536 / 8, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24cs64, 16,
+	AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL | AT24_FLAG_READONLY);
+AT24_CHIP_DATA(at24_data_24c128, 131072 / 8, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24c256, 262144 / 8, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24c512, 524288 / 8, AT24_FLAG_ADDR16);
+AT24_CHIP_DATA(at24_data_24c1024, 1048576 / 8, AT24_FLAG_ADDR16);
+/* identical to 24c08 ? */
+AT24_CHIP_DATA(at24_data_INT3499, 8192 / 8, 0);
+
 static const struct i2c_device_id at24_ids[] = {
-	/* needs 8 addresses as A0-A2 are ignored */
-	{ "24c00",	AT24_DEVICE_MAGIC(128 / 8,	AT24_FLAG_TAKE8ADDR) },
-	/* old variants can't be handled with this generic entry! */
-	{ "24c01",	AT24_DEVICE_MAGIC(1024 / 8,	0) },
-	{ "24cs01",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
-	{ "24c02",	AT24_DEVICE_MAGIC(2048 / 8,	0) },
-	{ "24cs02",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
-	{ "24mac402",	AT24_DEVICE_MAGIC(48 / 8,
-				AT24_FLAG_MAC | AT24_FLAG_READONLY) },
-	{ "24mac602",	AT24_DEVICE_MAGIC(64 / 8,
-				AT24_FLAG_MAC | AT24_FLAG_READONLY) },
-	/* spd is a 24c02 in memory DIMMs */
-	{ "spd",	AT24_DEVICE_MAGIC(2048 / 8,
-				AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
-	{ "24c04",	AT24_DEVICE_MAGIC(4096 / 8,	0) },
-	{ "24cs04",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
-	/* 24rf08 quirk is handled at i2c-core */
-	{ "24c08",	AT24_DEVICE_MAGIC(8192 / 8,	0) },
-	{ "24cs08",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
-	{ "24c16",	AT24_DEVICE_MAGIC(16384 / 8,	0) },
-	{ "24cs16",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
-	{ "24c32",	AT24_DEVICE_MAGIC(32768 / 8,	AT24_FLAG_ADDR16) },
-	{ "24cs32",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_ADDR16 |
-				AT24_FLAG_SERIAL |
-				AT24_FLAG_READONLY) },
-	{ "24c64",	AT24_DEVICE_MAGIC(65536 / 8,	AT24_FLAG_ADDR16) },
-	{ "24cs64",	AT24_DEVICE_MAGIC(16,
-				AT24_FLAG_ADDR16 |
-				AT24_FLAG_SERIAL |
-				AT24_FLAG_READONLY) },
-	{ "24c128",	AT24_DEVICE_MAGIC(131072 / 8,	AT24_FLAG_ADDR16) },
-	{ "24c256",	AT24_DEVICE_MAGIC(262144 / 8,	AT24_FLAG_ADDR16) },
-	{ "24c512",	AT24_DEVICE_MAGIC(524288 / 8,	AT24_FLAG_ADDR16) },
-	{ "24c1024",	AT24_DEVICE_MAGIC(1048576 / 8,	AT24_FLAG_ADDR16) },
-	{ "at24", 0 },
+	{ "24c00",	(kernel_ulong_t)&at24_data_24c00 },
+	{ "24c01",	(kernel_ulong_t)&at24_data_24c01 },
+	{ "24cs01",	(kernel_ulong_t)&at24_data_24cs01 },
+	{ "24c02",	(kernel_ulong_t)&at24_data_24c02 },
+	{ "24cs02",	(kernel_ulong_t)&at24_data_24cs02 },
+	{ "24mac402",	(kernel_ulong_t)&at24_data_24mac402 },
+	{ "24mac602",	(kernel_ulong_t)&at24_data_24mac602 },
+	{ "spd",	(kernel_ulong_t)&at24_data_spd },
+	{ "24c04",	(kernel_ulong_t)&at24_data_24c04 },
+	{ "24cs04",	(kernel_ulong_t)&at24_data_24cs04 },
+	{ "24c08",	(kernel_ulong_t)&at24_data_24c08 },
+	{ "24cs08",	(kernel_ulong_t)&at24_data_24cs08 },
+	{ "24c16",	(kernel_ulong_t)&at24_data_24c16 },
+	{ "24cs16",	(kernel_ulong_t)&at24_data_24cs16 },
+	{ "24c32",	(kernel_ulong_t)&at24_data_24c32 },
+	{ "24cs32",	(kernel_ulong_t)&at24_data_24cs32 },
+	{ "24c64",	(kernel_ulong_t)&at24_data_24c64 },
+	{ "24cs64",	(kernel_ulong_t)&at24_data_24cs64 },
+	{ "24c128",	(kernel_ulong_t)&at24_data_24c128 },
+	{ "24c256",	(kernel_ulong_t)&at24_data_24c256 },
+	{ "24c512",	(kernel_ulong_t)&at24_data_24c512 },
+	{ "24c1024",	(kernel_ulong_t)&at24_data_24c1024 },
+	{ "at24",	0 },
 	{ /* END OF LIST */ }
 };
 MODULE_DEVICE_TABLE(i2c, at24_ids);
 
 static const struct of_device_id at24_of_match[] = {
-	{
-		.compatible = "atmel,24c00",
-		.data = (void *)AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR)
-	},
-	{
-		.compatible = "atmel,24c01",
-		.data = (void *)AT24_DEVICE_MAGIC(1024 / 8, 0)
-	},
-	{
-		.compatible = "atmel,24c02",
-		.data = (void *)AT24_DEVICE_MAGIC(2048 / 8, 0)
-	},
-	{
-		.compatible = "atmel,spd",
-		.data = (void *)AT24_DEVICE_MAGIC(2048 / 8,
-				AT24_FLAG_READONLY | AT24_FLAG_IRUGO)
-	},
-	{
-		.compatible = "atmel,24c04",
-		.data = (void *)AT24_DEVICE_MAGIC(4096 / 8, 0)
-	},
-	{
-		.compatible = "atmel,24c08",
-		.data = (void *)AT24_DEVICE_MAGIC(8192 / 8, 0)
-	},
-	{
-		.compatible = "atmel,24c16",
-		.data = (void *)AT24_DEVICE_MAGIC(16384 / 8, 0)
-	},
-	{
-		.compatible = "atmel,24c32",
-		.data = (void *)AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16)
-	},
-	{
-		.compatible = "atmel,24c64",
-		.data = (void *)AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16)
-	},
-	{
-		.compatible = "atmel,24c128",
-		.data = (void *)AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16)
-	},
-	{
-		.compatible = "atmel,24c256",
-		.data = (void *)AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16)
-	},
-	{
-		.compatible = "atmel,24c512",
-		.data = (void *)AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16)
-	},
-	{
-		.compatible = "atmel,24c1024",
-		.data = (void *)AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16)
-	},
-	{ },
+	{ .compatible = "atmel,24c00",		.data = &at24_data_24c00 },
+	{ .compatible = "atmel,24c01",		.data = &at24_data_24c01 },
+	{ .compatible = "atmel,24c02",		.data = &at24_data_24c02 },
+	{ .compatible = "atmel,spd",		.data = &at24_data_spd },
+	{ .compatible = "atmel,24c04",		.data = &at24_data_24c04 },
+	{ .compatible = "atmel,24c08",		.data = &at24_data_24c08 },
+	{ .compatible = "atmel,24c16",		.data = &at24_data_24c16 },
+	{ .compatible = "atmel,24c32",		.data = &at24_data_24c32 },
+	{ .compatible = "atmel,24c64",		.data = &at24_data_24c64 },
+	{ .compatible = "atmel,24c128",		.data = &at24_data_24c128 },
+	{ .compatible = "atmel,24c256",		.data = &at24_data_24c256 },
+	{ .compatible = "atmel,24c512",		.data = &at24_data_24c512 },
+	{ .compatible = "atmel,24c1024",	.data = &at24_data_24c1024 },
+	{ /* END OF LIST */ },
 };
 MODULE_DEVICE_TABLE(of, at24_of_match);
 
 static const struct acpi_device_id at24_acpi_ids[] = {
-	{ "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) },
-	{ }
+	{ "INT3499",	(kernel_ulong_t)&at24_data_INT3499 },
+	{ /* END OF LIST */ }
 };
 MODULE_DEVICE_TABLE(acpi, at24_acpi_ids);
 
@@ -516,8 +501,8 @@ static unsigned int at24_get_offset_adj(u8 flags, unsigned int byte_len)
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
-	struct at24_platform_data chip;
-	kernel_ulong_t magic = 0;
+	struct at24_platform_data chip = { 0 };
+	const struct at24_chip_data *cd = NULL;
 	bool writable;
 	struct at24_data *at24;
 	int err;
@@ -535,28 +520,22 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		 */
 		if (client->dev.of_node &&
 		    of_match_device(at24_of_match, &client->dev)) {
-			magic = (kernel_ulong_t)
-				of_device_get_match_data(&client->dev);
+			cd = of_device_get_match_data(&client->dev);
 		} else if (id) {
-			magic = id->driver_data;
+			cd = (void *)id->driver_data;
 		} else {
 			const struct acpi_device_id *aid;
 
 			aid = acpi_match_device(at24_acpi_ids, &client->dev);
 			if (aid)
-				magic = aid->driver_data;
+				cd = (void *)aid->driver_data;
 		}
-		if (!magic)
+		if (!cd)
 			return -ENODEV;
 
-		chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
-		magic >>= AT24_SIZE_BYTELEN;
-		chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
-
+		chip.byte_len = cd->byte_len;
+		chip.flags = cd->flags;
 		at24_get_pdata(&client->dev, &chip);
-
-		chip.setup = NULL;
-		chip.context = NULL;
 	}
 
 	if (!is_power_of_2(chip.byte_len))
-- 
1.9.1

^ 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