Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v5 1/3] i2c: pxa: Add support for the I2C units found in Armada 3700
From: Wolfram Sang @ 2016-11-29 21:17 UTC (permalink / raw)
  To: Romain Perier
  Cc: Wolfram Sang, linux-i2c, devicetree, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
	Thomas Petazzoni, Nadav Haklai, Omri Itach, Shadi Ammouri,
	Yahuda Yitschak, Hanna Hawa, Neta Zur Hershkovits
In-Reply-To: <20161121133247.29889-2-romain.perier@free-electrons.com>

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

> +	if (of_device_is_compatible(np, "marvell,armada-3700-i2c")) {
> +		i2c->fm_mask = ICR_BUSMODE_FM;
> +		i2c->hs_mask = ICR_BUSMODE_HS;
> +	} else {
> +		i2c->fm_mask = ICR_FM;
> +		i2c->hs_mask = ICR_HS;
> +	}
>  
>  	*i2c_types = (enum pxa_i2c_types)(of_id->data);
>  
> @@ -1181,6 +1194,13 @@ static int i2c_pxa_probe_pdata(struct platform_device *pdev,
>  			i2c->master_code = 0xe;
>  		i2c->rate = plat->rate;
>  	}
> +	if (!strcmp(id->name, "armada-3700-i2c")) {
> +		i2c->fm_mask = ICR_BUSMODE_FM;
> +		i2c->hs_mask = ICR_BUSMODE_HS;
> +	} else {
> +		i2c->fm_mask = ICR_FM;
> +		i2c->hs_mask = ICR_HS;
> +	}

Okay, having the same code twice is not nice as well.

Sorry for missing this in the first review and going a step back, but I
think now the best solution is to have again a REGS_A3700 struct, but we
should extend it with new entries for the shifted bits. Then in the init
code, you can do something like:

	i2c->fm_mask = pxa_reg_layout[i2c_type].fm_mask ?: ICR_FM;

Makes sense?

Thanks,

   Wolfram


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

^ permalink raw reply

* Re: [PATCH v2 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2016-11-29 21:16 UTC (permalink / raw)
  To: Greg KH
  Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129193750.GD20341-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

On Tue, Nov 29, 2016 at 08:37:50PM +0100, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Tue, Nov 29, 2016 at 01:38:20AM +0300, Serge Semin wrote:
> > +struct idt_89hpesx_dev {
> > +	u32 eesize;
> > +	bool eero;
> > +	u8 eeaddr;
> > +
> > +	u8 inieecmd;
> > +	u8 inicsrcmd;
> > +	u8 iniccode;
> > +
> > +	atomic_t csr;
> > +
> > +	int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> > +	int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> > +	struct mutex smb_mtx;
> > +
> > +	struct i2c_client *client;
> > +
> > +	struct bin_attribute *ee_file;
> > +	struct dentry *csr_dir;
> > +	struct dentry *csr_file;
> > +};
> > +#define to_pdev_kobj(__kobj) \
> > +	dev_get_drvdata(container_of(__kobj, struct device, kobj))
> 
> Is it a struct device, or a kobject?  This is totally confusing to me.
> 
> And can't you just use kobj_to_dev()?
> 

I just didn't know about kobj_to_dev() inline function. Totally agree that
container_of() should be replaced with it.
What does look confusing to you? Do you mean the name "to_pdev_kobj" of the
macro?

> > +/*
> > + * eeprom_attribute - EEPROM sysfs-node attributes
> > + *
> > + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> > + * be read-only as well if the corresponding flag is specified in OF node.
> > + */
> > +BIN_ATTR(eeprom, 0644, idt_sysfs_eeprom_read, idt_sysfs_eeprom_write,
> > +	 EEPROM_DEF_SIZE);
> 
> static?
> 
> And BIN_ATTR_RW()?
> 
> thanks,
> 
> greg k-h

Of course it should be static. Thanks for noticing that.
But I intentionally utilized BIN_ATTR() instead of BIN_ATTR_RW(), because
the last one implies to define the read/write methods with names
"_name##_read"/"_name##_write", which totally get out of naming within the
driver source code. To tell the truth macro BIN_ATTR_RW() isn't that
popular in the kernel. Neither is BIN_ATTR() macro, but it suites my driver
better than the another one.

Thanks,
-Sergey

--
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 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Serge Semin @ 2016-11-29 21:15 UTC (permalink / raw)
  To: Greg KH
  Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161129193436.GB20341-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

On Tue, Nov 29, 2016 at 08:34:36PM +0100, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Tue, Nov 29, 2016 at 01:38:21AM +0300, Serge Semin wrote:
> > See cover-letter for changelog
> 
> There is no cover letter in an individual patch when it gets committed
> to the tree...
> 
> So please fix, personally, I never read cover letters, each patch should
> be "obvious" on it's own :)
> 
> thanks,
> 
> greg k-h

Understood. I'll send the v3 of altered patchset over with individual messages
for each patch.

Thanks,
-Sergey

--
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 0/3] increase TSCADC clock to 24MHz and fix ti,charge-delay to represent in nS
From: Dmitry Torokhov @ 2016-11-29 21:09 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: Lee Jones, linux-input, Jonathan Cameron, Rob Herring,
	Mark Rutland, Sekhar Nori, Vignesh R, devicetree, linux-omap,
	linux-kernel
In-Reply-To: <8b97b7e6-1ea4-bf13-7da5-55135a994231@ti.com>

On Tue, Nov 29, 2016 at 11:11:35AM +0530, Mugunthan V N wrote:
> On Friday 25 November 2016 03:29 PM, Lee Jones wrote:
> > On Fri, 25 Nov 2016, Mugunthan V N wrote:
> > 
> >> Hi Dmitry Torokhov,
> >>
> >> On Thursday 10 November 2016 10:05 PM, Mugunthan V N wrote:
> >>> This patch series enables ADC to be clocked at 24MHz as the
> >>> TI AM335x ADC driver has already adopted to use DMA to transfer
> >>> ADC samples. Now ADC can generated upto 800K Samples per second
> >>> with the patch [1] on AM335x BBB and AM437x GP EVM.
> >>>
> >>> when ADC ref clock is set at 24MHz, I am seeing some issue with
> >>> touch screen pointer as the pointer jumps to random locations
> >>> with free draw application. The issue is due to increase in ADC
> >>> clock and charge delay for the touchscreen ADC line duration
> >>> reduced.
> >>>
> >>> So the notation of ti,charge-delay in terms of ADC clock is
> >>> wrong, it has to be represented in time and driver has to convert
> >>> the charge delay time to ADC clocks based on what ADC clock
> >>> frequency is set.
> >>>
> >>> Measured the performance with the iio_generic_buffer with the
> >>> patch [2] applied
> >>>
> >>> Verified the touch screen on AM335x GP EVM and AM335x BBB LCD7
> >>> cape with [3] dts for display and touch screen to work.
> >>>
> >>
> >> Since there are acks from DT and MFD maintainers, can you pull the patch
> >> series if you do not have any more comments.
> > 
> > Cant do anything without *all* Acks.
> > 
> Hi Dmitry Torokhov,
> 
> Can you provide your inputs on the patch series.

