Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/5] fbdev: ssd1307fb: Remove reset-active-low from the DT binding document
From: Rob Herring @ 2017-01-18 22:28 UTC (permalink / raw)
  To: Jyri Sarha
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ, tomi.valkeinen-l0cyMroinI0,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	bcousson-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <b68e1b9b735543b9e1511cfeb8e5d9ceec6c7666.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>

On Fri, Jan 13, 2017 at 12:35:46PM +0200, Jyri Sarha wrote:
> Remove reset-active-low from the devicetree binding document. The actual
> implementation has never been there in the driver code and there is no
> reason to add it because the gpiod API supports gpio flags, including
> GPIO_ACTIVE_LOW, directly trough its own devicetree binding.
> 
> Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/display/ssd1307fb.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> index eb31ed4..4aee67f 100644
> --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> @@ -8,14 +8,14 @@ Required properties:
>           0x3c or 0x3d
>    - pwm: Should contain the pwm to use according to the OF device tree PWM
>           specification [0]. Only required for the ssd1307.
> -  - reset-gpios: Should contain the GPIO used to reset the OLED display
> +  - reset-gpios: Should contain the GPIO used to reset the OLED display. See
> +                 Documentation/devicetree/bindings/gpio/gpio.txt for details.

You need to define the active state. Does the active state actually 
vary? Sounds like the compatible is not specific enough unless some 
boards have an inverter.

>    - solomon,height: Height in pixel of the screen driven by the controller
>    - solomon,width: Width in pixel of the screen driven by the controller
>    - solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
>      mapped to.
>  
>  Optional properties:
> -  - reset-active-low: Is the reset gpio is active on physical low?
>    - solomon,segment-no-remap: Display needs normal (non-inverted) data column
>                                to segment mapping
>    - solomon,com-seq: Display uses sequential COM pin configuration
> -- 
> 1.9.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

* Re: [PATCH 2/2] ARM64: dts: meson-gxm-rbox-pro: Enable Bluetooth
From: Kevin Hilman @ 2017-01-18 22:29 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
	linux-kernel, Rob Herring, Carlo Caione, linux-amlogic,
	linux-arm-kernel
In-Reply-To: <20170117030611.23827-3-afaerber@suse.de>

Andreas Färber <afaerber@suse.de> writes:

> Add an SDIO reset GPIO and enable the serial used by the AP6255
> Bluetooth module. Based on work by Martin Blumenstingl.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
> index 9f04fa4e5aec..6ea225f584bd 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts
> @@ -58,6 +58,7 @@
>  
>  	aliases {
>  		serial0 = &uart_AO;
> +		serial1 = &uart_A;
>  	};
>  
>  	chosen {
> @@ -122,7 +123,8 @@
>  
>  	sdio_pwrseq: sdio-pwrseq {
>  		compatible = "mmc-pwrseq-simple";
> -		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>;
> +		reset-gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>,
> +				<&gpio GPIOX_17 GPIO_ACTIVE_LOW>;
>  		clocks = <&wifi32k>;
>  		clock-names = "ext_clock";

c.f. thread on Martin's series.   I don't like this (ab)use of
reset-gpios for sdio-pwrseq, and there's some work coming that will make
this much cleaner.

So, I'll hold off on this until the other solution is ready.

Kevin 

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

^ permalink raw reply

* Re: [PATCH 2/2] ARM64: dts: meson-gx: Add HDMI HPD/DDC pinctrl nodes
From: Kevin Hilman @ 2017-01-18 22:32 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484654738-5496-3-git-send-email-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:

> Add pinctrl nodes for HDMI HPD and DDC pins modes for Amlogic Meson GXL
> and GXBB SoCs.
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Applied to v4.11/dt64 with Linus' ack.

Kevin
--
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 4/5] fbdev/ssd1307fb: add support to enable VBAT
From: Rob Herring @ 2017-01-18 22:33 UTC (permalink / raw)
  To: Jyri Sarha
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ, tomi.valkeinen-l0cyMroinI0,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	bcousson-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <214ae9ca86009148f233be71a3606917f3150dcd.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>

On Fri, Jan 13, 2017 at 12:35:48PM +0200, Jyri Sarha wrote:
> From: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> 
> SSD1306 needs VBAT when it is wired in charge pump configuration. This
> patch adds support to the driver to enable VBAT regulator at init time.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> Reviewed-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/display/ssd1307fb.txt        |  1 +
>  drivers/video/fbdev/ssd1307fb.c                      | 20 +++++++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> index 6617df6..209d931 100644
> --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> @@ -16,6 +16,7 @@ Required properties:
>  Optional properties:
>    - reset-gpios: The GPIO used to reset the OLED display, if available. See
>                   Documentation/devicetree/bindings/gpio/gpio.txt for details.
> +  - vbat-supply: The supply for VBAT

According to the datasheet, SSD1307 has 2 supplies: Vdd and Vcc

I don't see any mention of a charge pump, so that must be an external 
component.

>    - solomon,segment-no-remap: Display needs normal (non-inverted) data column
>                                to segment mapping
>    - solomon,com-seq: Display uses sequential COM pin configuration
--
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/2] eeprom: Add IDT 89HPESx driver bindings file
From: Rob Herring @ 2017-01-18 22:34 UTC (permalink / raw)
  To: Serge Semin
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
	mark.rutland-5wv7dgnIgG8, Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484309813-25008-3-git-send-email-fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, Jan 13, 2017 at 03:16:53PM +0300, Serge Semin wrote:
> IDT 89HPESx PCIe-switches exposes SMBus interface to have an access to
> the device CSRs and EEPROM. So to properly utilize the interface
> functionality, developer should declare a valid dts-file node, which
> would refer to the corresponding 89HPESx device.
> 
> Signed-off-by: Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/idt_89hpesx.txt       | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt

Acked-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 v3 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
From: Rob Herring @ 2017-01-18 22:38 UTC (permalink / raw)
  To: Sergio Prado
  Cc: mark.rutland, devicetree, ulf.hansson, linux-mmc, linux-kernel,
	ben-linux, linux-arm-kernel
In-Reply-To: <1484319953-6479-2-git-send-email-sergio.prado@e-labworks.com>

On Fri, Jan 13, 2017 at 01:05:52PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C24XX
> MMC/SD/SDIO controller, used as a connectivity interface with external
> MMC, SD and SDIO storage mediums.
> 
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
>  .../devicetree/bindings/mmc/samsung,s3cmci.txt     | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> new file mode 100644
> index 000000000000..d09dbf4b3824
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> @@ -0,0 +1,34 @@
> +* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
> +
> +Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
> +with external MMC, SD and SDIO storage mediums.
> +
> +This file documents differences between the core mmc properties described by
> +mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
> +implementation.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> +  - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
> +  - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
> +  - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
> +- clocks: Should reference the controller clock
> +- clock-names: Should contain "sdi"

You are missing things from the example.

> +
> +Example:
> +	mmc0: mmc@5a000000 {
> +		compatible = "samsung,s3c2440-sdi";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sdi_pins>;
> +		reg = <0x5a000000 0x100000>;
> +		interrupts = <0 0 21 3>;
> +		clocks = <&clocks PCLK_SDI>;
> +		clock-names = "sdi";
> +		bus-width = <4>;
> +		cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
> +		wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	Note: This example shows both SoC specific and board specific properties
> +	in a single device node. The properties can be actually be separated
> +	into SoC specific node and board specific node.

The source structure is not relavent to the binding.

Rob

^ permalink raw reply

* Re: [PATCH 2/3] Documentation: devicetree: amlogic: Add R-Box Pro
From: Andreas Färber @ 2017-01-18 22:38 UTC (permalink / raw)
  To: Neil Armstrong, linux-amlogic, Kevin Hilman, Carlo Caione
  Cc: Mark Rutland, devicetree, Rob Herring, linux-kernel,
	linux-arm-kernel
In-Reply-To: <819ac0c0-92b7-bac7-dcfc-a56c34a6bf62@baylibre.com>

Am 18.01.2017 um 10:35 schrieb Neil Armstrong:
> On 01/17/2017 11:54 PM, Andreas Färber wrote:
>> Cc: ada@kingnoval.com
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  Originally I thought we would group by SoC (6, 8, 8b, gxbb, gxl, gmx, etc.)
>>  but this got out of order with nexbox,a95x - so inserting kingnovel between
>>  amlogic and nexbox here. If that's what we want going forward, we should move
>>  the old entries to make the scheme clearer. The alternative would be to
>>  reorder alphabetically within each SoC group, but nexbox,a95x with two SoCs
>>  makes it difficult to categorize, we could choose gxbb as the earlier one.
> 
> Andreas,
> 
> Feel free to provide a fixup.

Sure, the question is which way. ;) Kevin? Carlo?

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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

^ permalink raw reply

* Re: [PATCH v3 2/3] Documentation: devicetree: Add document bindings for mtk-cir
From: Rob Herring @ 2017-01-18 22:42 UTC (permalink / raw)
  To: Sean Wang
  Cc: Mauro Carvalho Chehab, Hans de Goede, Heiner Kallweit,
	Mark Rutland, Matthias Brugger, Andi Shyti, Hans Verkuil,
	Sean Young, Ivaylo Dimitrov, linux-media@vger.kernel.org,
	devicetree@vger.kernel.org, linux-mediatek,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, sean wang
In-Reply-To: <20170118222321.ec6p7oqwwqfiofhp@rob-hp-laptop>

On Wed, Jan 18, 2017 at 4:23 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Jan 13, 2017 at 03:35:38PM +0800, sean.wang@mediatek.com wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> This patch adds documentation for devicetree bindings for
>> consumer Mediatek IR controller.
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  .../devicetree/bindings/media/mtk-cir.txt          | 24 ++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/mtk-cir.txt
>
> Acked-by: Rob Herring <robh@kernel.org>

I guess I rescind my ack if this email doesn't work:

"The response from the remote server was:

550 Relaying mail to sean.wang@mediatek.com is not allowed"

^ permalink raw reply

* Re: [PATCH v7 2/3] input: tm2-touchkey: Add touchkey driver support for TM2
From: Dmitry Torokhov @ 2017-01-18 22:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, Chanwoo Choi, Catalin Marinas,
	Jaechul Lee, Will Deacon, lkml, Rob Herring,
	Javier Martinez Canillas, Kukjin Kim, Andi Shyti,
	linux-input@vger.kernel.org, 이재철, Beomho Seo,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170117181056.mrwyudslfgh2wfms@kozik-lap>

On Tue, Jan 17, 2017 at 08:10:56PM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jan 17, 2017 at 10:06:27AM -0800, Dmitry Torokhov wrote:
> > On Tue, Jan 17, 2017 at 9:20 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > On Tue, Jan 17, 2017 at 02:54:38PM +0900, Jaechul Lee wrote:
> > >> This patch adds support for the TM2 touch key and led
> > >> functionality.
> > >>
> > >> The driver interfaces with userspace through an input device and
> > >> reports KEY_PHONE and KEY_BACK event types. LED brightness can be
> > >> controlled by "/sys/class/leds/tm2-touchkey/brightness".
> > >>
> > >> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> > >> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> > >> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> > >> Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
> > >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> > >> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
> > >> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> > >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >
> > > This looks unusual. How did Dmitry's Sob end here?
> > 
> > I sent Jaechul a version of the patch to try out.
> 
> Ah, makes sense then.

I picked up (and folded) the binding doc and driver patches, DTS should
go through some other tree I believe.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/2] input: touchscreen: add driver for Zeitec ZET6223
From: Dmitry Torokhov @ 2017-01-18 22:49 UTC (permalink / raw)
  To: Jelle van der Waa; +Cc: Rob Herring, linux-input, devicetree