You have my ack for the input bit.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/3] Input: ti_am335x_tsc: Add support for ti,charge-delay-ns
From: Dmitry Torokhov @ 2016-11-29 21:09 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron, Rob Herring,
	Mark Rutland, Lee Jones, Sekhar Nori, Vignesh R,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161111075819.5760-1-mugunthanvnm-l0cyMroinI0@public.gmane.org>

On Fri, Nov 11, 2016 at 01:28:19PM +0530, Mugunthan V N wrote:
> ti,charge-delay will be deprecated as it represents number of
> clock cycles and the DT entries are done in assumption of 3MHz
> TSCADC clock, but clock can be set upto 24MHz. So driver add
> support for ti,charge-delay-ns and do not drop support for
> ti,charge-delay to support old dtbs and it will be assumed that
> it is for 3MHz TSCADC clock and will be calculated as per current
> clock speed.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>

Acked-by: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/input/touchscreen/ti_am335x_tsc.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index 7953381d939a..104b3640f728 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -379,15 +379,30 @@ static int titsc_parse_dt(struct platform_device *pdev,
>  		ts_dev->coordinate_readouts = 5;
>  	}
>  
> -	err = of_property_read_u32(node, "ti,charge-delay",
> +	err = of_property_read_u32(node, "ti,charge-delay-ns",
>  				   &ts_dev->charge_delay);
> -	/*
> -	 * If ti,charge-delay value is not specified, then use
> -	 * CHARGEDLY_OPENDLY as the default value.
> -	 */
> -	if (err < 0) {
> -		ts_dev->charge_delay = CHARGEDLY_OPENDLY;
> -		dev_warn(&pdev->dev, "ti,charge-delay not specified\n");
> +	if (err >= 0) {
> +		u64 charge_delay = ts_dev->charge_delay;
> +
> +		charge_delay *= ADC_CLK;
> +		do_div(charge_delay, 1E9);
> +		ts_dev->charge_delay = (u32)charge_delay;
> +	} else {
> +		err = of_property_read_u32(node, "ti,charge-delay",
> +					   &ts_dev->charge_delay);
> +		/*
> +		 * If ti,charge-delay value is not specified, then use
> +		 * CHARGEDLY_OPENDLY as the default value.
> +		 */
> +		if (err < 0) {
> +			ts_dev->charge_delay = CHARGEDLY_OPENDLY;
> +			dev_warn(&pdev->dev, "ti,charge-delay not specified\n");
> +		}
> +		/*
> +		 * ti,charge-delay is specified with referrence to 3MHz,
> +		 * so convert it to in referrence to current clock
> +		 */
> +		ts_dev->charge_delay *= ADC_CLK / 3000000;
>  	}
>  
>  	return of_property_read_u32_array(node, "ti,wire-config",
> -- 
> 2.11.0.rc0.7.gbe5a750
> 

-- 
Dmitry
--
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: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
From: Stephen Boyd @ 2016-11-29 21:05 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Rob Herring, Linux-ALSA, Linux-DT, Michael Turquette,
	Russell King, Linux-Kernel, Mark Brown,
	linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <87y409cw71.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

On 11/24, Kuninori Morimoto wrote:
> 
> Hi Stephen, again
> 
> > > I've seen bindings that have the 'clocks' property at the top
> > > level and the appropriate 'clock-names' property to relate the
> > > clocks to a subnode.
> > > 
> > >  	sound_soc {
> > > 		clocks = <&xxx>, <&xxx>;
> > > 		clock-names = "cpu", "codec";
> > >  		...
> > >  		cpu {
> > >  			...
> > >  		};
> > >  		codec {
> > >  			...
> > >  		};
> > >  	};
> > > 
> > > Then the subnodes call clk_get() with the top level device and
> > > the name of their node and things match up. I suppose this
> > > binding is finalized though, so we can't really do that?
> > > 
> > > I see that the gpio framework has a similar design called
> > > devm_get_gpiod_from_child(), so how about we add a
> > > devm_get_clk_from_child() API? That would more closely match the
> > > intent here, which is to restrict the clk_get() operation to
> > > child nodes of the device passed as the first argument.
> > > 
> > > struct clk *devm_get_clk_from_child(struct device *dev,
> > > 				    const char *con_id,
> > > 				    struct device_node *child);
> 
> Thanks, but, my point is that Linux already have "of_clk_get()",
> but we don't have its devm_ version.
> The point is that of_clk_get() can get clock from "device_node".
> Why having devm_ version become so problem ?

The problem is that it encourages the use of of_clk_get() when
clk_get() is more desirable. Ideally of_clk_get() is never used
when a device exists. In this case, it seems like we need to
support it though, hence the suggestion of having a
devm_get_clk_from_child() API, that explicitly reads as "get a
clock from a child node of this device". The distinction is
important, because of_clk_get() should rarely be used.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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] arm64: dts: juno: Correct PCI IO window
From: Jeremy Linton @ 2016-11-29 20:45 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	liviu.dudau-5wv7dgnIgG8, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8

The PCIe root complex on Juno translates the MMIO mapped
at 0x5f800000 to the PIO address range starting at 0
(which is common because PIO addresses are generally < 64k).
Correct the DT to reflect this.

Signed-off-by: Jeremy Linton <jeremy.linton-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
index 334271a..7d3a2ac 100644
--- a/arch/arm64/boot/dts/arm/juno-base.dtsi
+++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
@@ -393,7 +393,7 @@
 		#address-cells = <3>;
 		#size-cells = <2>;
 		dma-coherent;
-		ranges = <0x01000000 0x00 0x5f800000 0x00 0x5f800000 0x0 0x00800000>,
+		ranges = <0x01000000 0x00 0x00000000 0x00 0x5f800000 0x0 0x00800000>,
 			 <0x02000000 0x00 0x50000000 0x00 0x50000000 0x0 0x08000000>,
 			 <0x42000000 0x40 0x00000000 0x40 0x00000000 0x1 0x00000000>;
 		#interrupt-cells = <1>;
-- 
2.5.5

--
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 related

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-29 20:10 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161129210455.d0c8450d3ceb83c1abe521b0-GANU6spQydw@public.gmane.org>

Hi Jean-François,

On Tuesday 29 Nov 2016 21:04:55 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 21:33 +0200 Laurent Pinchart wrote:
> >>> You need a third port for the HDMI encoder output, connected to an
> >>> HDMI connector DT node.
> >> 
> >> I don't see what you mean. The HDMI device is both the encoder
> >> and connector (as the TDA998x):
> >
> > The driver might create both an encoder and a connector, but I very much
> > doubt that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector,
> > unless the SoC package has an HDMI connector coming out of it :-)
> > 
> >> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> >> ----- plane ------    - CRTC -   - encoder  \
> >>                                    connector -- (HDMI cable)
> >>          audio-controller -   - audio-codec /
> 
> The schema is the same as the Dove Cubox: the TDA998x is just a chip
> with some wires going out and the physical connector is supposed to be
> at the end of the wires.

I've missed the Dove Cubox DT bindings when they were submitted. Fortunately 
(or unfortunately for you, depending on how you look at it ;-)) I've paid more 
attention this time.

> Here, the HDMI pins of the SoC go to a pure hardware chip and then to
> the physical connector. Which software entity do you want to add?

I don't want to add a software entity, I just want to model the connector in 
DT as it's present in the system. Even though that's more common for other bus 
types than HDMI (LVDS for instance) it wouldn't be inconceivable to connect 
the HDMI signals to an on-board chim instead of an HDMI connector, so the HDMI 
encoder output should be modelled by a port and connected to a connector DT 
node in this case.

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-29 20:04 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <6356753.Ekaj7GdA0W@avalon>

On Tue, 29 Nov 2016 21:33 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:

> > > You need a third port for the HDMI encoder output, connected to an HDMI
> > > connector DT node.
> > 
> > I don't see what you mean. The HDMI device is both the encoder
> > and connector (as the TDA998x):
> 
> The driver might create both an encoder and a connector, but I very much doubt 
> that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless the 
> SoC package has an HDMI connector coming out of it :-)
> 
> > plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> > ----- plane ------    - CRTC -   - encoder  \
> >                                    connector -- (HDMI cable)
> >          audio-controller -   - audio-codec /

The schema is the same as the Dove Cubox: the TDA998x is just a chip
with some wires going out and the physical connector is supposed to be
at the end of the wires.

Here, the HDMI pins of the SoC go to a pure hardware chip and then to
the physical connector. Which software entity do you want to add?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Greg KH @ 2016-11-29 19:37 UTC (permalink / raw)
  To: Serge Semin
  Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480372701-30560-2-git-send-email-fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Nov 29, 2016 at 01:38:20AM +0300, Serge Semin wrote:
> +struct idt_89hpesx_dev {
> +	u32 eesize;
> +	bool eero;
> +	u8 eeaddr;
> +
> +	u8 inieecmd;
> +	u8 inicsrcmd;
> +	u8 iniccode;
> +
> +	atomic_t csr;
> +
> +	int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
> +	int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
> +	struct mutex smb_mtx;
> +
> +	struct i2c_client *client;
> +
> +	struct bin_attribute *ee_file;
> +	struct dentry *csr_dir;
> +	struct dentry *csr_file;
> +};
> +#define to_pdev_kobj(__kobj) \
> +	dev_get_drvdata(container_of(__kobj, struct device, kobj))

Is it a struct device, or a kobject?  This is totally confusing to me.

And can't you just use kobj_to_dev()?

> +/*
> + * eeprom_attribute - EEPROM sysfs-node attributes
> + *
> + * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
> + * be read-only as well if the corresponding flag is specified in OF node.
> + */
> +BIN_ATTR(eeprom, 0644, idt_sysfs_eeprom_read, idt_sysfs_eeprom_write,
> +	 EEPROM_DEF_SIZE);

static?

And BIN_ATTR_RW()?

thanks,

greg k-h
--
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: drm/bridge: adv7511: Add regulator bindings
From: Laurent Pinchart @ 2016-11-29 19:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: devicetree, linux-arm-msm, dri-devel
In-Reply-To: <20161129110125.sksjxfy56ofzyxw3@sirena.org.uk>

Hi Mark,

On Tuesday 29 Nov 2016 11:01:25 Mark Brown wrote:
> On Tue, Nov 29, 2016 at 11:11:03AM +0200, Laurent Pinchart wrote:
> > On Tuesday 29 Nov 2016 13:41:33 Archit Taneja wrote:
> >> I thought we couldn't add mandatory properties once the device is
> >> already present in DT for one or more platforms.
> > 
> > You can, as long as you treat them as optional in the driver to retain
> > backward compatibility. The DT bindings should document the properties
> > expected from a new platform (older versions of the bindings will always
> > be available in the git history).
> 
> The device probably never worked without power...  note that the kernel
> will substitute in dummy regulators for anything that isn't explicitly
> mapped so it won't actually break anything.
> 
> >> Say, if we do make it mandatory for future additions, we would need to
> >> have DT property for the supplies for the new platforms. If the
> >> regulators on these boards are fixed supplies, they would be need to be
> >> modeled using "regulator-fixed", possibly without any input supply. Is
> >> that what you're suggesting?
> > 
> > That's the idea, yes. Clock maintainers have a similar opinion regarding
> > the clock bindings, where a clock that is not optional at the hardware
> > level should be specified in DT even if it's always present.
> > 
> > Mark, any opinion ?
> 
> It's best practice to always describe the power.  The kernel will cope
> if people don't but it's not unknown for drivers to discover a reason
> for wanting information about their power and hard to retrofit that if
> it's not been in there from the get go.

Sounds good to me, thanks.

> Please note that if you're going to CC me into a graphics thread there's
> a good chance I will miss it, I get copied on quite a lot of graphics
> related mail that's not really relevant so I often skip it.  Changing
> the subject line would help with that.