In-Reply-To: <20170118205712.GA5333@gmail.com>

On Wed, Jan 18, 2017 at 09:57:14PM +0100, Jelle van der Waa wrote:
> On 01/14/17 at 11:14am, Dmitry Torokhov wrote:
> > > +
> > > +static const struct of_device_id zet6223_of_match[] = {
> > > +	{ .compatible = "zeitec", "zet6223" },
> > 
> > The compatible should be "zeitec,zet6223", what you have here is
> > equivalent of:
> > 
> > 	{ .compatible = "zeitec", .data = "zet6223" },
> 
> Ah that was silly, thanks :)
> 
> > I also have been looking at you previosu submission and had some draft
> > changes. I reconciled them in the patch below, if it still works for you
> > then I'll fold everything together and apply. Please let me know.
> 
> I'll find some time to test the patch below, I've one small suggestion
> to the patch below.
> 
> > +	ts->fingernum = buf[15] & 0x7F;
> > +	if (ts->fingernum > ZET6223_MAX_FINGERS) {
> > +		dev_warn(&ts->client->dev,
> > +			 "touchpanel reports %d fingers, limiting to %d\n",
> > +			 ts->fingernum, ZET6223_MAX_FINGERS);
> > +		ts->fingernum = 16;
> 
> Maybe use ZET6223_MAX_FINGERS?

Makes sense, I'll change before applying.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/3] ARM64: dts: meson-gxbb: Add support for WeTek Hub and Play
From: Kevin Hilman @ 2017-01-18 22:53 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: carlo, linux-amlogic, linux-kernel, linux-arm-kernel, devicetree
In-Reply-To: <1484736642-5451-3-git-send-email-narmstrong@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> Adds support for the WeTek Hub and Play2 boards.
> The Hub is an extremely small IPTv Set-Top-Box and the Play2 is a more
> traditionnal Satellite or Terrestrial and IPTv Set-Top-Box.
>
> Both are based on the p200 Reference Design and out-of-tree support is
> based on OpenELEC kernel at [1].
>
> [1] https://github.com/wetek-enigma/linux-amlogic
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Did you get the GPIO/button info from the LibreELEC tree or did you find
some schematics someplace?  If you found any schematics, it would be
nice to have links in the DTS files.

Kevin

^ permalink raw reply

* Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Rob Herring @ 2017-01-18 23:03 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Nishanth Menon, devicetree@vger.kernel.org, Ulf Hansson,
	Santosh Shilimkar, Dave Gerlach, Lokesh Vutla, Keerthy,
	linux-pm@vger.kernel.org, Rafael J . Wysocki,
	linux-kernel@vger.kernel.org, Tero Kristo, Russell King,
	Sudeep Holla, linux-arm-kernel@lists.infradead.org
In-Reply-To: <m2ziip2puf.fsf@baylibre.com>

On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>> On 17/01/17 00:12, Dave Gerlach wrote:
>>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:

[...]

>>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>>> should not
>>>>> go in the power-domains cells and go against what the power-domains
>>>>> binding
>>>>> says that the cell expects. We have one single power domain, and the new
>>>>> ti,sci-id binding is not something the genpd framework itself is
>>>>> concerned
>>>>> with as it's our property to identify a device inside a power domain,
>>>>> not to
>>>>> identify which power domain it is associated with.
>>>>
>>>> What is the id used for? I can understand why you need to know what
>>>> power domain a device is in (as power-domains identifies), but not
>>>> what devices are in a power domain.
>>>
>>> We have a system control processor that provides power management
>>> services to the OS and it responsible for handling the power state of
>>> each device. This control happens over a communication interface we have
>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>>> communication protocol uses these ids to identify each device within the
>>> power domain so that the control processor can do what is necessary to
>>> enable that device.
>>
>> I think a minor detail here that Rob might be missing right now is,
>> that the ti,sci-id is only controlling the PM runtime handling, and
>> providing the ID per-device for this purpose only. AFAIK, it is not
>> really connected to the power domain anymore as such, as we don't have
>> power-domains / per device anymore as was the case in some earlier
>> revision of this work.
>
> I think this gets to the heart of things.  IMO The confusion arises
> because we're throwing around the term "power domain" when there isn't
> an actual hardware power domain here.

I thought there was 1.

> Unfortunately, the genpd bindings have used the terminology power-domain
> when in fact genpd is more generic than that and can be used not just
> for hardware power domains, but for arbitrary grouping of devices that
> have common PM properties.  That's why genpd actually stands for generic
> PM domain, not power domain.  Unfortunately, the bindings have grown
> primarily out of the usage for hardware power domains.

Now it makes some sense.

So the question is does this PM domain grouping need to be described
in DT or not, and if so what does that look like?

We could continue to use the power domain binding (maybe we already
are and that ship has sailed). I'm not totally against the idea even
if there is no power domain, but I'm not sold on it either. If we do
go this route, then I still say the id should be a cell in the
power-domain phandle.

Another option is create something new either common or TI SCI
specific. It could be just a table of ids and phandles in the SCI
node. I'm much more comfortable with an isolated property in one node
than something scattered throughout the DT.

>> One could argue though that the whole usage of power-domains is now
>> moot, as we basically only have implemented one genpd in the whole
>> SoC, which doesn't really reflect the reality. I wonder if better
>> approach would be to have this replaced with proper power domains at
>> some point (if needed), and just have a runtime-pm implementation in
>> place for the devices that require it.
>>
>> So, as an example in DT, we would only have:
>>
>> uart0: serial@02530c00 {
>>   compatible = "xyz";
>>   ...
>>   ti,sci-id = <K2G_DEV_UART0>;
>> };
>>
>> This is somewhat analogous to what OMAP family of SoCs have in place
>> now, under "ti,hwmods" property. I also wonder if the "ti,sci-id"
>> should be replaced with something like "ti,sci-dev-id" to make its
>> purpose clearer.
>
> Unless I'm missing something, that still begs the question of who reads
> that property and takes care of the call into TI-SCI though.
>
> Kevin

^ permalink raw reply

* Re: [PATCH v5] ARM64: dts: meson-gx: Add firmware reserved memory zones
From: Kevin Hilman @ 2017-01-18 23:04 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, xypron.glpk, linux-kernel, carlo, linux-amlogic,
	afaerber, linux-arm-kernel
In-Reply-To: <1484758245-9354-1-git-send-email-narmstrong@baylibre.com>

Neil Armstrong <narmstrong@baylibre.com> writes:

> The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
> this patch adds these reserved zones.

Applying to v4.10/fixes for broader testing.

Will wait for ack from Andreas befores submitting to arm-soc fixes.

Kevin

> Without such reserved memory zones, running the following stress command :
> $ stress-ng --vm 16 --vm-bytes 128M --timeout 10s
> multiple times:
>
> Could lead to the following kernel crashes :
> [   46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
> ...
> [   47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
> ...
> Instead of the OOM killer.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> Changes since v4 at [5]:
> - Move start of ddr memory to reserved-memory node
> - Drop memory node move
> - Fix typo in sizes
>
> Changes since resent v2 at [4]:
> - Fix invalid comment of useable memory attributes
>
> Changes since original v2 at [3]:
> - Typo in commit 2GiB -> 1GiB, 4GiB -> 2GiB
>
> Changes since v2 at [2]:
> - Moved all memory node out of dtsi
> - Added comment about useable memory
> - Fixed comment about secmon reserved zone
>
> Changes since v1 at [1] :
> - Renamed reg into linux,usable-memory to ovveride u-boot memory
> - only kept secmon memory zone
>
> [1] http://lkml.kernel.org/r/20161212101801.28491-1-narmstrong@baylibre.com
> [2] http://lkml.kernel.org/r/1483105232-6242-1-git-send-email-narmstrong@baylibre.com
> [3] http://lkml.kernel.org/r/1484128128-22454-1-git-send-email-narmstrong@baylibre.com
> [4] http://lkml.kernel.org/r/1484128540-22662-1-git-send-email-narmstrong@baylibre.com
> [5] http://lkml.kernel.org/r/1484129414-23325-1-git-send-email-narmstrong@baylibre.com
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index eada0b5..63d52b7 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -55,6 +55,24 @@
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		/* 16 MiB reserved for Hardware ROM Firmware */
> +		hwrom: hwrom {
> +			reg = <0x0 0x0 0x0 0x1000000>;
> +			no-map;
> +		};
> +
> +		/* 2 MiB reserved for ARM Trusted Firmware (BL31) */
> +		secmon: secmon {
> +			reg = <0x0 0x10000000 0x0 0x200000>;
> +			no-map;
> +		};
> +	};
> +
>  	cpus {
>  		#address-cells = <0x2>;
>  		#size-cells = <0x0>;

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: gpu: Add Mali Utgard bindings
From: Linus Walleij @ 2017-01-18 23:09 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree@vger.kernel.org, Heiko Stuebner,
	Javier Martinez Canillas, Kevin Hilman, Antoine Ténart,
	Krzysztof Kozlowski, Matthias Brugger, Chen-Yu Tsai, Rob Herring,
	Alexandre Belloni, Kukjin Kim, Carlo Caione, Boris Brezillon,
	Thomas Petazzoni, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20170116132424.7038-1-maxime.ripard@free-electrons.com>

On Mon, Jan 16, 2017 at 2:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:

> The ARM Mali Utgard GPU family is embedded into a number of SoCs from
> Allwinner, Amlogic, Mediatek or Rockchip.
>
> Add a binding for the GPU of that family.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/gpu/arm,mali-utgard.txt    | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> new file mode 100644
> index 000000000000..df05ba0ec357
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
> @@ -0,0 +1,76 @@
> +ARM Mali Utgard GPU
> +===================
> +
> +Required properties:
> +  - compatible:
> +    * "arm,mali-utgard" and one of the following:
> +      + "arm,mali-300"
> +      + "arm,mali-400"
> +      + "arm,mali-450"
> +
> +  - reg: Physical base address and length of the GPU registers
> +
> +  - interrupts: an entry for each entry in interrupt-names.
> +    See ../interrupt-controller/interrupts.txt for details.
> +
> +  - interrupt-names:
> +    * ppX: Pixel Processor X interrupt (X from 0 to 7)
> +    * ppmmuX: Pixel Processor X MMU interrupt (X from 0 to 7)
> +    * pp: Pixel Processor broadcast interrupt (mali-450 only)
> +    * gp: Geometry Processor interrupt
> +    * gpmmu: Geometry Processor MMU interrupt
> +
> +
> +Optional properties:
> +  - interrupt-names:
> +    * pmu: Power Management Unit interrupt, if implemented in hardware

On the MALI-400 MP in the ST-Ericsson DB8500 we have an additional interrupt
called "Mali400 combined". This is simply the HW designer's
doing an OR over all the 4 IRQ lines. Is this useful to define in the
bindings? Then it should be an optional

 * combined: all lines OR:ed together (if available)

Also you are defining "resets" below in the examples, should
this be listed as an optional property?

> +The Mali GPU is integrated very differently from one SoC to
> +another. In order to accommodate those differences, you have the option
> +to specify one more vendor-specific compatible, among:
> +
> +  - allwinner,sun4i-a10-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU
> +
> +  - allwinner,sun7i-a20-mali
> +    Required properties:
> +      * clocks: an entry for each entry in clock-names
> +      * clock-names:
> +        + bus: bus clock for the GPU
> +        + core: clock driving the GPU itself
> +      * resets: phandle to the reset line for the GPU

Please add:

- stericsson,db8500-mali: also known as the "Smart Graphics
Accelerator" (SGA500)
   Required properties:
    * clocks: an entry for each entry in clock-names
    * clock-names:
      + bus: bus clock for the GPU (ICNCLK a.k.a. PRCMU_ACLK)
      + core: clock driving the GPU itself (PRCMU_SGACLK)

(It has no explicit reset line.)

With these:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 2/3] clk: ux500: Add device tree bindings for ABx500 clocks
From: Rob Herring @ 2017-01-18 23:09 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Michael Turquette, Stephen Boyd, linux-clk, devicetree,
	Ulf Hansson
In-Reply-To: <20170113150826.30202-1-linus.walleij@linaro.org>

On Fri, Jan 13, 2017 at 04:08:26PM +0100, Linus Walleij wrote:
> This adds device tree bindings for the ABx500 clocks on the
> ST-Ericsson platforms.
> 
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/clock/stericsson,abx500.txt         | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/stericsson,abx500.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/stericsson,abx500.txt b/Documentation/devicetree/bindings/clock/stericsson,abx500.txt
> new file mode 100644
> index 000000000000..8764d80e3412
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/stericsson,abx500.txt
> @@ -0,0 +1,13 @@
> +Clock bindings for ST-Ericsson ABx500 clocks
> +
> +Required properties :
> +- compatible : shall contain the following:
> +  "stericsson,ab8500-clk"
> +- #clock-cells should be <1>

Header for clock numbers?

> +
> +Example:
> +
> +ab8500_clock: ab8500-clock {

clock-controller {

or 

clocks {

> +	compatible = "stericsson,ab8500-clk";

No reg, they're all just fixed?

> +	#clock-cells = <1>;
> +};
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 1/4] dt-bindings: Add vendor prefix for LEGO
From: Rob Herring @ 2017-01-18 23:10 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland, devicetree, Kevin Hilman, Sekhar Nori, linux-kernel,
	linux-arm-kernel
In-Reply-To: <1484334222-14223-2-git-send-email-david@lechnology.com>

On Fri, Jan 13, 2017 at 01:03:39PM -0600, David Lechner wrote:
> Add a vendor prefix for LEGO Systems A/S
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

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

^ permalink raw reply

* Re: [PATCH v4 2/4] dt-bindings: Add LEGO MINDSTORMS EV3 compatible specification
From: Rob Herring @ 2017-01-18 23:11 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland, devicetree, Kevin Hilman, Sekhar Nori, linux-kernel,
	linux-arm-kernel
In-Reply-To: <1484334222-14223-3-git-send-email-david@lechnology.com>

On Fri, Jan 13, 2017 at 01:03:40PM -0600, David Lechner wrote:
> This adds the board level device tree specification for LEGO MINDSTORMS EV3
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  Documentation/devicetree/bindings/arm/davinci.txt | 4 ++++
>  1 file changed, 4 insertions(+)

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

^ permalink raw reply

* Re: [PATCH v4 3/4] dt-bindings: add "microchip,24c128" compatible string
From: Rob Herring @ 2017-01-18 23:11 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland, devicetree, Kevin Hilman, Sekhar Nori, linux-kernel,
	linux-arm-kernel
In-Reply-To: <1484334222-14223-4-git-send-email-david@lechnology.com>

On Fri, Jan 13, 2017 at 01:03:41PM -0600, David Lechner wrote:
> This adds "microchip,24c128" to the list of compatible strings for i2c
> eeproms.
> 
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  Documentation/devicetree/bindings/eeprom/eeprom.txt | 2 ++
>  1 file changed, 2 insertions(+)

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

^ permalink raw reply

* Re: [PATCH 4/4] drm: exynos: Remove support for Exynos4415 (SoC not supported anymore)
From: Rob Herring @ 2017-01-18 23:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Mark Rutland, devicetree, linux-samsung-soc, linux-kernel,
	Javier Martinez Canillas, Seung-Woo Kim, Michael Turquette,
	Stephen Boyd, Tomasz Figa, dri-devel, linux-clk, Chanwoo Choi,
	Kyungmin Park, Kukjin Kim, Sylwester Nawrocki, linux-gpio,
	linux-arm-kernel
In-Reply-To: <20170114123642.15581-5-krzk@kernel.org>

On Sat, Jan 14, 2017 at 02:36:42PM +0200, Krzysztof Kozlowski wrote:
> Support for Exynos4415 is going away because there are no internal nor
> external users.
> 
> Since commit 46dcf0ff0de3 ("ARM: dts: exynos: Remove exynos4415.dtsi"),
> the platform cannot be instantiated so remove also the drivers.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  .../devicetree/bindings/display/exynos/exynos_dsim.txt |  1 -
>  .../bindings/display/exynos/samsung-fimd.txt           |  1 -

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

>  drivers/gpu/drm/exynos/exynos_drm_dsi.c                | 15 +--------------
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c               | 18 ++----------------
>  4 files changed, 3 insertions(+), 32 deletions(-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Thomas Petazzoni @ 2017-01-18 23:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Boris Brezillon, Guochun Mao, devicetree,
	Richard Weinberger, Russell King, linux-kernel, Marek Vasut,
	linux-mtd, Matthias Brugger, linux-mediatek, Cyrille Pitchen,
	Brian Norris, David Woodhouse, linux-arm-kernel
In-Reply-To: <20170118222010.ivc6jxpnrumemvdf@rob-hp-laptop>

Hello,

On Wed, 18 Jan 2017 16:20:10 -0600, Rob Herring wrote:

> > > Rob, Mark, any opinion?  
> >  
> 
> Sigh, is how to do compatibles really not yet understood?

Well, it seems like not everyone necessarily understands what is the
best strategy to adopt (me included).

> > I agree that a clarification would be good. There are really two
> > options:
> > 
> >  1. Have two compatible strings in the DT, the one that matches the
> >     exact SoC where the IP is found (first compatible string) and the
> >     one that matches some other SoC where the same IP is found (second
> >     compatible string). Originally, Linux only supports the second
> >     compatible string in its device driver, but if it happens that a
> >     difference is found between two IPs that we thought were the same,
> >     we can add support for the first compatible string in the driver,
> >     with a slightly different behavior.  
> 
> This. And no wildcards in the compatible string. 

OK. So it means that today we do something like:

	compatible = "baz,foo-12", "baz,foo-00";

and support only baz,foo-00 in the driver. If tomorrow we discover
that there is in fact a difference between the two IP blocks, we can
add support for baz,foo-12 in the driver, and handle the differences.

But then, the DT still contains:

	compatible = "baz,foo-12", "baz,foo-00";

and therefore pretends that the IP block is compatible with
"baz,foo-00" which is in fact *not* the case. It was a mistake to
consider it as compatible. So we keep living with a DT that has
incorrect information.

> 
> >  2. Have a single compatible string in the DT, matching the exact SoC
> >     where the IP is found. This involves adding immediately this
> >     compatible string in the corresponding driver.  
> 
> I wouldn't object to this from a DT perspective as I have no clue 
> generally if IP blocks are "the same" or not. Subsystem maintainers will 
> object though.

Knowing if IP blocks are "the same" is in fact not necessarily trivial.
What appears to be identical IP blocks today might be discovered later
as actually having subtle differences (sometimes not even visible in
the datasheet).

> > I've not really been able to figure out which of the two options is the
> > most future-proof/appropriate.  
> 
> They are both future-proof. #2 has the disadvantage of requiring a 
> kernel update for a new SoC. 

Which is generally anyway needed because a new SoC will almost always
require some new drivers, adjusting pin-muxing or clock drivers, etc.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH 2/3] Documentation: devicetree: amlogic: Add R-Box Pro
From: Kevin Hilman @ 2017-01-18 23:38 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Mark Rutland, devicetree, Neil Armstrong, linux-kernel,
	Rob Herring, Carlo Caione, linux-amlogic, linux-arm-kernel
In-Reply-To: <9439ba55-c56c-71bf-00a5-870e2f4dd07b@suse.de>

Andreas Färber <afaerber@suse.de> writes:

> Am 18.01.2017 um 10:35 schrieb Neil Armstrong:
>> On 01/17/2017 11:54 PM, Andreas Färber wrote:
>>> Cc: ada@kingnoval.com
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  Originally I thought we would group by SoC (6, 8, 8b, gxbb, gxl, gmx, etc.)
>>>  but this got out of order with nexbox,a95x - so inserting kingnovel between
>>>  amlogic and nexbox here. If that's what we want going forward, we should move
>>>  the old entries to make the scheme clearer. The alternative would be to
>>>  reorder alphabetically within each SoC group, but nexbox,a95x with two SoCs
>>>  makes it difficult to categorize, we could choose gxbb as the earlier one.
>> 
>> Andreas,
>> 
>> Feel free to provide a fixup.
>
> Sure, the question is which way. ;) Kevin? Carlo?

I don't have a strong preference, but continuing to group by SoC makes
the most sense based on what we have there now.

Moving nexbox,a95x up to the first one (gxbb) is fine with me.

Kevin




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

^ permalink raw reply

* Re: [PATCH 01/13] Documentation: dt/bindings: Document pinctrl-ingenic
From: Linus Walleij @ 2017-01-18 23:45 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Rob Herring, Mark Rutland, Ralf Baechle, Ulf Hansson,
	Boris Brezillon, Thierry Reding, Bartlomiej Zolnierkiewicz,
	Maarten ter Huurne, Lars-Peter Clausen, Paul Burton,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linux MIPS,
	linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, linux-p
In-Reply-To: <20170117231421.16310-2-paul@crapouillou.net>

On Wed, Jan 18, 2017 at 12:14 AM, Paul Cercueil <paul@crapouillou.net> wrote:

> From: Paul Burton <paul.burton@imgtec.com>
>
> This commit adds documentation for the devicetree bidings of the
> pinctrl-ingenic driver, which handles pin configuration, pin muxing
> and GPIOs of the Ingenic SoCs currently supported by the Linux kernel.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

(...)

> +##### 'gpio-chips' sub-node #####
> +
> +The gpio-chips node will contain sub-nodes that correspond to GPIO controllers
> +(one sub-node per GPIO controller).
> +
> +Required properties:
> +- #address-cells: Should contain the integer 1.
> +- #size-cells: Should contain the integer 1.
> +- ranges: Should be empty.

I do not see why the GPIO needs a special subnode. Can the pin controller
and the GPIO not simply spawn from a single node?

> +##### GPIO controller node #####
> +
> +Each subnode of the 'gpio-chips' node is a GPIO controller node.
> +
> +Required properties:
> +- gpio-controller: Identifies this node as a GPIO controller.
> +- #gpio-cells: Should contain the integer 2.
> +- reg: Should contain the physical address and length of the GPIO controller's
> +  configuration registers.
> +
> +Optional properties:
> +- interrupt-controller: The GPIO controllers can optionally configure the
> +  GPIOs as interrupt sources. In this case, the 'interrupt-controller'
> +  standalone property should be supplied.
> +- #interrupt-cells: Required if 'interrupt-controller' is also specified.
> +  In that case, it should contain the integer 2.
> +- interrupts: Required if 'interrupt-controller' is also specified.
> +  In that case, it should contain the IRQ number of this GPIO controller.
> +- ingenic,pull-ups: A bit mask identifying the pins associated with this GPIO
> +  port which feature a pull-up resistor. The default mask is 0x0.
> +- ingenic,pull-downs: A bit mask identifying the pins associated with this GPIO
> +  port which feature a pull-down resistor. The default mask is 0x0.

So these bits tell us which lines have a pull up and pull down resistor?

Isn't that readily know from the compatible string? Then just hardcode
that into the driver for each variant, there is no need to define that in
the device tree.

> +##### Pin function node #####
> +
> +Each subnode of the 'functions' node is a pin function node.
> +
> +These subnodes represent a functionality of the SoC which may be exposed
> +through one or more groups of pins, represented as subnodes of the pin
> +function node. For example a function may be uart0, which may be exposed
> +through the group of pins PF0 to PF3.
> +
> +Required pin function node properties:
> +- None.
> +
> +
> +##### Pin group node #####
> +
> +Each subnode of a pin function node is a pin group node.
> +
> +Required pin group node properties:
> +- ingenic,pins: A set of values representing the pins within this pin group and
> +  their configuration.

Look into using the standard pins property from the pinctrl bindings
if yoy want to do this.

> Four values should be provided for each pin:
> +  - The phandle of the GPIO controller node for the GPIO port within which the
> +    pin is found.
> +  - The index of the pin within its GPIO port (an integer in the range 0 to 31
> +    inclusive).

This is already supported by gpio ranges, please do not reimplement
stuff we already have.

> +  - The index of the shared function port to be programmed in the GPIO port
> +    registers for this pin.

I don't see why this can not be stored in the driver.
But some people prefer to shovel everything into the device
tree, I don't know. Can you elaborate why this should be in the
device tree?

> +  - The phandle of a pin configuration node specifying the electrical
> +    configuration that should be applied to the pin.

Why? This is something the standard pin control states handles.
I'm confused.

> +For example the function 'msc0' may be exposed through 2 different pin groups,
> +one in GPIO port A and one in GPIO port E:
> +
> +  bias-configs {
> +    nobias: nobias {
> +      bias-disable;
> +    };
> +  };
> +
> +  functions {
> +    pinfunc_msc0: msc0 {
> +      pins_msc0_pa: msc0-pa {
> +        ingenic,pins = <&gpa  4 1 &nobias   /* d4 */
> +                        &gpa  5 1 &nobias   /* d5 */
> +                        &gpa  6 1 &nobias   /* d6 */
> +                        &gpa  7 1 &nobias   /* d7 */
> +                        &gpa 18 1 &nobias   /* clk */
> +                        &gpa 19 1 &nobias   /* cmd */
> +                        &gpa 20 1 &nobias   /* d0 */
> +                        &gpa 21 1 &nobias   /* d1 */
> +                        &gpa 22 1 &nobias   /* d2 */
> +                        &gpa 23 1 &nobias   /* d3 */
> +                        &gpa 24 1 &nobias>; /* rst */
> +      };

Please look at other bindings and drivers and read pinctrl.txt
closely. This makes no sense to me compared to other
examples.

This is something that seems to cross-mix gpio ranges
and pin config, that doesn't work for me, we can't have an
idiomatic binding like this. I understand that it may fit your
single usecase perfectly but it will be a maintenance nightmare
for me.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 05/13] MIPS: jz4740: DTS: Add node for the jz4740-pinctrl driver
From: Linus Walleij @ 2017-01-18 23:50 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Rob Herring, Mark Rutland, Ralf Baechle, Ulf Hansson,
	Boris Brezillon, Thierry Reding, Bartlomiej Zolnierkiewicz,
	Maarten ter Huurne, Lars-Peter Clausen, Paul Burton,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Linux MIPS,
	linux-mmc@vger.kernel.org, linux-mtd@lists.infradead.org, linux-p
In-Reply-To: <20170117231421.16310-6-paul@crapouillou.net>

On Wed, Jan 18, 2017 at 12:14 AM, Paul Cercueil <paul@crapouillou.net> wrote:

> For a description of the devicetree node, please read
> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

(...)

> +       pinctrl: ingenic-pinctrl@10010000 {
> +               compatible = "ingenic,jz4740-pinctrl";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +
> +               gpio-chips {
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges;
> +
> +                       gpa: gpa {
> +                               reg = <0x10010000 0x100>;
> +
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +
> +                               interrupt-controller;
> +                               #interrupt-cells = <2>;
> +
> +                               interrupt-parent = <&intc>;
> +                               interrupts = <28>;
> +
> +                               ingenic,pull-ups = <0xffffffff>;
> +                       };
> +
> +                       gpb: gpb {
> +                               reg = <0x10010100 0x100>;
> +
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +
> +                               interrupt-controller;
> +                               #interrupt-cells = <2>;
> +
> +                               interrupt-parent = <&intc>;
> +                               interrupts = <27>;
> +
> +                               ingenic,pull-ups = <0xffffffff>;
> +                       };
> +
> +                       gpc: gpc {
> +                               reg = <0x10010200 0x100>;
> +
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +
> +                               interrupt-controller;
> +                               #interrupt-cells = <2>;
> +
> +                               interrupt-parent = <&intc>;
> +                               interrupts = <26>;
> +
> +                               ingenic,pull-ups = <0xffffffff>;
> +                       };
> +
> +                       gpd: gpd {
> +                               reg = <0x10010300 0x100>;
> +
> +                               gpio-controller;
> +                               #gpio-cells = <2>;
> +
> +                               interrupt-controller;
> +                               #interrupt-cells = <2>;
> +
> +                               interrupt-parent = <&intc>;
> +                               interrupts = <25>;
> +
> +                               ingenic,pull-ups = <0xdfffffff>;
> +                       };
> +               };

Just pull all these down two levels and make them one device
each instead of having them inside the pin controller node
like this.

Then make a pin controller node separately, it can reference the
pin controller by phandles if necessary, and use the standard
gpio-ranges property to cross make GPIO and pin control.

It seems you driver is similar to for example the
drivers/pinctrl/nomadik/* pin controller.

Look in arch/arm/boot/dts/ste-dbx500.dtsi for examples,
NB: I'm not fully using standard bindings in it, because they
were not invented at the time.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 1/4] Documentation: dt: mailbox: Add Aspeed ast2400/2500 bindings
From: Cyril Bur @ 2017-01-19  0:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, andrew-zrmu5oMJ5Fs,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, xow-hpIqsD4AKlfQT0dZR+AlfA,
	jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170118203833.3htpccig67kpd6xl@rob-hp-laptop>

On Wed, 2017-01-18 at 14:38 -0600, Rob Herring wrote:
> On Thu, Jan 12, 2017 at 11:29:07AM +1100, Cyril Bur wrote:
> > Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/mailbox/aspeed-mbox.txt    | 44 ++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt b/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
> > new file mode 100644
> > index 000000000000..633cd534d91c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
> > @@ -0,0 +1,44 @@
> > +ASpeed Mailbox Driver
> > +=====================
> > +
> > +The ASpeed mailbox allows for communication between different
> > +processors. The mailbox on the ASpeed ast2400 and ast2500 is a set of
> > +16 single byte data registers along with interrupt and configuration
> > +registers directly on the SoC. These are memory mapped on the aspeed
> > +and can be accessed via the SuperIO registers on the other processor.
> > +
> > +Device Node:
> > +============
> > +This represents the mailbox on the Soc.
> > +
> > +As the mailbox registers sit on the LPC bus, it makes most sense for
> > +the device to be within the LPC host node. See
> > +Documentation/devicetree/bindings/mfd/aspeed-lpc.txt for more
> > +information. This does not have to be the case, provided the reg
> > +property can give the full address of the mbox registers.
> 
> This does have to be the case. I'd expect all devices on the LPC bus to 
> be under a LPC bus node.
> 
> Drop the last sentence, and:
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Will do, thanks for the review.

Cyril
--
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 2/4] Documentation: dt: misc: Add Aspeed ast2400/2500 LPC Control bindings
From: Cyril Bur @ 2017-01-19  0:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, andrew-zrmu5oMJ5Fs,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, xow-hpIqsD4AKlfQT0dZR+AlfA,
	jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170118211645.v54xvmxcsfzxior2@rob-hp-laptop>

On Wed, 2017-01-18 at 15:16 -0600, Rob Herring wrote:
> On Thu, Jan 12, 2017 at 11:29:08AM +1100, Cyril Bur wrote:
> > Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/misc/aspeed-lpc-ctrl.txt   | 78 ++++++++++++++++++++++
> >  1 file changed, 78 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt b/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
> > new file mode 100644
> > index 000000000000..f84ac83211ec
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
> > @@ -0,0 +1,78 @@
> > +ASpeed LPC Control
> > +==================
> > +This binding defines the LPC control for ASpeed SoCs. Partitions of
> > +the LPC bus can be access by other processors on the system, address
> > +ranges on the bus can map accesses from another processor to regions
> > +of the ASpeed SoC memory space.
> > +
> > +Reserved Memory:
> > +================
> > +The driver provides functionality to map the LPC bus to a region of
> > +ASpeed ram. A phandle to a reserved memory node must be provided so
> > +that the driver can safely use this region.
> > +
> > +Flash:
> > +======
> > +The driver provides functionality to unmap the LPC bus from ASpeed
> > +RAM, historically the default mapping has been to the SPI flash
> > +controller on the ASpeed SoC, a phandle to this node should be
> > +supplied.
> > +
> > +Device Node:
> > +============
> > +
> > +As LPC bus configuration registers are at the start of the LPC bus
> > +memory space, it makes most sense for the device to be within the LPC
> > +host node. See Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > +for more information. This does not have to be the case, provided the
> > +reg property can give the full address of the LPC bus.
> 
> Same comment here.
> 

Hi Rob,

Yes, thanks.

> > +
> > +Required properties:
> > +--------------------
> > +
> > +- compatible:		"aspeed,ast2400-lpc-ctrl" for ASpeed ast2400 SoCs
> > +					"aspeed,ast2500-lpc-ctrl" for ASpeed ast2500 SoCs
> > +
> > +- reg:				Location and size of the configuration registers
> > +					for the LPC bus. Note that if the device node is
> > +					within the LPC host node then base is relative to
> > +					that.
> > +
> > +- memory-region:	phandle of the reserved memory region
> > +- flash:			phandle of the SPI flash controller
> > +
> > +Example:
> > +--------
> > +
> > +reserved-memory {
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	ranges;
> > +
> > +	...
> > +
> > +	flash_memory: region@54000000 {
> > +		compatible = "aspeed,ast2400-lpc-ctrl";
> 
> This doesn't look right?
> 

Correct, my mistake, I'll remove.

> > +		no-map;
> > +		reg = <0x54000000 0x04000000>; /* 64M */
> 
> Is this system RAM? reserved-memory is generally for carveouts in system 
> RAM (e.g. the memory node).
> 

Yes it will be a chunk of system RAM. Our intended use case is to use
system ram to buffer host accesses to system flash (on the bmc). This
provides control over concurrent access to the flash and place to add
security measures to prevent the host from backdooring through the
flash. With the use of a protocol through the platform mailbox.

Having said that I don't want to limit myself to just that - there has
been other ideas for a host<->bmc ram buffer which may or may not see
the light of day.

I hope that makes sense,

Thanks for the review,

Cyril

> > +	};
> > +};
> > +
> > +host_pnor: spi@1e630000 {
> > +	reg = < 0x1e630000 0x18
> > +			0x30000000 0x02000000 >;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	compatible = "aspeed,ast2400-smc";
> > +
> > +	...
> > +
> > +};
> > +
> > +lpc-ctrl@0 {
> > +	compatible = "aspeed,ast2400-lpc-ctrl";
> > +	memory-region = <&flash_memory>;
> > +	flash = <&host_pnor>;
> > +	reg = <0x0 0x80>;
> > +};
> > +
> > -- 
> > 2.11.0
> > 
--
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


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