I try to add a (CC'ing xxx) at the beginning of the e-mail to draw attention 
when a question is targetted at a particular person. If that's not enough I 
can change the subject, but might forget to do so from time to time. Or you 
could whitelist me, unless I'm already blacklisted ;-)

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH v2 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Greg KH @ 2016-11-29 19:34 UTC (permalink / raw)
  To: Serge Semin
  Cc: srinivas.kandagatla, andrew, robh+dt, mark.rutland, Sergey.Semin,
	linux-kernel, devicetree
In-Reply-To: <1480372701-30560-2-git-send-email-fancer.lancer@gmail.com>

On Tue, Nov 29, 2016 at 01:38:20AM +0300, Serge Semin wrote:
> See cover-letter for changelog

Same here.

^ permalink raw reply

* Re: [PATCH v2 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Greg KH @ 2016-11-29 19:34 UTC (permalink / raw)
  To: Serge Semin
  Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480372701-30560-3-git-send-email-fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, Nov 29, 2016 at 01:38:21AM +0300, Serge Semin wrote:
> See cover-letter for changelog

There is no cover letter in an individual patch when it gets committed
to the tree...

So please fix, personally, I never read cover letters, each patch should
be "obvious" on it's own :)

thanks,

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

^ permalink raw reply

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-29 19:33 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161129202751.e94f72a1b9e2c2fb064e9933-GANU6spQydw@public.gmane.org>

Hi Jean-François,

On Tuesday 29 Nov 2016 20:27:51 Jean-Francois Moine wrote:
> On Tue, 29 Nov 2016 20:46:22 +0200 Laurent Pinchart wrote:
> [snip]
> 
> >> +Example:
> >> +
> >> +	hdmi: hdmi@01ee0000 {
> >> +		compatible = "allwinner,sun8i-a83t-hdmi";
> >> +		reg = <0x01ee0000 0x20000>;
> >> +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> >> +			 <&ccu CLK_HDMI_DDC>;
> >> +		clock-names = "bus", "clock", "ddc-clock";
> >> +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> >> +		reset-names = "hdmi0", "hdmi1";
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&hdmi_pins_a>;
> >> +		status = "disabled";
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +		port@0 {			/* video */
> >> +			reg = <0>;
> >> +			hdmi_tcon1: endpoint {
> >> +				remote-endpoint = <&tcon1_hdmi>;
> >> +			};
> >> +		};
> >> +		port@1 {			/* audio */
> >> +			reg = <1>;
> >> +			hdmi_i2s2: endpoint {
> >> +				remote-endpoint = <&i2s2_hdmi>;
> >> +			};
> >> +		};
> > 
> > You need a third port for the HDMI encoder output, connected to an HDMI
> > connector DT node.
> 
> I don't see what you mean. The HDMI device is both the encoder
> and connector (as the TDA998x):

The driver might create both an encoder and a connector, but I very much doubt 
that the "allwinner,sun8i-a83t-hdmi" hardware contains a connector, unless the 
SoC package has an HDMI connector coming out of it :-)

> plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
> ----- plane ------    - CRTC -   - encoder  \
>                                    connector -- (HDMI cable)
>          audio-controller -   - audio-codec /
> 
> > > +	};

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-29 19:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Dave Airlie,
	Maxime Ripard, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <5996605.Gh0PjPIAcn@avalon>

On Tue, 29 Nov 2016 20:46:22 +0200
Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
	[snip]
> > +Example:
> > +
> > +	hdmi: hdmi@01ee0000 {
> > +		compatible = "allwinner,sun8i-a83t-hdmi";
> > +		reg = <0x01ee0000 0x20000>;
> > +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> > +			 <&ccu CLK_HDMI_DDC>;
> > +		clock-names = "bus", "clock", "ddc-clock";
> > +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> > +		reset-names = "hdmi0", "hdmi1";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&hdmi_pins_a>;
> > +		status = "disabled";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		port@0 {			/* video */
> > +			reg = <0>;
> > +			hdmi_tcon1: endpoint {
> > +				remote-endpoint = <&tcon1_hdmi>;
> > +			};
> > +		};
> > +		port@1 {			/* audio */
> > +			reg = <1>;
> > +			hdmi_i2s2: endpoint {
> > +				remote-endpoint = <&i2s2_hdmi>;
> > +			};
> > +		};
> 
> You need a third port for the HDMI encoder output, connected to an HDMI 
> connector DT node.

I don't see what you mean. The HDMI device is both the encoder
and connector (as the TDA998x):

plane -> DE2 mixer ---> TCON -----> HDMI -----> display device
----- plane ------    - CRTC -   - encoder  \
                                   connector -- (HDMI cable)
         audio-controller -   - audio-codec /

> > +	};

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes
From: Laurent Pinchart @ 2016-11-29 19:19 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: airlied-cv59FeDIM0c, khilman-rdvid1DuHRBWk0Htik3J/w,
	carlo-KA+7E9HrN00dnm+yROfE0A,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	victor.wan-LpR1jeaWuhtBDgjK7y7TUQ,
	jerry.cao-LpR1jeaWuhtBDgjK7y7TUQ, Xing.Xu-LpR1jeaWuhtBDgjK7y7TUQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA, daniel-/w4YWyX8dFk
In-Reply-To: <1812737.pZP73QSyU1@avalon>

Hi Neil,

On Tuesday 29 Nov 2016 21:16:17 Laurent Pinchart wrote:
> On Tuesday 29 Nov 2016 11:47:47 Neil Armstrong wrote:
> > Add Video Processing Unit and CVBS Output nodes, and enable CVBS on
> > selected boards.
> > 
> > Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > ---
> > 
> >  arch/arm64/boot/dts/amlogic/meson-gx.dtsi          | 46 +++++++++++++++++
> >  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts    |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  8 ++++
> >  .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts     |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  8 ++++
> >  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  4 ++
> >  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi         |  8 ++++
> >  8 files changed, 86 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..644d5f6 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> > @@ -153,6 +153,27 @@
> >  		};
> >  	};
> > 
> > +	venc_cvbs: venc-cvbs {
> > +		compatible = "amlogic,meson-gx-cvbs";
> > +		status = "disabled";
> 
> Still no registers here ?

Or is this the internal video encoder, not the HHI VDAC ? If so, haven't we 
decided when discussing the previous version that there's no need for a DT 
node to model the video encoder as it's part of the VPU ?

> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			venc_cvbs_in: port@0 {
> 
> Nitpicking, you don't need a label here as ports are never referenced by
> phandle. Same for the vpu node below.
> 
> > +				 #address-cells = <1>;
> > +				 #size-cells = <0>;
> > +				 reg = <0>;
> > +
> > +				 venc_cvbs_in_vpu: endpoint@0 {
> > +					 reg = <0>;
> 
> And there's no requirement to number the endpoint if there's a single one of
> them (but it's not forbidden either).
> 
> > +					 remote-endpoint =
> 
> <&vpu_out_venc_cvbs>;
> 
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> > 
> >  	soc {

Shouldn't the above node be placed inside the soc ?

> >  		compatible = "simple-bus";
> >  		#address-cells = <2>;
> > 
> > @@ -356,5 +377,30 @@
> > 
> >  				status = "disabled";
> >  			
> >  			};
> >  		
> >  		};
> > 
> > +
> > +		vpu: vpu@d0100000 {
> > +			compatible = "amlogic,meson-gx-vpu";
> > +			reg = <0x0 0xd0100000 0x0 0x100000>,
> > +			      <0x0 0xc883c000 0x0 0x1000>,
> > +			      <0x0 0xc8838000 0x0 0x1000>;
> > +			reg-names = "base", "hhi", "dmc";
> > +			interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> > +
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +
> > +				vpu_out: port@1 {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +					reg = <1>;
> > +
> > +					vpu_out_venc_cvbs: endpoint@0 {
> > +						reg = <0>;
> > +						remote-endpoint =
> 
> <&venc_cvbs_in_vpu>;
> 
> > +					};
> > +				};
> > +			};
> > +		};
> > 
> >  	};
> >  
> >  };
> > 
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
> > 9696820..a55d1cf 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> > @@ -229,3 +229,7 @@
> > 
> >  	clocks = <&clkc CLKID_FCLK_DIV4>;
> >  	clock-names = "clkin0";
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 5e5e2de..3c09bd1
> > 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> > @@ -266,3 +266,7 @@
> > 
> >  	clocks = <&clkc CLKID_FCLK_DIV4>;
> >  	clock-names = "clkin0";
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..1a321c8f
> > 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> > @@ -506,3 +506,11 @@
> > 
> >  		 <&clkc CLKID_FCLK_DIV2>;
> >  	
> >  	clock-names = "core", "clkin0", "clkin1";
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	compatible = "amlogic,meson-gxbb-cvbs", "amlogic,meson-gx-cvbs";
> > +};
> > +
> > +&vpu {
> > +	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts index
> > e99101a..2a9b46f 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> > @@ -203,3 +203,7 @@
> > 
> >  	clocks = <&clkc CLKID_FCLK_DIV4>;
> >  	clock-names = "clkin0";
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi index 3af54dc..b60c5ce 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> > @@ -299,3 +299,11 @@
> > 
> >  		 <&clkc CLKID_FCLK_DIV2>;
> >  	
> >  	clock-names = "core", "clkin0", "clkin1";
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	compatible = "amlogic,meson-gxl-cvbs", "amlogic,meson-gx-cvbs";
> > +};
> > +
> > +&vpu {
> > +	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts index
> > d320727..1ae2451 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> > @@ -167,3 +167,7 @@
> > 
> >  		max-speed = <1000>;
> >  	
> >  	};
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	status = "okay";
> > +};
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> > b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi index c1974bb..fecd8c2 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> > @@ -112,3 +112,11 @@
> > 
> >  		};
> >  	
> >  	};
> >  
> >  };
> > 
> > +
> > +&venc_cvbs {
> > +	compatible = "amlogic,meson-gxm-cvbs", "amlogic,meson-gx-cvbs";
> > +};
> > +
> > +&vpu {
> > +	compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
> > +};

-- 
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 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes
From: Laurent Pinchart @ 2016-11-29 19:16 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, Xing.Xu, victor.wan, khilman, linux-kernel, dri-devel,
	jerry.cao, carlo, linux-amlogic, linux-arm-kernel
In-Reply-To: <1480416469-9655-3-git-send-email-narmstrong@baylibre.com>

Hi Neil,

Thank you for the patch.

On Tuesday 29 Nov 2016 11:47:47 Neil Armstrong wrote:
> Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
> boards.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi          | 46 +++++++++++++++++++
>  .../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts    |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi   |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  8 ++++
>  .../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts     |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi         |  8 ++++
>  .../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts |  4 ++
>  arch/arm64/boot/dts/amlogic/meson-gxm.dtsi         |  8 ++++
>  8 files changed, 86 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi index fc033c0..644d5f6 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -153,6 +153,27 @@
>  		};
>  	};
> 
> +	venc_cvbs: venc-cvbs {
> +		compatible = "amlogic,meson-gx-cvbs";
> +		status = "disabled";

Still no registers here ?

> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			venc_cvbs_in: port@0 {

Nitpicking, you don't need a label here as ports are never referenced by 
phandle. Same for the vpu node below.

> +				 #address-cells = <1>;
> +				 #size-cells = <0>;
> +				 reg = <0>;
> +
> +				 venc_cvbs_in_vpu: endpoint@0 {
> +					 reg = <0>;

And there's no requirement to number the endpoint if there's a single one of 
them (but it's not forbidden either).

> +					 remote-endpoint = 
<&vpu_out_venc_cvbs>;
> +				};
> +			};
> +		};
> +	};
> +
>  	soc {
>  		compatible = "simple-bus";
>  		#address-cells = <2>;
> @@ -356,5 +377,30 @@
>  				status = "disabled";
>  			};
>  		};
> +
> +		vpu: vpu@d0100000 {
> +			compatible = "amlogic,meson-gx-vpu";
> +			reg = <0x0 0xd0100000 0x0 0x100000>,
> +			      <0x0 0xc883c000 0x0 0x1000>,
> +			      <0x0 0xc8838000 0x0 0x1000>;
> +			reg-names = "base", "hhi", "dmc";
> +			interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				vpu_out: port@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					reg = <1>;
> +
> +					vpu_out_venc_cvbs: endpoint@0 {
> +						reg = <0>;
> +						remote-endpoint = 
<&venc_cvbs_in_vpu>;
> +					};
> +				};
> +			};
> +		};
>  	};
>  };
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts index
> 9696820..a55d1cf 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
> @@ -229,3 +229,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&venc_cvbs {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi index 5e5e2de..3c09bd1
> 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
> @@ -266,3 +266,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&venc_cvbs {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi index ac5ad3b..1a321c8f
> 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -506,3 +506,11 @@
>  		 <&clkc CLKID_FCLK_DIV2>;
>  	clock-names = "core", "clkin0", "clkin1";
>  };
> +
> +&venc_cvbs {
> +	compatible = "amlogic,meson-gxbb-cvbs", "amlogic,meson-gx-cvbs";
> +};
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts index
> e99101a..2a9b46f 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
> @@ -203,3 +203,7 @@
>  	clocks = <&clkc CLKID_FCLK_DIV4>;
>  	clock-names = "clkin0";
>  };
> +
> +&venc_cvbs {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi index 3af54dc..b60c5ce 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -299,3 +299,11 @@
>  		 <&clkc CLKID_FCLK_DIV2>;
>  	clock-names = "core", "clkin0", "clkin1";
>  };
> +
> +&venc_cvbs {
> +	compatible = "amlogic,meson-gxl-cvbs", "amlogic,meson-gx-cvbs";
> +};
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts index
> d320727..1ae2451 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
> @@ -167,3 +167,7 @@
>  		max-speed = <1000>;
>  	};
>  };
> +
> +&venc_cvbs {
> +	status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi index c1974bb..fecd8c2 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
> @@ -112,3 +112,11 @@
>  		};
>  	};
>  };
> +
> +&venc_cvbs {
> +	compatible = "amlogic,meson-gxm-cvbs", "amlogic,meson-gx-cvbs";
> +};
> +
> +&vpu {
> +	compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
> +};

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCHv2] mfd: cpcap: Add minimal support
From: Tony Lindgren @ 2016-11-29 19:12 UTC (permalink / raw)
  To: Lee Jones, Samuel Ortiz
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Marcel Partap, Mark Rutland,
	Michael Scott, Rob Herring
In-Reply-To: <20161129164702.5334-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161129 08:48]:
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/motorola-cpcap.txt
> @@ -0,0 +1,31 @@
> +Motorola CPCAP PMIC device tree binding
> +
> +Required properties:
> +- compatible		: One or both of "motorola,cpcap" or "ste,6556002"
> +- reg			: SPI chip select
> +- interrupt-parent	: The parent interrupt controller
> +- interrupts		: The interrupt line the device is connected to
> +- interrupt-controller	: Marks the device node as an interrupt controller
> +- #interrupt-cells	: The number of cells to describe an IRQ, should be 2
> +- #address-cells	: Child device offset number of cells, typically 1
> +- #size-cells		: Child device size number of cells, typically 1
> +- spi-max-frequency	: Typically set to 3000000
> +- spi-cs_high		: SPI chip select direction
> +
> +Example:
> +
> +&mcspi1 {
> +	cpcap: pmic@0 {
> +		compatible = "motorola,cpcap", "ste,6556002";
> +		reg = <0>;	/* cs0 */
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <7 IRQ_TYPE_EDGE_RISING>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <3000000>;
> +		spi-cs-high;
> +	};
> +};

The #address-cells should be 0 instead of 1 above.

Tony
--
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/1] scripts: Fixing NULL pointer dereference when pos->file is NULL
From: Frank Rowand @ 2016-11-29 19:08 UTC (permalink / raw)
  To: Maninder Singh, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	v.narang-Sze3O3UU22JBDgjK7y7TUQ, pankaj.m-Sze3O3UU22JBDgjK7y7TUQ,
	ajeet.y-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <1480415699-35335-1-git-send-email-maninder1.s-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On 11/29/16 02:34, Maninder Singh wrote:
> This patch fixes NULL pointer dereference when pos->file is NULL.
> 
> caught with static analysis tool.
> Signed-off-by: Maninder Singh <maninder1.s-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Vaneet Narang <v.narang-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  scripts/dtc/srcpos.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/scripts/dtc/srcpos.c b/scripts/dtc/srcpos.c
> index f534c22..360fd14 100644
> --- a/scripts/dtc/srcpos.c
> +++ b/scripts/dtc/srcpos.c
> @@ -252,12 +252,11 @@ struct srcpos *
>  srcpos_dump(struct srcpos *pos)
>  {
>  	printf("file        : \"%s\"\n",
> -	       pos->file ? (char *) pos->file : "<no file>");
> +	       pos->file ?  pos->file->name : "<no file>");
>  	printf("first_line  : %d\n", pos->first_line);
>  	printf("first_column: %d\n", pos->first_column);
>  	printf("last_line   : %d\n", pos->last_line);
>  	printf("last_column : %d\n", pos->last_column);
> -	printf("file        : %s\n", pos->file->name);
>  }
>  
>  
> 

Hi Maninder,

Please send any patches for dtc to the devicetree-compiler
mail list.  For details, see:

   http://vger.kernel.org/vger-lists.html#devicetree-compiler

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

^ permalink raw reply

* Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-29 18:46 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Jean-Francois Moine, Dave Airlie, Maxime Ripard, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <cd1caff75b6069af98680f78d672546748cb43c5.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>

Hi Jean-François,

Thank you for the patch.

On Tuesday 29 Nov 2016 10:08:25 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> ---
>  .../devicetree/bindings/display/sunxi/hdmi.txt     | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt new file mode
> 100644
> index 0000000..1e107cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/hdmi.txt
> @@ -0,0 +1,56 @@
> +Allwinner HDMI Transmitter
> +==========================
> +
> +The Allwinner HDMI transmitters are included in the SoCs.
> +They support audio and video.
> +
> +Required properties:
> + - compatible : should be one of
> +		"allwinner,sun8i-a83t-hdmi"
> +		"allwinner,sun8i-h3-hdmi"
> + - reg: base address and size of the I/O memory
> + - clocks : phandles to the HDMI clocks as described in
> +	Documentation/devicetree/bindings/clock/clock-bindings.txt
> + - clock-names : must be
> +		"bus" : bus gate
> +		"clock" : streaming clock
> +		"ddc-clock" : DDC clock
> + - resets : One or two phandles to the HDMI resets
> + - reset-names : when 2 phandles, must be
> +		"hdmi0" and "hdmi1"
> + - #address-cells : should be <1>
> + - #size-cells : should be <0>
> +
> +Required nodes:
> + - port: Audio and video input port nodes with endpoint definitions
> +	as defined in Documentation/devicetree/bindings/graph.txt.
> +	port@0 is video and port@1 is audio.
> +
> +Example:
> +
> +	hdmi: hdmi@01ee0000 {
> +		compatible = "allwinner,sun8i-a83t-hdmi";
> +		reg = <0x01ee0000 0x20000>;
> +		clocks = <&ccu CLK_BUS_HDMI>, <&ccu CLK_HDMI>,
> +			 <&ccu CLK_HDMI_DDC>;
> +		clock-names = "bus", "clock", "ddc-clock";
> +		resets = <&ccu RST_HDMI0>, <&ccu RST_HDMI1>;
> +		reset-names = "hdmi0", "hdmi1";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hdmi_pins_a>;
> +		status = "disabled";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {			/* video */
> +			reg = <0>;
> +			hdmi_tcon1: endpoint {
> +				remote-endpoint = <&tcon1_hdmi>;
> +			};
> +		};
> +		port@1 {			/* audio */
> +			reg = <1>;
> +			hdmi_i2s2: endpoint {
> +				remote-endpoint = <&i2s2_hdmi>;
> +			};
> +		};

You need a third port for the HDMI encoder output, connected to an HDMI 
connector DT node.

> +	};

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2
From: Laurent Pinchart @ 2016-11-29 18:45 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring, Maxime Ripard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <36111311.d8ul9hQ2CU@avalon>

Hi Jean-François,

A brief update.

On Tuesday 29 Nov 2016 20:41:30 Laurent Pinchart wrote:
> On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> > ---
> > 
> >  .../bindings/display/sunxi/sun8i-de2.txt           | 121 ++++++++++++++++
> >  1 file changed, 121 insertions(+)
> >  create mode 100644
> > 
> > Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> > mode 100644
> > index 0000000..edf38b8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> > @@ -0,0 +1,121 @@
> > +Allwinner sun8i Display Engine 2 subsystem
> > +==========================================
> > +
> > +The Allwinner DE2 subsystem contains a display controller (DE2),
> > +one or two LCD controllers (Timing CONtrollers) and their external
> > +interfaces (encoders/connectors).
> > +
> > +          +-----------+
> > +          | DE2       |
> > +          |           |
> > +          | +-------+ |
> > +  plane --->|       | |   +------+
> > +          | | mixer |---->| TCON |---> encoder  ---> display
> > +  plane --->|       | |   +------+     connector     device
> > +          | +-------+ |
> > +          |           |
> > +          | +-------+ |
> > +  plane --->|       | |   +------+
> > +          | | mixer |---->| TCON |---> encoder  ---> display
> > +  plane --->|       | |   +------+     connector     device
> > +          | +-------+ |
> > +          +-----------+
> > +
> > +The DE2 contains a processor which mixes the input planes and creates
> > +the images which are sent to the TCONs.
> > +From the software point of vue, there are 2 independent real-time
> > +mixers, each one being statically associated to one TCON.
> > +
> > +The TCONs adjust the image format to the one of the display device.
> > +
> > +Display controller (DE2)
> > +========================
> > +
> > +Required properties:
> > +
> > +- compatible: value should be one of the following
> > +		"allwinner,sun8i-a83t-display-engine"
> > +		"allwinner,sun8i-h3-display-engine"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > +		clock-names property.
> > +
> > +- clock-names: must contain
> > +		"bus": bus gate
> > +		"clock": clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- ports: must contain a list of 2 phandles, indexed by mixer number,
> > +	and pointing to display interface ports of TCONs
> > +
> > +LCD controller (TCON)
> > +=====================
> > +
> > +Required properties:
> > +
> > +- compatible: should be
> > +		"allwinner,sun8i-a83t-tcon"
> > +
> > +- reg: base address and size of the I/O memory
> > +
> > +- clocks: must include clock specifiers corresponding to entries in the
> > +		clock-names property.
> > +
> > +- clock-names: must contain
> > +		"bus": bus gate
> > +		"clock": pixel clock
> > +
> > +- resets: phandle to the reset of the device
> > +
> > +- interrupts: interrupt number for the TCON
> > +
> > +- port: port node with endpoint definitions as defined in
> > +	Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +Example:
> > +
> > +	de: de-controller@01000000 {
> > +		compatible = "allwinner,sun8i-h3-display-engine";
> > +		reg = <0x01000000 0x400000>;
> > +		clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> > +		clock-names = "bus", "clock";
> > +		resets = <&ccu RST_BUS_DE>;
> > +		ports = <&tcon0_p>, <&tcon1_p>;
> 
> This isn't how the OF graph DT bindings are used. You should instead have
> 
> 	ports {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		port@0 {

I forgot to add reg = <0>; (and similarly for port 1).

> 			de_out_0: endpoint {
> 				remote_endpoint = <&tcon0_hdmi>;
> 			};
> 		};
> 		port@1 {
> 			/* No endpoint as the port is not connected */
> 		};
> 	};
> 
> > +	};
> > +
> > +	tcon0: lcd-controller@01c0c000 {
> > +		compatible = "allwinner,sun8i-a83t-tcon";
> > +		reg = <0x01c0c000 0x400>;
> > +		clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> > +		clock-names = "bus", "clock";
> > +		resets = <&ccu RST_BUS_TCON0>;
> > +		interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		tcon0_p: port {
> > +			tcon0_hdmi: endpoint {
> > +				remote-endpoint = <&hdmi_tcon0>;
> > +			};
> > +		};
> 
> and here
> 
> 	port {
> 		tcon0_hdmi: endpoint {
> 			remote-endpoint = <&de_out_0>;
> 		};
> 	};

The TCON has an output, so this should instead be

	port@0 {
		reg = <0>;
 		tcon0_in: endpoint {
 			remote-endpoint = <&de_out_0>;
 		};
	};
	port@1 {
		reg = <1>;
 		tcon1_out: endpoint {
 			remote-endpoint = <&hdmi_in>;
 		};
	};

(the second port requires adding the HDMI encoder to the example)

> > +	};
> > +
> > +	/* not used */
> > +	tcon1: lcd-controller@01c0d000 {
> > +		compatible = "allwinner,sun8i-h3-tcon";
> > +		reg = <0x01c0d000 0x400>;
> > +		clocks = <&ccu CLK_BUS_TCON1>,
> > +			 <&ccu CLK_TCON0>;	/* no clock */
> > +		clock-names = "bus", "clock";
> > +		interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > +		status = "disabled";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		tcon1_p: port {
> > +			endpoint {
> > +				/* empty */
> > +			};
> > +		};
> 
> and here
> 
> 	port {
> 		/* No endpoint as the port is not connected */
> 	};
> 
> (although I'm not sure why you don't connect it)
> 
> > +	};

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v7 2/8] drm/sun8i: Add DT bindings documentation of Allwinner DE2
From: Laurent Pinchart @ 2016-11-29 18:41 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Jean-Francois Moine, Dave Airlie, Maxime Ripard, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <92fc53084274d5ab04fe28f2dc0b16cf5d94481e.1480414715.git.moinejf-GANU6spQydw@public.gmane.org>

Hi Jean-François,

Thank you for the patch.

On Monday 28 Nov 2016 19:02:39 Jean-Francois Moine wrote:
> Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> ---
>  .../bindings/display/sunxi/sun8i-de2.txt           | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt new file
> mode 100644
> index 0000000..edf38b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun8i-de2.txt
> @@ -0,0 +1,121 @@
> +Allwinner sun8i Display Engine 2 subsystem
> +==========================================
> +
> +The Allwinner DE2 subsystem contains a display controller (DE2),
> +one or two LCD controllers (Timing CONtrollers) and their external
> +interfaces (encoders/connectors).
> +
> +          +-----------+
> +          | DE2       |
> +          |           |
> +          | +-------+ |
> +  plane --->|       | |   +------+
> +          | | mixer |---->| TCON |---> encoder  ---> display
> +  plane --->|       | |   +------+     connector     device
> +          | +-------+ |
> +          |           |
> +          | +-------+ |
> +  plane --->|       | |   +------+
> +          | | mixer |---->| TCON |---> encoder  ---> display
> +  plane --->|       | |   +------+     connector     device
> +          | +-------+ |
> +          +-----------+
> +
> +The DE2 contains a processor which mixes the input planes and creates
> +the images which are sent to the TCONs.
> +From the software point of vue, there are 2 independent real-time
> +mixers, each one being statically associated to one TCON.
> +
> +The TCONs adjust the image format to the one of the display device.
> +
> +Display controller (DE2)
> +========================
> +
> +Required properties:
> +
> +- compatible: value should be one of the following
> +		"allwinner,sun8i-a83t-display-engine"
> +		"allwinner,sun8i-h3-display-engine"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> +		clock-names property.
> +
> +- clock-names: must contain
> +		"bus": bus gate
> +		"clock": clock
> +
> +- resets: phandle to the reset of the device
> +
> +- ports: must contain a list of 2 phandles, indexed by mixer number,
> +	and pointing to display interface ports of TCONs
> +
> +LCD controller (TCON)
> +=====================
> +
> +Required properties:
> +
> +- compatible: should be
> +		"allwinner,sun8i-a83t-tcon"
> +
> +- reg: base address and size of the I/O memory
> +
> +- clocks: must include clock specifiers corresponding to entries in the
> +		clock-names property.
> +
> +- clock-names: must contain
> +		"bus": bus gate
> +		"clock": pixel clock
> +
> +- resets: phandle to the reset of the device
> +
> +- interrupts: interrupt number for the TCON
> +
> +- port: port node with endpoint definitions as defined in
> +	Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +	de: de-controller@01000000 {
> +		compatible = "allwinner,sun8i-h3-display-engine";
> +		reg = <0x01000000 0x400000>;
> +		clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> +		clock-names = "bus", "clock";
> +		resets = <&ccu RST_BUS_DE>;
> +		ports = <&tcon0_p>, <&tcon1_p>;

This isn't how the OF graph DT bindings are used. You should instead have

	ports {
		#address-cells = <1>;
		#size-cells = <0>;
		port@0 {
			de_out_0: endpoint {
				remote_endpoint = <&tcon0_hdmi>;
			};
		};
		port@1 {
			/* No endpoint as the port is not connected */
		};
	};

> +	};
> +
> +	tcon0: lcd-controller@01c0c000 {
> +		compatible = "allwinner,sun8i-a83t-tcon";
> +		reg = <0x01c0c000 0x400>;
> +		clocks = <&ccu CLK_BUS_TCON0>, <&ccu CLK_TCON0>;
> +		clock-names = "bus", "clock";
> +		resets = <&ccu RST_BUS_TCON0>;
> +		interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		tcon0_p: port {
> +			tcon0_hdmi: endpoint {
> +				remote-endpoint = <&hdmi_tcon0>;
> +			};
> +		};

and here

	port {
		tcon0_hdmi: endpoint {
			remote-endpoint = <&de_out_0>;
		};
	};

> +	};
> +
> +	/* not used */
> +	tcon1: lcd-controller@01c0d000 {
> +		compatible = "allwinner,sun8i-h3-tcon";
> +		reg = <0x01c0d000 0x400>;
> +		clocks = <&ccu CLK_BUS_TCON1>,
> +			 <&ccu CLK_TCON0>;	/* no clock */
> +		clock-names = "bus", "clock";
> +		interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		tcon1_p: port {
> +			endpoint {
> +				/* empty */
> +			};
> +		};

and here

	port {
		/* No endpoint as the port is not connected */
	};

(although I'm not sure why you don't connect it)

> +	};

-- 
Regards,

Laurent Pinchart

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v3 1/2] of: Fix issue where code would fall through to error case.
From: Frank Rowand @ 2016-11-29 18:41 UTC (permalink / raw)
  To: Moritz Fischer, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
	mdf-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1480278058-19688-1-git-send-email-moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>

On 11/27/16 12:20, Moritz Fischer wrote:
> No longer fall through into the error case that prints out
> an error if no error (err = 0) occurred.
> 
> Rework error handling to print error where it occured instead
> of having a global catch-all at the end of the function.
> 
> Fixes d9181b20a83(of: Add back an error message, restructured)
> Signed-off-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
> ---
> Hi Rob, Frank
> 
> this has already Frank's Reviewed-by: tag on it, but since I changed around the
> earlier part (before tree_node gets assigned) Frank might wanna take another look
> at this.

< snip >

Hi Moritz,

I agree with Rob's suggested one line solution (that I initially misunderstood).
The one line fix is to add "if (err)" immediately following label "err_out" in
of_resolve_phandles().

As far as patch 2/2, I'm not bothered by the two instances of line over 80 chars.
But if Rob wants to take patch 2/2 I have no objection.

-Frank
--
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 01/13] devicetree/bindings: display: Document common panel properties
From: Laurent Pinchart @ 2016-11-29 18:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:MEDIA DRIVERS FOR RENESAS - FCP,
	devicetree@vger.kernel.org, Tomi Valkeinen, Laurent Pinchart,
	dri-devel
In-Reply-To: <CAL_JsqL0+4--nE1UDHWzqS-s18A+q2i76S8=woCb5huV3jUMew@mail.gmail.com>

Hi Rob,

On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote:
> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote:
> > On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote:
> >> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote:
> >>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote:
> >>>> Document properties common to several display panels in a central
> >>>> location that can be referenced by the panel device tree bindings.
> >>> 
> >>> Looks good. Just one comment...
> >>> 
> >>> [...]
> >>> 
> >>>> +Connectivity
> >>>> +------------
> >>>> +
> >>>> +- ports: Panels receive video data through one or multiple
> >>>> connections. While
> >>>> +  the nature of those connections is specific to the panel type, the
> >>>> +  connectivity is expressed in a standard fashion using ports as
> >>>> specified in
> >>>> +  the device graph bindings defined in
> >>>> +  Documentation/devicetree/bindings/graph.txt.
> >>> 
> >>> We allow panels to either use graph binding or be a child of the
> >>> display controller.
> >> 
> >> I knew that some display controllers use a phandle to the panel (see the
> >> fsl,panel and nvidia,panel properties), but I didn't know we had panels
> >> as children of display controller nodes. I don't think we should allow
> >> that for anything but DSI panels, as the DT hierarchy is based on control
> >> buses. Are you sure we have other panels instantiated through that
> >> mechanism ?
>
> Some panels have no control bus, so were do we place them?

I'd say under the root node, like all similar control-less devices.

> I would say the hierarchy is based on buses with a preference for the
> control bus when there are multiple buses. I'm not a fan of just sticking
> things are the top level.

OK, so much for my comment a few lines up :-)

The problem with placing non-DSI panels as children of the display controller 
and not using OF graph is that the panel bindings become dependent of the 
display controller being used. A display controller using OF graph would 
require the panel to do the same, while a display controller expecting a panel 
child node (with a specific name) would require DT properties for the panel 
node.

I'm also not sure the complexity of OF graph is really that prohibitive if you 
compare it to panels as child nodes. To get the panel driver to bind to the 
panel DT node the display controller driver would need to create a platform 
device for the panel and register it. That's not very difficult, but parsing a 
single port and endpoint isn't either (and we could even provide a helper 
function for that, a version of of_drm_find_panel() that would take as an 
argument the display controller device node instead of the panel device node).

> > Ping ?
> > 
> > Please note that this file documents properties common to multiple panel
> > DT bindings, but in no way makes it mandatory to use the OF graph bindings
> > for panels. The decision is left to individual bindings.
> 
> It is mandatory in the sense that we don't want more cases of "fsl,panel".

That I agree with :-)

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-29 18:20 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Uwe Kleine-König, Rob Herring, Frank Rowand,
	Andreas Färber, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Michal Hrusecki, Tomas Hlavacek, Bed??icha Ko??atu,
	Florian Fainelli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87oa0yb29b.fsf-BOtmAI95c/+pXNIQCVAXCG0Lkn3mC4nZ0tOlhedn3YvkypF1WZHjJXhe7Zk3YmMvjmZSf7Nhrd8@public.gmane.org>

On Tue, Nov 29, 2016 at 12:54:24PM -0500, Vivien Didelot wrote:
> Hi,
> 
> Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org> writes:
> 
> > Also it seems wrong to write "marvell,mv88e6085" (only) if I know the
> > hardware is really a "marvell,mv88e6176".
> 
> I agree. It might be complex for a user to dig into the driver in order
> to figure out how the switch ID is read and which compatible to choose.
> 
> I've sent a patch to change this https://lkml.org/lkml/2016/6/8/1198 but
> Andrew had a stronger opinion on compatible strings, which makes sense.
> 
> >> Linus has said he does not like ARM devices because of all the busses
> >> which are not enumerable. Here we have a device which with a little
> >> bit of help we can enumerate. So we should. 
> >
> > If you write
> >
> > 	compatible = "marvell,mv88e6176", "marvell,mv88e6085";
> >
> > you can still enumerate in the same way as before.
> 
> So we don't break the existing DTS files, I like this.
> 
> The driver already prints info about the detected switch. Instead of
> failing at probe, which seems against the notion of compatible and
> breaks the existing behavior, it could report the eventual mismatch?

I'm still against it.

Say we let the driver probe and warn when the compatible string is
wrong. Is this likely to get fixed? Probably not, it is just a
warning, people ignore them.

A few years later, we have accumulated a number of broken device
trees. And suddenly we really do have a Marvell device with a broken
ID in its port register, or more likely, the revision number was not
incremented but there was incompatible register changes. We suddenly
do have to look at the compatible string. But we know many are actually
wrong. How do we know which to trust? We basically have to say, if the
compatible is "marvell,mv88e6042" we trust it, all the others we don't
trust and ignore.

Isn't that madness?

What we have today is verified correct. If this hypothetical
"marvell,mv88e6042" does happen, then no problems, we add a compatible
string for it, and it works.

We should probably add a comment to the mv88e6xxx_of_match array,
explaining how it currently works.

      Andrew
--
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