Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v9 0/8] drivers: touchscreen: tsc2007 and ads7846/tsc2046 improvements (use common touchscreen bindings, pre-calibration, spi fix and provide iio raw values)
From: H. Nikolaus Schaller @ 2017-01-16  6:34 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
	Michael Welling, Mika Penttilä, Javier Martinez Canillas,
	Igor Grinberg, Andrew F. Davis, Mark Brown, Jonathan Cameron,
	Rob Herring, Nikolaus Schaller, Alexander Stein, Eric Engestrom
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <cover.1482936802.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

ping

> Am 28.12.2016 um 15:53 schrieb H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>:
> 
> Changes V9:
> * added explicit CONFIG for TSC2007_IIO and simplified Makefile (suggested by Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>)
> * make the tsc2007 iio patch the last one in this sequence
> 
> 2016-11-22 15:02:42: Changes V8:
> * fix compilation for CONFIG_IIO=m (reported by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> * add some more Reviewed-by: and Acked-by:
> * mutiple improvements suggested by Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> ** drop patch "send pendown and penup only once" (input core does take care of it now)
> ** remove not necessary EXPORT_SYMBOL
> ** remove explicit file names from comment header
> ** move tsc2007_iio_configure() to the end of the probe process and simplify the error path again
> ** remove unnecessary input_unregister_device() 
> ** improvement for compilation with CONFIG_IIO=m and CONFIG_TSC2007=y
> 
> 2016-11-11 20:02:11: Changes V7:
> * rearranged the include files (asked for by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> * forward reference struct iio_dev * instead of condition in tsc2007.h (asked for by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> * add some Acked-by:
> 
> 2016-10-27 10:44:29: Changes V6:
> * iio patch (no changes elsewhere)
> 	- tsc2007_iio: fix a missing return 0 for non-iio case (found by kbuid test robot)
> 	- tsc2007_core: group error return paths so that tsc2007_iio_unconfigure is called at only one place
> 	- tsc2007_iio: fix copyright (this file is 100% original work)
> 
> 2016-10-25 21:26:46: Changes V5:
> * ads7846: remove an empty line (suggested by Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>)
> * ads7846: remove MODULE_ALIAS for SPI (suggested by Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>)
> * tsc2007: fix a bug from swapping patch 3/n and patch 4/n (found by kbuild test robot)
> * refactored tsc2007 into tsc2007_core and tsc2007_iio (asked for by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> 
> 2016-10-17 16:00:02: Changes V4:
> * fix a merge/squash issue resulting in a non-bisectable patch set (suggested by kbuid test robot)
> * remove some unnecessary #include (suggested by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> * make the iio extension depend on CONFIG_IIO rather than selecting it (suggested by Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>)
> * swapped patch 3/n and patch 4/n to remove internal dependency
> 
> 2016-09-23 14:41:23: Changes V3:
> * fix an issue with swapping
> * remove hard clipping to min/max rectangle - some systems expect to handle negative coordinates
> * make use of commit ed7c9870c9bc ("Input: of_touchscreen - add support for inverted / swapped axes")
> 
> 2015-11-13 21:36:07: Changes V2:
> * add a patch to make drivers still recognise the old "ti,swap-xy" property (suggested by Rob Herring)
> 
> 2015-11-06 16:14:53: V1: This patch series improves the drivers for the tsc2007 and
> ads7846/tsc2046 touchscreen controllers which are e.g. used by the GTA04
> OpenPandora and Pyra devices.
> 
> New common bindings have been defined by
> commit b98abe52fa8e ("Input: add common DT binding for touchscreens"):
> 
> 	Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> 
> which also defines a helper function to parse the DT. These new parameters
> allow to specify the fuzz factors (jitter suppression), inversion of x or y axis and
> swapping of x and y to achieve inversion and rotation so that the touch
> coordinate axes match the natural orientation of the display panel.
> 
> Another improvement is to better use the min/max ADC values and
> scale to the screen size as defined by the DT. This allows to coarsely
> calibrate the touch to match the LCD to which it is glued on so that the
> touch can quite precisely be operated before any user-space fine-calibration
> can be (and needs to be) started.
> 
> For the adc7846 we fix an issue with the spi module table.
> 
> Finally we add an iio interface for the AUX and temperature ADC channels of
> the tsc2007 and also provide the touch screen raw values. This allows to read
> an optional ambient light sensor installed on the gta04 board and improves
> calibration and hardware monitoring.
> 
> 
> H. Nikolaus Schaller (8):
>  drivers:input:tsc2007: add new common binding names, pre-calibration,
>    flipping and rotation
>  drivers:input:tsc2007: check for presence and power down tsc2007
>    during probe
>  DT:omap3+tsc2007: use new common touchscreen bindings
>  drivers:input:ads7846(+tsc2046): add new common binding names,
>    pre-calibration and flipping
>  dt-bindings: input: move ads7846 bindings to touchscreen subdirectory
>  drivers:input:ads7846(+tsc2046): fix spi module table
>  DT:omap3+ads7846: use new common touchscreen bindings
>  drivers:input:tsc2007: add iio interface to read external ADC input
>    and temperature
> 
> .../bindings/input/{ => touchscreen}/ads7846.txt   |   9 +-
> .../bindings/input/touchscreen/tsc2007.txt         |  20 +-
> arch/arm/boot/dts/omap3-gta04.dtsi                 |  25 ++-
> arch/arm/boot/dts/omap3-lilly-a83x.dtsi            |   2 +-
> arch/arm/boot/dts/omap3-pandora-common.dtsi        |  17 +-
> .../boot/dts/omap3-panel-sharp-ls037v7dw01.dtsi    |   3 +-
> drivers/input/touchscreen/Kconfig                  |  10 +
> drivers/input/touchscreen/Makefile                 |   2 +
> drivers/input/touchscreen/ads7846.c                |  71 ++++++--
> drivers/input/touchscreen/tsc2007.h                | 116 ++++++++++++
> .../touchscreen/{tsc2007.c => tsc2007_core.c}      | 201 ++++++++++++---------
> drivers/input/touchscreen/tsc2007_iio.c            | 150 +++++++++++++++
> include/linux/i2c/tsc2007.h                        |   8 +
> 13 files changed, 520 insertions(+), 114 deletions(-)
> rename Documentation/devicetree/bindings/input/{ => touchscreen}/ads7846.txt (90%)
> create mode 100644 drivers/input/touchscreen/tsc2007.h
> rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (74%)
> create mode 100644 drivers/input/touchscreen/tsc2007_iio.c
> 
> -- 
> 2.7.3
> 

^ permalink raw reply

* Re: [PATCH] pcie: ti: Provide patch to force GEN1 PCIe operation
From: Kishon Vijay Abraham I @ 2017-01-16  6:37 UTC (permalink / raw)
  To: Lukasz Majewski, Bjorn Helgaas
  Cc: Rob Herring, Mark Rutland, Jingoo Han, Joao Pinto, linux-omap,
	linux-pci, devicetree, linux-kernel
In-Reply-To: <1484486354-4585-1-git-send-email-lukma@denx.de>

Hi,

On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote:
> Some devices (due to e.g. bad PCIe signal integrity) require to run
> with forced GEN1 speed on PCIe bus.
> 
> This patch changes the speed explicitly on dra7 based devices when
> proper device tree attribute is defined for the PCIe controller.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>

Bjorn has already queued a patch to do the same thing
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xx

Thanks
Kishon

> ---
> 
> Patch applies on newest origin/master
> SHA1: f4d3935e4f4884ba80561db5549394afb8eef8f7
> 
> Tested at AM5728
> 
> ---
>  Documentation/devicetree/bindings/pci/ti-pci.txt |  1 +
>  drivers/pci/host/pci-dra7xx.c                    | 23 +++++++++++++++++++++++
>  drivers/pci/host/pcie-designware.h               |  1 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> index 60e2516..9f97409 100644
> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> @@ -25,6 +25,7 @@ PCIe Designware Controller
>  
>  Optional Property:
>   - gpios : Should be added if a gpio line is required to drive PERST# line
> + - to,pcie-is-gen1: Indicates that forced gen1 port operation is needed.
>  
>  Example:
>  axi {
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 9595fad..eec5fae 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -63,6 +63,13 @@
>  #define	LINK_UP						BIT(16)
>  #define	DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
>  
> +#define         PCIECTRL_EP_DBICS_LNK_CAP                       0x007C
> +#define         MAX_LINK_SPEEDS_MASK				GENMASK(3, 0)
> +#define         MAX_LINK_SPEEDS_GEN1                            BIT(0)
> +
> +#define         PCIECTRL_PL_WIDTH_SPEED_CTL                     0x080C
> +#define         CFG_DIRECTED_SPEED_CHANGE                       BIT(17)
> +
>  struct dra7xx_pcie {
>  	struct pcie_port	pp;
>  	void __iomem		*base;		/* DT ti_conf */
> @@ -270,6 +277,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  	struct pcie_port *pp = &dra7xx->pp;
>  	struct device *dev = pp->dev;
>  	struct resource *res;
> +	u32 val;
>  
>  	pp->irq = platform_get_irq(pdev, 1);
>  	if (pp->irq < 0) {
> @@ -296,6 +304,18 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  	if (!pp->dbi_base)
>  		return -ENOMEM;
>  
> +	if (pp->is_gen1) {
> +		dev_info(dev, "GEN1 forced\n");
> +
> +		val = readl(pp->dbi_base + PCIECTRL_EP_DBICS_LNK_CAP);
> +		set_mask_bits(&val, MAX_LINK_SPEEDS_MASK, MAX_LINK_SPEEDS_GEN1);
> +		writel(val, pp->dbi_base + PCIECTRL_EP_DBICS_LNK_CAP);
> +
> +		val = readl(pp->dbi_base + PCIECTRL_PL_WIDTH_SPEED_CTL);
> +		val &= ~CFG_DIRECTED_SPEED_CHANGE;
> +		writel(val, pp->dbi_base + PCIECTRL_PL_WIDTH_SPEED_CTL);
> +	}
> +
>  	ret = dw_pcie_host_init(pp);
>  	if (ret) {
>  		dev_err(dev, "failed to initialize host\n");
> @@ -404,6 +424,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  		goto err_gpio;
>  	}
>  
> +	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
> +		pp->is_gen1 = true;
> +
>  	reg = dra7xx_pcie_readl(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
>  	reg &= ~LTSSM_EN;
>  	dra7xx_pcie_writel(dra7xx, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index a567ea2..2fb0b18 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -50,6 +50,7 @@ struct pcie_port {
>  	struct irq_domain	*irq_domain;
>  	unsigned long		msi_data;
>  	u8			iatu_unroll_enabled;
> +	u8                      is_gen1;
>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>  };
>  
> 

^ permalink raw reply

* Re: [PATCH] pcie: ti: Provide patch to force GEN1 PCIe operation
From: Lukasz Majewski @ 2017-01-16  6:49 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Bjorn Helgaas, Rob Herring, Mark Rutland, Jingoo Han, Joao Pinto,
	linux-omap, linux-pci, devicetree, linux-kernel
In-Reply-To: <587C6A40.8070608@ti.com>

Hi Kishon,

> Hi,
> 
> On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote:
> > Some devices (due to e.g. bad PCIe signal integrity) require to run
> > with forced GEN1 speed on PCIe bus.
> > 
> > This patch changes the speed explicitly on dra7 based devices when
> > proper device tree attribute is defined for the PCIe controller.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> Bjorn has already queued a patch to do the same thing
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xx

It seems like Bjorn only modifies CAP registers.

He also needs to change register with 0x080C offset to actually
( PCIECTRL_PL_WIDTH_SPEED_CTL )

Best regards,
Łukasz

> 
> Thanks
> Kishon
> 
> > ---
> > 
> > Patch applies on newest origin/master
> > SHA1: f4d3935e4f4884ba80561db5549394afb8eef8f7
> > 
> > Tested at AM5728
> > 
> > ---
> >  Documentation/devicetree/bindings/pci/ti-pci.txt |  1 +
> >  drivers/pci/host/pci-dra7xx.c                    | 23
> > +++++++++++++++++++++++
> > drivers/pci/host/pcie-designware.h               |  1 + 3 files
> > changed, 25 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt
> > b/Documentation/devicetree/bindings/pci/ti-pci.txt index
> > 60e2516..9f97409 100644 ---
> > a/Documentation/devicetree/bindings/pci/ti-pci.txt +++
> > b/Documentation/devicetree/bindings/pci/ti-pci.txt @@ -25,6 +25,7
> > @@ PCIe Designware Controller 
> >  Optional Property:
> >   - gpios : Should be added if a gpio line is required to drive
> > PERST# line
> > + - to,pcie-is-gen1: Indicates that forced gen1 port operation is
> > needed. 
> >  Example:
> >  axi {
> > diff --git a/drivers/pci/host/pci-dra7xx.c
> > b/drivers/pci/host/pci-dra7xx.c index 9595fad..eec5fae 100644
> > --- a/drivers/pci/host/pci-dra7xx.c
> > +++
> > b/drivers/pci/host/pci-https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xxdra7xx.c
> > @@ -63,6 +63,13 @@ #define
> > LINK_UP						BIT(16)
> > #define
> > DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
> > +#define         PCIECTRL_EP_DBICS_LNK_CAP
> > 0x007C +#define
> > MAX_LINK_SPEEDS_MASK				GENMASK(3, 0)
> > +#define         MAX_LINK_SPEEDS_GEN1
> > BIT(0) + +#define
> > PCIECTRL_PL_WIDTH_SPEED_CTL                     0x080C
> > +#define         CFG_DIRECTED_SPEED_CHANGE
> > BIT(17) + struct dra7xx_pcie { struct pcie_port	pp;
> >  	void __iomem		*base;		/* DT
> > ti_conf */ @@ -270,6 +277,7 @@ static int __init
> > dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, struct pcie_port
> > *pp = &dra7xx->pp; struct device *dev = pp->dev;
> >  	struct resource *res;
> > +	u32 val;
> >  
> >  	pp->irq = platform_get_irq(pdev, 1);
> >  	if (pp->irq < 0) {
> > @@ -296,6 +304,18 @@ static int __init dra7xx_add_pcie_port(struct
> > dra7xx_pcie *dra7xx, if (!pp->dbi_base)
> >  		return -ENOMEM;
> >  
> > +	if (pp->is_gen1) {
> > +		dev_info(dev, "GEN1 forced\n");
> > +
> > +		val = readl(pp->dbi_base +
> > PCIECTRL_EP_DBICS_LNK_CAP);
> > +		set_mask_bits(&val, MAX_LINK_SPEEDS_MASK,
> > MAX_LINK_SPEEDS_GEN1);
> > +		writel(val, pp->dbi_base +
> > PCIECTRL_EP_DBICS_LNK_CAP); +
> > +		val = readl(pp->dbi_base +
> > PCIECTRL_PL_WIDTH_SPEED_CTL);
> > +		val &= ~CFG_DIRECTED_SPEED_CHANGE;
> > +		writel(val, pp->dbi_base +
> > PCIECTRL_PL_WIDTH_SPEED_CTL);
> > +	}
> > +
> >  	ret = dw_pcie_host_init(pp);
> >  	if (ret) {
> >  		dev_err(dev, "failed to initialize host\n");
> > @@ -404,6 +424,9 @@ static int __init dra7xx_pcie_probe(struct
> > platform_device *pdev) goto err_gpio;
> >  	}
> >  
> > +	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
> > +		pp->is_gen1 = true;
> > +
> >  	reg = dra7xx_pcie_readl(dra7xx,
> > PCIECTRL_DRA7XX_CONF_DEVICE_CMD); reg &= ~LTSSM_EN;
> >  	dra7xx_pcie_writel(dra7xx,
> > PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg); diff --git
> > a/drivers/pci/host/pcie-designware.h
> > b/drivers/pci/host/pcie-designware.h index a567ea2..2fb0b18 100644
> > --- a/drivers/pci/host/pcie-designware.h +++
> > b/drivers/pci/host/pcie-designware.h @@ -50,6 +50,7 @@ struct
> > pcie_port { struct irq_domain	*irq_domain;
> >  	unsigned long		msi_data;
> >  	u8			iatu_unroll_enabled;
> > +	u8                      is_gen1;
> >  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> >  };
> >  
> > 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

^ permalink raw reply

* Re: [PATCH v1 1/3] dt: bindings: add documentation for zx2967 family reset controller
From: Shawn Guo @ 2017-01-16  7:09 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: jun.nie, p.zabel, robh+dt, mark.rutland, linux-arm-kernel,
	devicetree, linux-kernel, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1484377530-30635-1-git-send-email-baoyou.xie@linaro.org>

On Sat, Jan 14, 2017 at 03:05:28PM +0800, Baoyou Xie wrote:
> This patch adds dt-binding documentation for zx2967 family
> reset controller.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

Reviewed-by: Shawn Guo <shawnguo@kernel.org>

^ permalink raw reply

* Re: [PATCH v6 2/3] input: tm2-touchkey: Add touchkey driver support for TM2
From: Jaechul Lee @ 2017-01-16  7:24 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	Kukjin Kim, Krzysztof Kozlowski, Javier Martinez Canillas,
	Andi Shyti, Chanwoo Choi, beomho.seo, galaxyra, linux-arm-kernel,
	linux-input, devicetree, linux-kernel, linux-samsung-soc
In-Reply-To: <20170115071110.GA24007@dtor-ws>

Dear Dmitry Torokhov,

On Sat, Jan 14, 2017 at 11:11:10PM -0800, Dmitry Torokhov wrote:
> Hi Jaechul,
> 
> On Mon, Jan 09, 2017 at 04:22:14PM +0900, Jaechul Lee wrote:
> > +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid)
> > +{
> > +	struct tm2_touchkey_data *touchkey = devid;
> > +	u32 data;
> > +
> > +	data = i2c_smbus_read_byte_data(touchkey->client,
> > +					TM2_TOUCHKEY_KEYCODE_REG);
> > +
> > +	if (data < 0) {
> 
> You declared data as u32 so it will never be negative.

Yes, it won't be negative.

> 
> > +		dev_err(&touchkey->client->dev, "Failed to read i2c data\n");
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	touchkey->keycode_type = data & TM2_TOUCHKEY_BIT_KEYCODE;
> > +	touchkey->pressed = !(data & TM2_TOUCHKEY_BIT_PRESS_EV);
> 
> There is no need to store this in touchkey structure as you are not
> going to use it past this function.

I agree with you. it doesn't need to store variables in touchkey structure.

> 
> Does the version of the patch below work for you?

I found that the condition is inverted.
if data & TM2_TOUCHKEY_BIT_PRESS_EV is true, it means touchkey is released.

it should be changed like this.

	if (data & TM2_TOUCHKEY_BIT_PRESS_EV) {
		input_report_key(touchkey->input_dev, KEY_PHONE, 0);
		input_report_key(touchkey->input_dev, KEY_BACK, 0);
	} else {
		input_report_key(touchkey->input_dev, key, 1);
	}

I will prepare for patch v7 based on your modifications.
Thank you very much for your reviews.

Best Regards,
Jaechul

> 
> Thanks.
> 
> -- 
> Dmitry
> 
> 
> Input: tm2-touchkey - add touchkey driver support for TM2
> 
> From: Jaechul Lee <jcsing.lee@samsung.com>
> 
> 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>
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> Patchwork-Id: 9504149
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/Kconfig        |   11 +
>  drivers/input/keyboard/Makefile       |    1 
>  drivers/input/keyboard/tm2-touchkey.c |  286 +++++++++++++++++++++++++++++++++
>  3 files changed, 298 insertions(+)
>  create mode 100644 drivers/input/keyboard/tm2-touchkey.c
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index cbd75cf44739..97acd6524ad7 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -666,6 +666,17 @@ config KEYBOARD_TC3589X
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called tc3589x-keypad.
>  
> +config KEYBOARD_TM2_TOUCHKEY
> +	tristate "TM2 touchkey support"
> +	depends on I2C
> +	depends on LEDS_CLASS
> +	help
> +	  Say Y here to enable device driver for tm2-touchkey with
> +	  LED control for the Exynos5433 TM2 board.
> +
> +	  To compile this driver as a module, choose M here.
> +	  module will be called tm2-touchkey.
> +
>  config KEYBOARD_TWL4030
>  	tristate "TI TWL4030/TWL5030/TPS659x0 keypad support"
>  	depends on TWL4030_CORE
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index d9f4cfcf3410..7d9acff819a7 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_KEYBOARD_SUN4I_LRADC)	+= sun4i-lradc-keys.o
>  obj-$(CONFIG_KEYBOARD_SUNKBD)		+= sunkbd.o
>  obj-$(CONFIG_KEYBOARD_TC3589X)		+= tc3589x-keypad.o
>  obj-$(CONFIG_KEYBOARD_TEGRA)		+= tegra-kbc.o
> +obj-$(CONFIG_KEYBOARD_TM2_TOUCHKEY)	+= tm2-touchkey.o
>  obj-$(CONFIG_KEYBOARD_TWL4030)		+= twl4030_keypad.o
>  obj-$(CONFIG_KEYBOARD_XTKBD)		+= xtkbd.o
>  obj-$(CONFIG_KEYBOARD_W90P910)		+= w90p910_keypad.o
> diff --git a/drivers/input/keyboard/tm2-touchkey.c b/drivers/input/keyboard/tm2-touchkey.c
> new file mode 100644
> index 000000000000..79bc2d2bd4b9
> --- /dev/null
> +++ b/drivers/input/keyboard/tm2-touchkey.c
> @@ -0,0 +1,286 @@
> +/*
> + * TM2 touchkey device driver
> + *
> + * Copyright 2005 Phil Blundell
> + * Copyright 2016 Samsung Electronics Co., Ltd.
> + *
> + * Author: Beomho Seo <beomho.seo@samsung.com>
> + * Author: Jaechul Lee <jcsing.lee@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pm.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define TM2_TOUCHKEY_DEV_NAME		"tm2-touchkey"
> +#define TM2_TOUCHKEY_KEYCODE_REG	0x03
> +#define TM2_TOUCHKEY_BASE_REG		0x00
> +#define TM2_TOUCHKEY_CMD_LED_ON		0x10
> +#define TM2_TOUCHKEY_CMD_LED_OFF	0x20
> +#define TM2_TOUCHKEY_BIT_PRESS_EV	BIT(3)
> +#define TM2_TOUCHKEY_BIT_KEYCODE	GENMASK(2, 0)
> +#define TM2_TOUCHKEY_LED_VOLTAGE_MIN	2500000
> +#define TM2_TOUCHKEY_LED_VOLTAGE_MAX	3300000
> +
> +enum {
> +	TM2_TOUCHKEY_KEY_MENU = 0x1,
> +	TM2_TOUCHKEY_KEY_BACK,
> +};
> +
> +struct tm2_touchkey_data {
> +	struct i2c_client *client;
> +	struct input_dev *input_dev;
> +	struct led_classdev led_dev;
> +	struct regulator *vdd;
> +	struct regulator_bulk_data regulators[2];
> +};
> +
> +static void tm2_touchkey_led_brightness_set(struct led_classdev *led_dev,
> +					    enum led_brightness brightness)
> +{
> +	struct tm2_touchkey_data *touchkey =
> +		container_of(led_dev, struct tm2_touchkey_data, led_dev);
> +	u32 volt;
> +	u8 data;
> +
> +	if (brightness == LED_OFF) {
> +		volt = TM2_TOUCHKEY_LED_VOLTAGE_MIN;
> +		data = TM2_TOUCHKEY_CMD_LED_OFF;
> +	} else {
> +		volt = TM2_TOUCHKEY_LED_VOLTAGE_MAX;
> +		data = TM2_TOUCHKEY_CMD_LED_ON;
> +	}
> +
> +	regulator_set_voltage(touchkey->vdd, volt, volt);
> +	i2c_smbus_write_byte_data(touchkey->client,
> +				  TM2_TOUCHKEY_BASE_REG, data);
> +}
> +
> +static int tm2_touchkey_power_enable(struct tm2_touchkey_data *touchkey)
> +{
> +	int error;
> +
> +	error = regulator_bulk_enable(ARRAY_SIZE(touchkey->regulators),
> +				      touchkey->regulators);
> +	if (error)
> +		return error;
> +
> +	/* waiting for device initialization, at least 150ms */
> +	msleep(150);
> +
> +	return 0;
> +}
> +
> +static void tm2_touchkey_power_disable(void *data)
> +{
> +	struct tm2_touchkey_data *touchkey = data;
> +
> +	regulator_bulk_disable(ARRAY_SIZE(touchkey->regulators),
> +			       touchkey->regulators);
> +}
> +
> +static irqreturn_t tm2_touchkey_irq_handler(int irq, void *devid)
> +{
> +	struct tm2_touchkey_data *touchkey = devid;
> +	int data;
> +	int key;
> +
> +	data = i2c_smbus_read_byte_data(touchkey->client,
> +					TM2_TOUCHKEY_KEYCODE_REG);
> +	if (data < 0) {
> +		dev_err(&touchkey->client->dev,
> +			"failed to read i2c data: %d\n", data);
> +		goto out;
> +	}
> +
> +	switch (data & TM2_TOUCHKEY_BIT_KEYCODE) {
> +	case TM2_TOUCHKEY_KEY_MENU:
> +		key = KEY_PHONE;
> +		break;
> +
> +	case TM2_TOUCHKEY_KEY_BACK:
> +		key = KEY_BACK;
> +		break;
> +
> +	default:
> +		dev_warn(&touchkey->client->dev,
> +			 "unhandled keycode, data %#02x\n", data);
> +		goto out;
> +	}
> +
> +	if (data & TM2_TOUCHKEY_BIT_PRESS_EV) {
> +		input_report_key(touchkey->input_dev, key, 1);
> +	} else {
> +		input_report_key(touchkey->input_dev, KEY_PHONE, 0);
> +		input_report_key(touchkey->input_dev, KEY_BACK, 0);
> +	}
> +
> +	input_sync(touchkey->input_dev);
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +static int tm2_touchkey_probe(struct i2c_client *client,
> +			      const struct i2c_device_id *id)
> +{
> +	struct tm2_touchkey_data *touchkey;
> +	int error;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +			I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA)) {
> +		dev_err(&client->dev, "incompatible I2C adapter\n");
> +		return -EIO;
> +	}
> +
> +	touchkey = devm_kzalloc(&client->dev, sizeof(*touchkey), GFP_KERNEL);
> +	if (!touchkey)
> +		return -ENOMEM;
> +
> +	touchkey->client = client;
> +	i2c_set_clientdata(client, touchkey);
> +
> +	touchkey->regulators[0].supply = "vcc";
> +	touchkey->regulators[1].supply = "vdd";
> +	error = devm_regulator_bulk_get(&client->dev,
> +					ARRAY_SIZE(touchkey->regulators),
> +					touchkey->regulators);
> +	if (error) {
> +		dev_err(&client->dev, "failed to get regulators: %d\n", error);
> +		return error;
> +	}
> +
> +	/* Save VDD for easy access */
> +	touchkey->vdd = touchkey->regulators[1].consumer;
> +
> +	error = tm2_touchkey_power_enable(touchkey);
> +	if (error) {
> +		dev_err(&client->dev, "failed to power up device: %d\n", error);
> +		return error;
> +	}
> +
> +	error = devm_add_action_or_reset(&client->dev,
> +					 tm2_touchkey_power_disable, touchkey);
> +	if (error) {
> +		dev_err(&client->dev,
> +			"failed to install poweroff handler: %d\n", error);
> +		return error;
> +	}
> +
> +	/* input device */
> +	touchkey->input_dev = devm_input_allocate_device(&client->dev);
> +	if (!touchkey->input_dev) {
> +		dev_err(&client->dev, "failed to allocate input device\n");
> +		return -ENOMEM;
> +	}
> +
> +	touchkey->input_dev->name = TM2_TOUCHKEY_DEV_NAME;
> +	touchkey->input_dev->id.bustype = BUS_I2C;
> +
> +	input_set_capability(touchkey->input_dev, EV_KEY, KEY_PHONE);
> +	input_set_capability(touchkey->input_dev, EV_KEY, KEY_BACK);
> +
> +	input_set_drvdata(touchkey->input_dev, touchkey);
> +
> +	error = input_register_device(touchkey->input_dev);
> +	if (error) {
> +		dev_err(&client->dev,
> +			"failed to register input device: %d\n", error);
> +		return error;
> +	}
> +
> +	error = devm_request_threaded_irq(&client->dev, client->irq,
> +					  NULL, tm2_touchkey_irq_handler,
> +					  IRQF_ONESHOT,
> +					  TM2_TOUCHKEY_DEV_NAME, touchkey);
> +	if (error) {
> +		dev_err(&client->dev,
> +			"failed to request threaded irq: %d\n", error);
> +		return error;
> +	}
> +
> +	/* led device */
> +	touchkey->led_dev.name = TM2_TOUCHKEY_DEV_NAME;
> +	touchkey->led_dev.brightness = LED_FULL;
> +	touchkey->led_dev.max_brightness = LED_FULL;
> +	touchkey->led_dev.brightness_set = tm2_touchkey_led_brightness_set;
> +
> +	error = devm_led_classdev_register(&client->dev, &touchkey->led_dev);
> +	if (error) {
> +		dev_err(&client->dev,
> +			"failed to register touchkey led: %d\n", error);
> +		return error;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused tm2_touchkey_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client);
> +
> +	disable_irq(client->irq);
> +	tm2_touchkey_power_disable(touchkey);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused tm2_touchkey_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct tm2_touchkey_data *touchkey = i2c_get_clientdata(client);
> +	int ret;
> +
> +	enable_irq(client->irq);
> +
> +	ret = tm2_touchkey_power_enable(touchkey);
> +	if (ret)
> +		dev_err(dev, "failed to enable power: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(tm2_touchkey_pm_ops,
> +			 tm2_touchkey_suspend, tm2_touchkey_resume);
> +
> +static const struct i2c_device_id tm2_touchkey_id_table[] = {
> +	{ TM2_TOUCHKEY_DEV_NAME, 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, tm2_touchkey_id_table);
> +
> +static const struct of_device_id tm2_touchkey_of_match[] = {
> +	{ .compatible = "cypress,tm2-touchkey", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, tm2_touchkey_of_match);
> +
> +static struct i2c_driver tm2_touchkey_driver = {
> +	.driver = {
> +		.name = TM2_TOUCHKEY_DEV_NAME,
> +		.pm = &tm2_touchkey_pm_ops,
> +		.of_match_table = of_match_ptr(tm2_touchkey_of_match),
> +	},
> +	.probe = tm2_touchkey_probe,
> +	.id_table = tm2_touchkey_id_table,
> +};
> +module_i2c_driver(tm2_touchkey_driver);
> +
> +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
> +MODULE_AUTHOR("Jaechul Lee <jcsing.lee@samsung.com>");
> +MODULE_DESCRIPTION("Samsung touchkey driver");
> +MODULE_LICENSE("GPL v2");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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 0/2] Add rockchip serial flash controller support
From: Shawn Lin @ 2017-01-16  7:28 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Cyrille Pitchen
  Cc: shawn.lin-TNX95d0MmH7DzftRWevZcw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner, Marek Vasut,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1481794068-241619-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Hi Cyrille,

On 2016/12/15 17:27, Shawn Lin wrote:
>
> Here is another try for adding serial flash controller
> , namely SFC, found on Rockchip RK1108 platform.
>

Seems this patchset was acked by Marek and there is no more
comments from then. Does it look good to you?


> Feature:
> (1) Support x1, x2, x4 data bits mode
> (2) Support up to 4 chip select
> (3) Support two independent clock domain: AHB clock and SPI clock
> (4) Support DMA master up to 16KB/transfer
>
> Test environment:
> This patchset was tested on RK1108 evb boards with Winboud flash
> (w25q256) and working fine with PIO or DMA mode.
>
> How-to:
> Any rockchip guys who are interested in testing it could refer to
> the following steps:
> (1) enable CONFIG_MTD_M25P80
> (2) enable CONFIG_SPI_ROCKCHIP_SFC
> (3) enable CONFIG_MTD_CMDLINE_PARTS
> (4) enable CONFIG_SQUASHFS
> (4) CONFIG_CMDLINE="root=/dev/mtdblock2
> 	mtdparts=spi-nor:256k@0(loader)ro,8m(kernel)ro,7m(rootfs),-(freedisk)"
> 	Of course, you should check the partition layout if you modify it. Also
> 	you could pass it from your loader to the kernel's cmdline.
> (5) Add dts support:
> nor_flash: sfc@301c0000 {
> 	compatible = "rockchip,rk1108-sfc", "rockchip,sfc";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
> 	clock-names = "sfc", "hsfc";
> 	interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> 	reg = <0x301c0000 0x1000>;
> 	/* If you want to use PIO mode, activate this */
> 	#rockchip,sfc-no-dma;
> 	spi-nor@0 {
> 		compatible = "jedec,spi-nor";
> 		spi-max-frequency = <12000000>;
> 		reg = <0>;
> 	}
> };
>
> please make sure your DT's mdtid matchs what you assgin to the
> mdtparts(cmdline), namely they are both *spi-nor* here.
>
> With enabling DBG for cmdlinepart.c, you could get following log and
> boot kernel and rootfs successfully.
>
> [    0.481420] rockchip-sfc 301c0000.sfc: w25q256 (32768 Kbytes)
> [    0.481962] DEBUG-CMDLINE-PART: parsing
> <256k@0(loader)ro,8m(kernel)ro,7m(rootfs)ro,-(freedisk)>
> [    0.482897] DEBUG-CMDLINE-PART: partition 3: name
> <freedisk>, offset ffffffffffffffff, size ffffffffffffffff, mask flags 0
> [    0.484021] DEBUG-CMDLINE-PART: partition 2: name
> <rootfs>, offset ffffffffffffffff, size 700000, mask flags 400
> [    0.485066] DEBUG-CMDLINE-PART: partition 1: name
> <kernel>, offset ffffffffffffffff, size 800000, mask flags 400
> [    0.486108] DEBUG-CMDLINE-PART: partition 0: name
> <loader>, offset 0, size 40000, mask flags 400
> [    0.487152] DEBUG-CMDLINE-PART: mtdid=<spi-nor> num_parts=<4>
> [    0.487827] 4 cmdlinepart partitions found on MTD device spi-nor
> [    0.488370] Creating 4 MTD partitions on "spi-nor":
> [    0.488826] 0x000000000000-0x000000040000 : "loader"
> [    0.492340] 0x000000040000-0x000000840000 : "kernel"
> [    0.495679] 0x000000840000-0x000000f40000 : "rootfs"
> [    0.499241] 0x000000f40000-0x000002000000 : "freedisk"
>
> [root@arm-linux]#
> [root@arm-linux]#mount
> /dev/root on / type squashfs (ro,relatime)
> devtmpfs on /dev type devtmpfs
> (rw,relatime,size=26124k,nr_inodes=6531,mode=755)
> proc on /proc type proc (rw,relatime)
> none on /tmp type ramfs (rw,relatime)
> none on /var type ramfs (rw,relatime)
> sysfs on /sys type sysfs (rw,relatime)
> debug on /sys/kernel/debug type debugfs (rw,relatime)
> none on /dev/pts type devpts (rw,relatime,mode=600,ptmxmode=000)
>
>
> Changes in v4:
> - use uppercase DMA for description
> - simplify the code of get_if_type
> - use dma_dir to simplify the code
> - simplify the rockchip_sfc_do_rd_wr
> - some minor improvements
> - add reset controller when doing resume
>
> Changes in v3:
> - use io{read32,write32}_rep to simplify the corner cases
> - remove more unnecessary bit definitions
> - some minor comment fixes and improvement
> - fix wrong unregister function
> - unify more code
> - use nor to avoid constantly replicating the whole
>   sfc->flash[sfc->num_chip].nor
> - add email for MODULE_AUTHOR
> - remove #if 1 --- #endif
> - extract DMA code to imporve the code structure
> - reset all when failing to do dma
> - pass sfc to get_if_type
> - rename sfc-no-dma to sfc-no-DMA
>
> Changes in v2:
> - fix typos
> - add some comment for buffer and others operations
> - rename SFC_MAX_CHIP_NUM to MAX_CHIPSELECT_NUM
> - use u8 for cs
> - return -EINVAL for default case of get_if_type
> - use readl_poll_*() to check timeout cases
> - simplify and clarify some condition checks
> - rework the bitshifts to simplify the code
> - define SFC_CMD_DUMMY(x)
> - fix ummap for dma read path and finish all the
>   cache maintenance.
> - rename to rockchip_sfc_chip_priv and embed struct spi_nor
>   in it.
> - add MODULE_AUTHOR
> - add runtime PM and general PM support.
> - Thanks for Marek's comments. Link:
>   http://lists.infradead.org/pipermail/linux-mtd/2016-November/070321.html
>
> Shawn Lin (2):
>   mtd: spi-nor: Bindings for Rockchip serial flash controller
>   mtd: spi-nor: add rockchip serial flash controller driver
>
>  .../devicetree/bindings/mtd/rockchip-sfc.txt       |  31 +
>  MAINTAINERS                                        |   8 +
>  drivers/mtd/spi-nor/Kconfig                        |   7 +
>  drivers/mtd/spi-nor/Makefile                       |   1 +
>  drivers/mtd/spi-nor/rockchip-sfc.c                 | 872 +++++++++++++++++++++
>  5 files changed, 919 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
>  create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c
>


-- 
Best Regards
Shawn Lin

--
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] usb: xhci: add support for performing fake doorbell
From: Rafał Miłecki @ 2017-01-16  7:32 UTC (permalink / raw)
  To: Mathias Nyman, Jon Mason, Florian Fainelli, BCM Kernel Feedback
  Cc: Mathias Nyman, Greg Kroah-Hartman, Hauke Mehrtens, Rob Herring,
	Mark Rutland, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org, Linux Kernel Mailing List,
	Rafał Miłecki
In-Reply-To: <58331360.5000508@linux.intel.com>

On 21 November 2016 at 16:31, Mathias Nyman
<mathias.nyman@linux.intel.com> wrote:
> On 21.11.2016 09:57, Rafał Miłecki wrote:
>>
>> Hi Mathias,
>>
>> On 17 October 2016 at 22:30, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Broadcom's Northstar XHCI controllers seem to need a special start
>>> procedure to work correctly. There isn't any official documentation of
>>> this, the problem is that controller doesn't detect any connected
>>> devices with default setup. Moreover connecting USB device to controller
>>> that doesn't run properly can cause SoC's watchdog issues.
>>>
>>> A workaround that was successfully tested on multiple devices is to
>>> perform a fake doorbell. This patch adds code for doing this and enables
>>> it on BCM4708 family.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> V2: Enable quirk for brcm,bcm4708 machines instead of adding separated
>>> binding
>>>      for it. Thanks Rob for your comment on this.
>>
>>
>> Do you think you can pick & push this one? V2 follows Rob's suggestion
>> and he has some DT knowledge for sure, so I guess it should be OK.
>> --
>
>
> Is there some more background information on this?
>
> I don't have any contacts to Broadcom myself, adding the BMC Kernel Feedback
> list to CC.
> Maybe someone over there has an errata, documentation or just general
> feedback.
>
> How was this workaround even figured out? ringing the doorbell for the first
> device doesn't seem like something found by trial and error,  especially
> when
> xhci specs state that:
>
> "Software shall not write the Doorbell of an endpoint until after it has
> issued a
> Configure Endpoint Command for the endpoint and received a successful
> Command
> Completion Event."
>
> The whole workaround is a bit intrusive, allocating a fake device, ring a
> doorbell for a
> fake device in the wrong state, clearing off HSE (host system error) which
> should only be set
> when things really go bad, some random usleeps, and possible calling
> xhci_start() twice.
>
> I can't take this as is without some more info.

Hi (ping) Broadcom guys, could you help us with this USB workaround, please?

-- 
Rafał

^ permalink raw reply

* Re: [PATCH v1 3/3] reset: zx2967: add reset controller driver for ZTE's zx2967 family
From: Shawn Guo @ 2017-01-16  7:44 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: jun.nie-QSEj5FYQhm4dnm+yROfE0A, p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1484377530-30635-3-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Sat, Jan 14, 2017 at 03:05:30PM +0800, Baoyou Xie wrote:
> This patch adds reset controller driver for ZTE's zx2967 family.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/reset/Kconfig        |   6 ++
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-zx2967.c | 136 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+)
>  create mode 100644 drivers/reset/reset-zx2967.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 172dc96..972d077 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -92,6 +92,12 @@ config RESET_ZYNQ
>  	help
>  	  This enables the reset controller driver for Xilinx Zynq SoCs.
>  
> +config RESET_ZX2967
> +	bool "ZX2967 Reset Driver"
> +	depends on ARCH_ZX || COMPILE_TEST
> +	help
> +	  This enables the reset controller driver for ZTE zx2967 family.
> +

The config options seem to be sorted alphabetically, so RESET_ZX2967
should be put before RESET_ZYNQ.

>  source "drivers/reset/sti/Kconfig"
>  source "drivers/reset/hisilicon/Kconfig"
>  source "drivers/reset/tegra/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 13b346e..807b77b 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> +obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o

Ditto

> diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
> new file mode 100644
> index 0000000..63f9c41
> --- /dev/null
> +++ b/drivers/reset/reset-zx2967.c
> @@ -0,0 +1,136 @@
> +/*
> + * ZTE's zx2967 family thermal sensor driver
> + *
> + * Copyright (C) 2017 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +
> +struct zx2967_reset {
> +	void __iomem			*reg_base;
> +	spinlock_t			lock;
> +	struct reset_controller_dev	rcdev;
> +};
> +
> +static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;
> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg & ~BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));
> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)
> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;
> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg | BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));
> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static struct reset_control_ops zx2967_reset_ops = {
> +	.assert		= zx2967_reset_assert,
> +	.deassert	= zx2967_reset_deassert,
> +};
> +
> +static int zx2967_reset_probe(struct platform_device *pdev)
> +{
> +	struct zx2967_reset *reset;
> +	struct resource *res;
> +
> +	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> +	if (!reset)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset->reg_base))
> +		return PTR_ERR(reset->reg_base);
> +
> +	spin_lock_init(&reset->lock);
> +
> +	reset->rcdev.owner = THIS_MODULE;
> +	reset->rcdev.nr_resets = resource_size(res) * 8;
> +	reset->rcdev.ops = &zx2967_reset_ops;
> +	reset->rcdev.of_node = pdev->dev.of_node;
> +
> +	dev_info(&pdev->dev, "reset controller cnt:%d",
> +		  reset->rcdev.nr_resets);
> +
> +	return reset_controller_register(&reset->rcdev);

Use devm_reset_controller_register(), then we can save the call to
reset_controller_unregister().

Shawn

> +}
> +
> +static int zx2967_reset_remove(struct platform_device *pdev)
> +{
> +	struct zx2967_reset *reset = platform_get_drvdata(pdev);
> +
> +	reset_controller_unregister(&reset->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id zx2967_reset_dt_ids[] = {
> +	 { .compatible = "zte,zx296718-reset", },
> +	 {},
> +};
> +MODULE_DEVICE_TABLE(of, zx2967_reset_dt_ids);
> +
> +static struct platform_driver zx2967_reset_driver = {
> +	.probe	= zx2967_reset_probe,
> +	.remove	= zx2967_reset_remove,
> +	.driver = {
> +		.name		= "zx2967-reset",
> +		.of_match_table	= zx2967_reset_dt_ids,
> +	},
> +};
> +
> +static int __init zx2967_reset_init(void)
> +{
> +	return platform_driver_register(&zx2967_reset_driver);
> +}
> +arch_initcall(zx2967_reset_init);
> +
> +static void __exit zx2967_reset_exit(void)
> +{
> +	platform_driver_unregister(&zx2967_reset_driver);
> +}
> +module_exit(zx2967_reset_exit);
> +
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("ZTE zx2967 Reset Controller Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.7.4
> 
--
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 v1 3/3] reset: zx2967: add reset controller driver for ZTE's zx2967 family
From: Shawn Guo @ 2017-01-16  7:58 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: jun.nie, p.zabel, robh+dt, mark.rutland, linux-arm-kernel,
	devicetree, linux-kernel, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1484377530-30635-3-git-send-email-baoyou.xie@linaro.org>

On Sat, Jan 14, 2017 at 03:05:30PM +0800, Baoyou Xie wrote:
> +static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;

u32 is probably better for register value.

> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg & ~BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));

Is this read on the register is necessary?  If so, we should probably
have a comment for that.

> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)

Please indent the line right after parentheses.

> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;
> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg | BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));
> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}

Only difference between these two functions is only one line.  Should we
consolidate them a bit?

Shawn

^ permalink raw reply

* Re: [PATCH] fbdev: ssd1307fb: allow reset-gpios is missing
From: Maxime Ripard @ 2017-01-16  8:02 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Rob Herring, David Airlie, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170115112146.1787-1-icenowy-ymACFijhrKM@public.gmane.org>

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

On Sun, Jan 15, 2017 at 07:21:46PM +0800, Icenowy Zheng wrote:
> Currently some SSD1306 OLED modules are sold without a reset pin (only
> VCC, GND, SCK, SDA four pins).
> 
> Add support for missing reset-gpios property.
> 
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

Unfortunately, a similar patch has been sent a couple of times
already:
https://www.spinics.net/lists/devicetree/msg158330.html

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

^ permalink raw reply

* Re: [PATCH] pcie: ti: Provide patch to force GEN1 PCIe operation
From: Kishon Vijay Abraham I @ 2017-01-16  8:12 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Bjorn Helgaas, Rob Herring, Mark Rutland, Jingoo Han, Joao Pinto,
	linux-omap, linux-pci, devicetree, linux-kernel
In-Reply-To: <20170116074927.086418f4@jawa>

Hi Łukasz,

On Monday 16 January 2017 12:19 PM, Lukasz Majewski wrote:
> Hi Kishon,
> 
>> Hi,
>>
>> On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote:
>>> Some devices (due to e.g. bad PCIe signal integrity) require to run
>>> with forced GEN1 speed on PCIe bus.
>>>
>>> This patch changes the speed explicitly on dra7 based devices when
>>> proper device tree attribute is defined for the PCIe controller.
>>>
>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>
>> Bjorn has already queued a patch to do the same thing
>> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xx
> 
> It seems like Bjorn only modifies CAP registers.

The patch also modifies the LNKCTL2 register.
> 
> He also needs to change register with 0x080C offset to actually
> ( PCIECTRL_PL_WIDTH_SPEED_CTL )

This bit is used to initiate speed change (after the link is initialized in
GEN1). Resetting the bit (like what you have done here) prevents speed change.

IMO the better way is to set the LNKCTL2 to GEN1 instead of hacking the IP
register.

Thanks
Kishon

> 
> Best regards,
> Łukasz
> 
>>
>> Thanks
>> Kishon
>>
>>> ---
>>>
>>> Patch applies on newest origin/master
>>> SHA1: f4d3935e4f4884ba80561db5549394afb8eef8f7
>>>
>>> Tested at AM5728
>>>
>>> ---
>>>  Documentation/devicetree/bindings/pci/ti-pci.txt |  1 +
>>>  drivers/pci/host/pci-dra7xx.c                    | 23
>>> +++++++++++++++++++++++
>>> drivers/pci/host/pcie-designware.h               |  1 + 3 files
>>> changed, 25 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt
>>> b/Documentation/devicetree/bindings/pci/ti-pci.txt index
>>> 60e2516..9f97409 100644 ---
>>> a/Documentation/devicetree/bindings/pci/ti-pci.txt +++
>>> b/Documentation/devicetree/bindings/pci/ti-pci.txt @@ -25,6 +25,7
>>> @@ PCIe Designware Controller 
>>>  Optional Property:
>>>   - gpios : Should be added if a gpio line is required to drive
>>> PERST# line
>>> + - to,pcie-is-gen1: Indicates that forced gen1 port operation is
>>> needed. 
>>>  Example:
>>>  axi {
>>> diff --git a/drivers/pci/host/pci-dra7xx.c
>>> b/drivers/pci/host/pci-dra7xx.c index 9595fad..eec5fae 100644
>>> --- a/drivers/pci/host/pci-dra7xx.c
>>> +++
>>> b/drivers/pci/host/pci-https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xxdra7xx.c
>>> @@ -63,6 +63,13 @@ #define
>>> LINK_UP						BIT(16)
>>> #define
>>> DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
>>> +#define         PCIECTRL_EP_DBICS_LNK_CAP
>>> 0x007C +#define
>>> MAX_LINK_SPEEDS_MASK				GENMASK(3, 0)
>>> +#define         MAX_LINK_SPEEDS_GEN1
>>> BIT(0) + +#define
>>> PCIECTRL_PL_WIDTH_SPEED_CTL                     0x080C
>>> +#define         CFG_DIRECTED_SPEED_CHANGE
>>> BIT(17) + struct dra7xx_pcie { struct pcie_port	pp;
>>>  	void __iomem		*base;		/* DT
>>> ti_conf */ @@ -270,6 +277,7 @@ static int __init
>>> dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx, struct pcie_port
>>> *pp = &dra7xx->pp; struct device *dev = pp->dev;
>>>  	struct resource *res;
>>> +	u32 val;
>>>  
>>>  	pp->irq = platform_get_irq(pdev, 1);
>>>  	if (pp->irq < 0) {
>>> @@ -296,6 +304,18 @@ static int __init dra7xx_add_pcie_port(struct
>>> dra7xx_pcie *dra7xx, if (!pp->dbi_base)
>>>  		return -ENOMEM;
>>>  
>>> +	if (pp->is_gen1) {
>>> +		dev_info(dev, "GEN1 forced\n");
>>> +
>>> +		val = readl(pp->dbi_base +
>>> PCIECTRL_EP_DBICS_LNK_CAP);
>>> +		set_mask_bits(&val, MAX_LINK_SPEEDS_MASK,
>>> MAX_LINK_SPEEDS_GEN1);
>>> +		writel(val, pp->dbi_base +
>>> PCIECTRL_EP_DBICS_LNK_CAP); +
>>> +		val = readl(pp->dbi_base +
>>> PCIECTRL_PL_WIDTH_SPEED_CTL);
>>> +		val &= ~CFG_DIRECTED_SPEED_CHANGE;
>>> +		writel(val, pp->dbi_base +
>>> PCIECTRL_PL_WIDTH_SPEED_CTL);
>>> +	}
>>> +
>>>  	ret = dw_pcie_host_init(pp);
>>>  	if (ret) {
>>>  		dev_err(dev, "failed to initialize host\n");
>>> @@ -404,6 +424,9 @@ static int __init dra7xx_pcie_probe(struct
>>> platform_device *pdev) goto err_gpio;
>>>  	}
>>>  
>>> +	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
>>> +		pp->is_gen1 = true;
>>> +
>>>  	reg = dra7xx_pcie_readl(dra7xx,
>>> PCIECTRL_DRA7XX_CONF_DEVICE_CMD); reg &= ~LTSSM_EN;
>>>  	dra7xx_pcie_writel(dra7xx,
>>> PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg); diff --git
>>> a/drivers/pci/host/pcie-designware.h
>>> b/drivers/pci/host/pcie-designware.h index a567ea2..2fb0b18 100644
>>> --- a/drivers/pci/host/pcie-designware.h +++
>>> b/drivers/pci/host/pcie-designware.h @@ -50,6 +50,7 @@ struct
>>> pcie_port { struct irq_domain	*irq_domain;
>>>  	unsigned long		msi_data;
>>>  	u8			iatu_unroll_enabled;
>>> +	u8                      is_gen1;
>>>  	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
>>>  };
>>>  
>>>
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> 

^ permalink raw reply

* RE: [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-type-adjustment" for INCR burst type
From: Jerry Huang @ 2017-01-16  8:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland@arm.com, balbi@kernel.org,
	devicetree@vger.kernel.org, catalin.marinas@arm.com,
	linux-usb@vger.kernel.org, will.deacon@arm.com,
	linux@armlinux.org.uk, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAL_JsqJuifiSQAt-6FgtYYSukAnjzOLuTGEsO-Spn5LxkU-iwQ@mail.gmail.com>

> -----Original Message-----
> From: Jerry Huang
> Sent: Wednesday, January 04, 2017 10:25 AM
> To: 'Rob Herring' <robh@kernel.org>
> Cc: balbi@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com;
> will.deacon@arm.com; linux@armlinux.org.uk; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: RE: [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-
> type-adjustment" for INCR burst type
> 
> Hi, Rob,
> 
> > -----Original Message-----
> > From: Rob Herring [mailto:robh@kernel.org]
> > Sent: Wednesday, January 04, 2017 5:24 AM
> > To: Jerry Huang <jerry.huang@nxp.com>
> > Cc: balbi@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com;
> > will.deacon@arm.com; linux@armlinux.org.uk;
> > devicetree@vger.kernel.org; linux-usb@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-arm- kernel@lists.infradead.org
> > Subject: Re: [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-
> > type-adjustment" for INCR burst type
> >
> > On Thu, Dec 22, 2016 at 8:52 PM, Jerry Huang <jerry.huang@nxp.com>
> wrote:
> > > Hi, Rob,
> > >> -----Original Message-----
> > >> From: Rob Herring [mailto:robh@kernel.org]
> > >> Sent: Friday, December 23, 2016 2:45 AM
> > >> To: Jerry Huang <jerry.huang@nxp.com>
> > >> Cc: balbi@kernel.org; mark.rutland@arm.com;
> > >> catalin.marinas@arm.com; will.deacon@arm.com;
> > >> linux@armlinux.org.uk; devicetree@vger.kernel.org;
> > >> linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > >> kernel@lists.infradead.org
> > >> Subject: Re: [PATCH v3 2/3] USB3/DWC3: Add property "snps,
> > >> incr-burst- type-adjustment" for INCR burst type
> > >>
> > >> On Mon, Dec 19, 2016 at 05:25:53PM +0800, Changming Huang wrote:
> > >> > New property "snps,incr-burst-type-adjustment = <x>, <y>" for
> > >> > USB3.0
> > >> DWC3.
> > >> > Field "x": 1/0 - undefined length INCR burst type enable or not;
> > >> > Field
> > >> > "y": INCR4/INCR8/INCR16/INCR32/INCR64/INCR128/INCR256 burst
> type.
> > >> >
> > >> > While enabling undefined length INCR burst type and INCR16 burst
> > >> > type, get better write performance on NXP Layerscape platform:
> > >> > around 3% improvement (from 364MB/s to 375MB/s).
> > >> >
> > >> > Signed-off-by: Changming Huang <jerry.huang@nxp.com>
> > >> > ---
> > >> > Changes in v3:
> > >> >   - add new property for INCR burst in usb node.
> > >> >
> > >> >  Documentation/devicetree/bindings/usb/dwc3.txt |    5 +++++
> > >> >  arch/arm/boot/dts/ls1021a.dtsi                 |    1 +
> > >> >  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |    3 +++
> > >> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |    2 ++
> > >> >  4 files changed, 11 insertions(+)
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
> > >> > b/Documentation/devicetree/bindings/usb/dwc3.txt
> > >> > index e3e6983..8c405a3 100644
> > >> > --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> > >> > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> > >> > @@ -55,6 +55,10 @@ Optional properties:
> > >> >     fladj_30mhz_sdbnd signal is invalid or incorrect.
> > >> >
> > >> >   - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to
> > >> > be
> > >> reallocated.
> > >> > + - snps,incr-burst-type-adjustment: Value for INCR burst type of
> > >> GSBUSCFG0
> > >> > +   register, undefined length INCR burst type enable and INCRx type.
> > >> > +   First field is for undefined length INCR burst type enable or not.
> > >> > +   Second field is for largest INCRx type enabled.
> > >>
> > >> Why do you need the first field? Is the 2nd field used if the 1st is 0?
> > >> If not, then just use the presence of the property to enable or not.
> > > The first field is one switch.
> > > When it is 1, means undefined length INCR burst type enabled, we can
> > > use
> > any length less than or equal to the largest-enabled burst length of
> > INCR4/8/16/32/64/128/256.
> > > When it is zero, means INCRx burst mode enabled, we can use one
> > > fixed
> > burst length of 1/4/8/16/32/64/128/256 byte.
> > > So, the 2nd field is used if the 1st is 0, we need to select one
> > > largest burst
> > length the USB controller can support.
> > > If we don't want to change the value of this register (use the
> > > default value),
> > we don't need to add this property to usb node.
> >
> > Just make this a single value with 0 meaning INCR and 4/8/16/etc being
> INCRx.
> Maybe, I didn't describe it clearly.
> According to DWC3 spec, the value "0" of field INCRBrstEna means INCRx
> burst mode, 1 means INCR burst mode.
> Regardless of the value of INCRBrstEna [bit0], we need to modify the other
> field bit[1,2,3,4,5,6,7] to one INCR burst type  for the platform supported.
> Ad you mentioned, if we just use a single value with 0 meaning INCR and
> 4/8/16/etc being INCRx.
> I understand totally that when it is none-zero, we can use it for INCR burst
> mode.
> Then, when it is 0, how to select the INCRx value?
> 
> So, I think we still need two vaue to specify INCRBrstEna and INCRx burst
> type.
Hi, Balbi, 
It seems there is no feedback for my comment, so these patches can be accepted?

^ permalink raw reply

* [PATCH v2 0/4] Add support for es8388 on the rock2
From: Romain Perier @ 2017-01-16  8:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier

This set of patches adds a machine driver for rockchip boards that use
ES8388 codecs. It also adds slave mode to the es8328 driver that
currently only supported the master mode. Then, it adds the required DT
definitions to link rockchip-i2s to the es8388 analog output.

This work is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.co.uk> with some improvements, changes and more
commits.

Changes in v2:
 - Fixed wrong use of the data structure i2c_device_id
 - Fixed wrong dependencies for SND_SOC_ROCKCHIP_ES8388

Romain Perier (4):
  ASoC: es8328-i2c: Add compatible for ES8388
  ASoC: es8328: Add support for slave mode
  ASoC: rockchip: Add machine driver for ES8388 codecs
  arm: dts: Add support for ES8388 to the Radxa Rock 2

 Documentation/devicetree/bindings/sound/es8328.txt |   2 +-
 .../devicetree/bindings/sound/rockchip-es8388.txt  |  28 +++
 arch/arm/boot/dts/rk3288-rock2-square.dts          |  37 +++
 sound/soc/codecs/es8328-i2c.c                      |   2 +
 sound/soc/codecs/es8328.c                          |  28 ++-
 sound/soc/rockchip/Kconfig                         |   9 +
 sound/soc/rockchip/Makefile                        |   2 +
 sound/soc/rockchip/rockchip_es8388.c               | 270 +++++++++++++++++++++
 8 files changed, 371 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
 create mode 100644 sound/soc/rockchip/rockchip_es8388.c

-- 
2.9.3

^ permalink raw reply

* [PATCH v2 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Romain Perier @ 2017-01-16  8:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170116081753.13666-1-romain.perier@collabora.com>

This commit adds a compatible string for everest,es8388. This is
an audio codec that is compatible with es8328.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2:
- Fixed wrong use of the i2c_device_id data structure

 Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
 sound/soc/codecs/es8328-i2c.c                      | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/es8328.txt b/Documentation/devicetree/bindings/sound/es8328.txt
index 30ea8a3..33fbf05 100644
--- a/Documentation/devicetree/bindings/sound/es8328.txt
+++ b/Documentation/devicetree/bindings/sound/es8328.txt
@@ -4,7 +4,7 @@ This device supports both I2C and SPI.
 
 Required properties:
 
-  - compatible : "everest,es8328"
+  - compatible  : Should be "everest,es8328" or "everest,es8388"
   - DVDD-supply : Regulator providing digital core supply voltage 1.8 - 3.6V
   - AVDD-supply : Regulator providing analog supply voltage 3.3V
   - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
index 2d05b5d..318ab28 100644
--- a/sound/soc/codecs/es8328-i2c.c
+++ b/sound/soc/codecs/es8328-i2c.c
@@ -20,12 +20,14 @@
 
 static const struct i2c_device_id es8328_id[] = {
 	{ "es8328", 0 },
+	{ "es8388", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, es8328_id);
 
 static const struct of_device_id es8328_of_match[] = {
 	{ .compatible = "everest,es8328", },
+	{ .compatible = "everest,es8388", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, es8328_of_match);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 2/4] ASoC: es8328: Add support for slave mode
From: Romain Perier @ 2017-01-16  8:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170116081753.13666-1-romain.perier@collabora.com>

Currently, the function that changes the DAI format only supports master
mode. Trying to use a slave mode exits the function with -EINVAL and
leave the codec misconfigured. This commits adds support for enabling
the slave mode.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2: None

 sound/soc/codecs/es8328.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index 37722194..054e123 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -588,10 +588,18 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	struct snd_soc_codec *codec = codec_dai->codec;
 	u8 dac_mode = 0;
 	u8 adc_mode = 0;
+	bool master;
 
-	/* set master/slave audio interface */
-	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)
-		return -EINVAL;
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+		case SND_SOC_DAIFMT_CBM_CFM:
+			master = true;
+			break;
+		case SND_SOC_DAIFMT_CBS_CFS:
+			master = false;
+			break;
+		default:
+			return -EINVAL;
+	}
 
 	/* interface format */
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -620,9 +628,17 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	snd_soc_update_bits(codec, ES8328_ADCCONTROL4,
 			ES8328_ADCCONTROL4_ADCFORMAT_MASK, adc_mode);
 
-	/* Master serial port mode, with BCLK generated automatically */
-	snd_soc_update_bits(codec, ES8328_MASTERMODE,
-			ES8328_MASTERMODE_MSC, ES8328_MASTERMODE_MSC);
+	if (master) {
+		/* Master serial port mode, with BCLK generated automatically */
+		snd_soc_update_bits(codec, ES8328_MASTERMODE,
+				    ES8328_MASTERMODE_MSC,
+				    ES8328_MASTERMODE_MSC);
+	} else {
+		/* Slave serial port mode */
+		snd_soc_update_bits(codec, ES8328_MASTERMODE,
+				    ES8328_MASTERMODE_MSC,
+				    0);
+	}
 
 	return 0;
 }
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 3/4] ASoC: rockchip: Add machine driver for ES8388 codecs
From: Romain Perier @ 2017-01-16  8:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170116081753.13666-1-romain.perier@collabora.com>

The driver is used for Rockchip boards using an audio codec compatible
with ES8388.

This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.com> with some improvements.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2:
- Fixed wrong dependencies for SND_SOC_ROCKCHIP_ES8388, SND_SOC_ROCKCHIP_I2S
  was selected with unmet direct dependencies

 .../devicetree/bindings/sound/rockchip-es8388.txt  |  28 +++
 sound/soc/rockchip/Kconfig                         |   9 +
 sound/soc/rockchip/Makefile                        |   2 +
 sound/soc/rockchip/rockchip_es8388.c               | 270 +++++++++++++++++++++
 4 files changed, 309 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
 create mode 100644 sound/soc/rockchip/rockchip_es8388.c

diff --git a/Documentation/devicetree/bindings/sound/rockchip-es8388.txt b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
new file mode 100644
index 0000000..03a0507
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
@@ -0,0 +1,28 @@
+ROCKCHIP with ES8388 CODECS
+
+Required properties:
+- compatible: "rockchip,rockchip-audio-es8388"
+- rockchip,model: The user-visible name of this sound complex
+- rockchip,i2s-controller: The phandle of the Rockchip I2S controller that's
+  connected to the CODEC
+- rockchip,audio-codec: The phandle of the es8388 audio codec
+
+Optionnal properties:
+- rockchip,hp-en-gpios = The phandle of the GPIO that power up/down the
+  headphone.
+- rockchip,hp-det-gpios = The phandle of the GPIO that detects the headphone
+- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
+
+Example:
+	
+sound {
+	compatible = "rockchip,rockchip-audio-es8388";
+	rockchip,model = "Analog audio output";
+	rockchip,i2s-controller = <&i2s>;
+	rockchip,audio-codec = <&es8388>;
+	rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+	rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&headphone>;
+};
+
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index c783f9a..b159744 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -42,6 +42,15 @@ config SND_SOC_ROCKCHIP_RT5645
 	  Say Y or M here if you want to add support for SoC audio on Rockchip
 	  boards using the RT5645/RT5650 codec, such as Veyron.
 
+config SND_SOC_ROCKCHIP_ES8388
+	tristate "ASoC support for Rockchip boards using a ES8388 codec"
+	depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB && CLKDEV_LOOKUP
+	select SND_SOC_ROCKCHIP_I2S
+	select SND_SOC_ES8328_I2C
+	help
+	  Say Y or M here if you want to add support for SoC audio on Rockchip
+	  boards using the ES8388 Codec such as Radxa Rock 2
+
 config SND_SOC_RK3399_GRU_SOUND
 	tristate "ASoC support multiple codecs for Rockchip RK3399 GRU boards"
 	depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB && CLKDEV_LOOKUP && SPI
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 84e5c7c..759db5e 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -7,8 +7,10 @@ obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
 
 snd-soc-rockchip-max98090-objs := rockchip_max98090.o
 snd-soc-rockchip-rt5645-objs := rockchip_rt5645.o
+snd-soc-rockchip-es8388-objs := rockchip_es8388.o
 snd-soc-rk3399-gru-sound-objs := rk3399_gru_sound.o
 
 obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip-max98090.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_RT5645) += snd-soc-rockchip-rt5645.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_ES8388) += snd-soc-rockchip-es8388.o
 obj-$(CONFIG_SND_SOC_RK3399_GRU_SOUND) += snd-soc-rk3399-gru-sound.o
diff --git a/sound/soc/rockchip/rockchip_es8388.c b/sound/soc/rockchip/rockchip_es8388.c
new file mode 100644
index 0000000..b23ce8e
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_es8388.c
@@ -0,0 +1,270 @@
+/*
+ * Rockchip machine ASoC driver for boards using a ES8388 CODEC.
+ *
+ * Copyright (c) 2016, Collabora Ltd.
+ *
+ * Authors: Sjoerd Simons <sjoerd.simons@collabora.com>,
+ *	    Romain Perier <romain.perier@collabora.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include "rockchip_i2s.h"
+
+#define DRV_NAME "rockchip-snd-es8388"
+
+struct rk_es8388_drvdata {
+	int gpio_hp_en;
+	int gpio_hp_det;
+};
+
+static int rk_es8388_hp_power(struct snd_soc_dapm_widget *w,
+			      struct snd_kcontrol *k, int event)
+{
+	struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(w->dapm->card);
+
+	if (!gpio_is_valid(machine->gpio_hp_en))
+		return 0;
+
+	gpio_set_value_cansleep(machine->gpio_hp_en,
+				SND_SOC_DAPM_EVENT_ON(event));
+
+	return 0;
+}
+
+static struct snd_soc_jack headphone_jack;
+static struct snd_soc_jack_pin headphone_jack_pins[] = {
+	{
+		.pin = "Headphone",
+		.mask = SND_JACK_HEADPHONE
+	},
+};
+
+static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", rk_es8388_hp_power),
+};
+
+static const struct snd_soc_dapm_route rk_audio_map[] = {
+	{"Headphone", NULL, "LOUT2"},
+	{"Headphone", NULL, "ROUT2"},
+};
+
+static const struct snd_kcontrol_new rk_mc_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+};
+
+static int rk_hw_params(struct snd_pcm_substream *substream,
+			struct snd_pcm_hw_params *params)
+{
+	int ret = 0;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int mclk;
+
+	switch (params_rate(params)) {
+	case 8000:
+	case 16000:
+	case 24000:
+	case 32000:
+	case 48000:
+	case 64000:
+	case 96000:
+		mclk = 12288000;
+		break;
+	case 11025:
+	case 22050:
+	case 44100:
+	case 88200:
+		mclk = 11289600;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+				     SND_SOC_CLOCK_OUT);
+
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+				     SND_SOC_CLOCK_IN);
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_jack_gpio rk_hp_jack_gpio = {
+	.name = "Headphone detection",
+	.report = SND_JACK_HEADPHONE,
+	.debounce_time = 150
+};
+
+static int rk_init(struct snd_soc_pcm_runtime *runtime)
+{
+	struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(runtime->card);
+
+	/* Enable Headset Jack detection */
+	if (gpio_is_valid(machine->gpio_hp_det)) {
+		snd_soc_card_jack_new(runtime->card, "Headphone Jack",
+				      SND_JACK_HEADPHONE, &headphone_jack,
+				      headphone_jack_pins,
+				      ARRAY_SIZE(headphone_jack_pins));
+		rk_hp_jack_gpio.gpio = machine->gpio_hp_det;
+		snd_soc_jack_add_gpios(&headphone_jack, 1, &rk_hp_jack_gpio);
+	}
+
+	return 0;
+}
+
+static struct snd_soc_ops rk_ops = {
+	.hw_params = rk_hw_params,
+};
+
+static struct snd_soc_dai_link rk_dailink = {
+	.name = "ES8388",
+	.stream_name = "Audio",
+	.codec_dai_name = "es8328-hifi-analog",
+	.init = rk_init,
+	.ops = &rk_ops,
+	/* Set es8388 as slave */
+	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+		SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card snd_soc_card_rk = {
+	.name = "I2S-ES8388",
+	.dai_link = &rk_dailink,
+	.num_links = 1,
+	.num_aux_devs = 0,
+	.dapm_widgets = rk_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(rk_dapm_widgets),
+	.dapm_routes = rk_audio_map,
+	.num_dapm_routes = ARRAY_SIZE(rk_audio_map),
+	.controls = rk_mc_controls,
+	.num_controls = ARRAY_SIZE(rk_mc_controls),
+};
+
+static int snd_rk_mc_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct snd_soc_card *card = &snd_soc_card_rk;
+	struct device_node *np = pdev->dev.of_node;
+	struct rk_es8388_drvdata *machine;
+
+	machine = devm_kzalloc(&pdev->dev, sizeof(struct rk_es8388_drvdata),
+			       GFP_KERNEL);
+
+	if (!machine)
+		return -ENOMEM;
+
+	card->dev = &pdev->dev;
+
+	machine->gpio_hp_det = of_get_named_gpio(np,
+		"rockchip,hp-det-gpios", 0);
+	if (machine->gpio_hp_det == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	machine->gpio_hp_en = of_get_named_gpio(np,
+		"rockchip,hp-en-gpios", 0);
+	if (machine->gpio_hp_en == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	if (gpio_is_valid(machine->gpio_hp_en)) {
+		ret = devm_gpio_request_one(&pdev->dev, machine->gpio_hp_en,
+					    GPIOF_OUT_INIT_LOW, "hp_en");
+		if (ret) {
+			dev_err(card->dev, "cannot get hp_en gpio\n");
+			return ret;
+		}
+	}
+
+	ret = snd_soc_of_parse_card_name(card, "rockchip,model");
+	if (ret) {
+		dev_err(card->dev, "SoC parse card name failed %d\n", ret);
+		return ret;
+	}
+
+	rk_dailink.codec_of_node = of_parse_phandle(np, "rockchip,audio-codec",
+						    0);
+	if (!rk_dailink.codec_of_node) {
+		dev_err(&pdev->dev,
+			"Property 'rockchip,audio-codec' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	rk_dailink.cpu_of_node = of_parse_phandle(np, "rockchip,i2s-controller",
+						  0);
+	if (!rk_dailink.cpu_of_node) {
+		dev_err(&pdev->dev,
+			"Property 'rockchip,i2s-controller' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	rk_dailink.platform_of_node = rk_dailink.cpu_of_node;
+	snd_soc_card_set_drvdata(card, machine);
+
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Soc register card failed %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, card);
+
+	return ret;
+}
+
+static const struct of_device_id rockchip_es8388_of_match[] = {
+	{ .compatible = "rockchip,rockchip-audio-es8388", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_es8388_of_match);
+
+static struct platform_driver snd_rk_es8388_driver = {
+	.probe = snd_rk_mc_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.pm = &snd_soc_pm_ops,
+		.of_match_table = rockchip_es8388_of_match,
+	},
+};
+
+module_platform_driver(snd_rk_es8388_driver);
+
+MODULE_AUTHOR("Sjoerd Simons");
+MODULE_DESCRIPTION("Rockchip es8388 machine ASoC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 4/4] arm: dts: Add support for ES8388 to the Radxa Rock 2
From: Romain Perier @ 2017-01-16  8:17 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170116081753.13666-1-romain.perier@collabora.com>

This commit adds the DT definition of the es8388 i2c device
found at address 0x10. It also adds the definition for connecting
the Rockchip I2S to the es8388 analog output.

This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.com> with some improvements.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---

Changes in v2: None

 arch/arm/boot/dts/rk3288-rock2-square.dts | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index 96a2e74..4f9b7d2 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -86,6 +86,17 @@
 		#sound-dai-cells = <0>;
 	};
 
+	sound_es8388 {
+		compatible = "rockchip,rockchip-audio-es8388";
+		rockchip,model = "Analog audio output";
+		rockchip,i2s-controller = <&i2s>;
+		rockchip,audio-codec = <&es8388>;
+		rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+		rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&headphone>;
+	};
+
 	sdio_pwrseq: sdio-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		clocks = <&hym8563>;
@@ -173,10 +184,29 @@
 	};
 };
 
+&i2c2 {
+	status = "okay";
+
+	es8388: es8388@10 {
+		compatible = "everest,es8388", "everest,es8328";
+		reg = <0x10>;
+		AVDD-supply = <&vcca_codec>;
+		DVDD-supply = <&vcca_codec>;
+		HPVDD-supply = <&vcca_codec>;
+		PVDD-supply = <&vcca_codec>;
+		clocks = <&cru SCLK_I2S0_OUT>;
+		clock-names = "i2s_clk_out";
+	};
+};
+
 &i2c5 {
 	status = "okay";
 };
 
+&i2s {
+	status = "okay";
+};
+
 &pinctrl {
 	ir {
 		ir_int: ir-int {
@@ -190,6 +220,13 @@
 		};
 	};
 
+	sound {
+		headphone: headphone {
+			rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>,
+					<7 7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	usb {
 		host_vbus_drv: host-vbus-drv {
 			rockchip,pins = <0 14 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Romain Perier @ 2017-01-16  8:19 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <3724319.rPxRAuBZBQ@phil>

Hi Heiko,

Le 13/01/2017 à 17:43, Heiko Stuebner a écrit :
> Hi Romain,
>
> Am Freitag, 13. Januar 2017, 17:14:49 CET schrieb Romain Perier:
>> This commit adds a compatible string for everest,es8388. This is
>> an audio codec that is compatible with es8328.
>>
>> Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
>>   sound/soc/codecs/es8328-i2c.c                      | 3 ++-
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/es8328.txt
>> b/Documentation/devicetree/bindings/sound/es8328.txt index 30ea8a3..33fbf05
>> 100644
>> --- a/Documentation/devicetree/bindings/sound/es8328.txt
>> +++ b/Documentation/devicetree/bindings/sound/es8328.txt
>> @@ -4,7 +4,7 @@ This device supports both I2C and SPI.
>>
>>   Required properties:
>>
>> -  - compatible : "everest,es8328"
>> +  - compatible  : Should be "everest,es8328" or "everest,es8388"
>>     - DVDD-supply : Regulator providing digital core supply voltage 1.8 -
>> 3.6V - AVDD-supply : Regulator providing analog supply voltage 3.3V
>>     - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
>> diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
>> index 2d05b5d..09ae67f 100644
>> --- a/sound/soc/codecs/es8328-i2c.c
>> +++ b/sound/soc/codecs/es8328-i2c.c
>> @@ -19,13 +19,14 @@
>>   #include "es8328.h"
>>
>>   static const struct i2c_device_id es8328_id[] = {
>> -	{ "es8328", 0 },
>> +	{ "es8328", "es8388", 0 },
> that looks strange or I'm just blind.
>
> struct i2c_device_id {
>          char name[I2C_NAME_SIZE];
>          kernel_ulong_t driver_data;     /* Data private to the driver */
> };
>
> so shouldn't this be
>
>   static const struct i2c_device_id es8328_id[] = {
>   	{ "es8328", 0 },
> +	{ "es8388", 0 },
>
> instead?

Good catch!

thanks,
Romain
--
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 v29 9/9] Documentation: dt: chosen properties for arm64 kdump
From: AKASHI Takahiro @ 2017-01-16  8:25 UTC (permalink / raw)
  To: Mark Rutland
  Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, james.morse-5wv7dgnIgG8,
	geoff-wEGCiKHe2LqWVfeAwA7xHQ,
	bauerman-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	dyoung-H+wXaHxf7aLQT0dZR+AlfA,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170113111756.GC26120@leverpostej>

On Fri, Jan 13, 2017 at 11:17:56AM +0000, Mark Rutland wrote:
> On Fri, Jan 13, 2017 at 06:13:49PM +0900, AKASHI Takahiro wrote:
> > On Thu, Jan 12, 2017 at 03:39:45PM +0000, Mark Rutland wrote:
> > > On Wed, Dec 28, 2016 at 01:37:34PM +0900, AKASHI Takahiro wrote:
> > > > +linux,crashkernel-base
> > > > +linux,crashkernel-size
> > > > +----------------------
> > > > +
> > > > +These properties (currently used on PowerPC and arm64) indicates
> > > > +the base address and the size, respectively, of the reserved memory
> > > > +range for crash dump kernel.
> > > 
> > > From this description, it's not clear to me what the (expected)
> > > consumers of this property are, nor what is expected to provide it.
> > > 
> > > In previous rounds of review, I had assumed that this was used to
> > > describe a preference to the first kernel as to what region of memory
> > > should be used for a subsequent kdump kernel. Looking around, I'm not
> > > sure if I was correct in that assessment.
> > > 
> > > I see that arch/powerpc seems to consume this property to configure
> > > crashk_res, but it also rewrites it based on crashk_res, presumably for
> > > the benefit of userspace. It's not clear to me how on powerpc the kdump
> > > kernel knows its memory range -- is more DT modification done in the
> > > kernel and/or userspace?
> > 
> > I don't believe that powerpc will rewrite the property any way.
> > As far as I know from *the source code*, powerpc kernel retrieves
> > the memory range for crash dump kernel from a kernel command line, i.e.
> > crashkernel=, and then exposes it through DT to userspace (assuming
> > kexec-tools).
> 
> The rewriting I describe is in export_crashk_values() in
> arch/powerpc/kernel/machine_kexec.c, where the code deletes existing the
> properties, and adds new ones, to the DT exposed to userspace.
> 
> So I think we're just quibbling over the definition of "rewrite".

Gotcha

> > > arm64 we should either ensure that /proc/iomem is consistently usable
> > > (and have userspace consistently use it), or we should expose a new file
> > > specifically to expose this information.
> > 
> > The thing that I had in my mind when adding this property is that
> > /proc/iomem would be obsolete in the future, then we should have
> > an alternative in hand.
> 
> Ok.
> 
> My disagreement is with using the DT as a channel to convey information
> from the kernel to userspace.
> 
> I'm more than happy for a new file or other mechanism to express this
> information. For example, we could add
> /sys/kernel/kexec_crash_{base,size} or similar.

It may make sense because /sys/kernel/kexec_crash_size already exists,
so why not kexec_crash_base?
My concern, however, is that this kind of interface might prevent us from
allowing multiple regions to be reserved for crash dump kernel in the future.
(There is an assumption that we have only one region at least on arm64 though.)

Thanks,
-Takahiro AKASHI

> 
> > > Further, I do not think we need this property. It makes more sense to me
> > > for the preference of a a region to be described to the *first* kernel
> > > using the command line consistently.
> > > 
> > > So I think we should drop this property, and not use it on arm64. Please
> > > document this as powerpc only.
> > 
> > OK, but if we drop the property from arm64 code, we have no reason
> > to leave its description in this patch.
> > (In fact, there are a few more (undocumented) properties that only ppc
> > uses for kdump.)
> 
> I'm happy to drop it, then.
> 
> > > > +linux,usable-memory-range
> > > > +-------------------------
> > > > +
> > > > +This property (currently used only on arm64) holds the memory range,
> > > > +the base address and the size, which can be used as system ram on
> > > > +the *current* kernel. Note that, if this property is present, any memory
> > > > +regions under "memory" nodes in DT blob or ones marked as "conventional
> > > > +memory" in EFI memory map should be ignored.
> > > 
> > > Could you please replace this with:
> > > 
> > >   This property (arm64 only) holds a base address and size, describing a
> > >   limited region in which memory may be considered available for use by
> > >   the kernel. Memory outside of this range is not available for use.
> > >   
> > >   This property describes a limitation: memory within this range is only
> > >   valid when also described through another mechanism that the kernel
> > >   would otherwise use to determine available memory (e.g. memory nodes
> > >   or the EFI memory map). Valid memory may be sparse within the range.
> > 
> > Sure.
> 
> Cheers!
> 
> Thanks,
> Mark.
--
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/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
From: Kishon Vijay Abraham I @ 2017-01-16  8:37 UTC (permalink / raw)
  To: Jaehoon Chung, linux-pci
  Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
	mark.rutland, kgene, krzk, jingoohan1, vivek.gautam, pankaj.dubey,
	alim.akhtar, cpgs
In-Reply-To: <20170104123435.30740-3-jh80.chung@samsung.com>

Hi,

On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
> This patch supports to use Generic Phy framework for Exynos PCIe phy.
> When Exynos that supported the pcie want to use the PCIe,
> it needs to control the phy resgister.
> But it should be more complex to control in their own PCIe device drivers.
> 
> Currently, there is an exynos5440 case to support the pcie.
> So this driver is based on Exynos5440 PCIe.
> In future, will support the Other exynos SoCs likes exynos5433, exynos7.

please re-write the commit message.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Not include the codes relevant to pci-exynos.
> - Remove the getting child node.
> 
>  drivers/phy/Kconfig           |   9 ++
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f2..2dddef4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>  	  This driver provides PHY interface for USB 3.0 DRD controller
>  	  present on Exynos5 SoC series.
>  
> +config PHY_EXYNOS_PCIE
> +	bool "Exynos PCIe PHY driver"
> +	depends on ARCH_EXYNOS && OF

include COMPILE_TEST
> +	depends on PCI_EXYNOS

PCI_EXYNOS should depend on PHY_EXYNOS_PCIE if at all required. Or else do away
with this dependency.
> +	select GENERIC_PHY
> +	help
> +	  Enable PCIe PHY support for Exynos SoC series.
> +	  This driver provides PHY interface for Exynos PCIe controller.
> +
>  config PHY_PISTACHIO_USB
>  	tristate "IMG Pistachio USB2.0 PHY driver"
>  	depends on MACH_PISTACHIO
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f4..081aeb4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2)	+= phy-exynos4x12-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2)	+= phy-exynos5250-usb2.o
>  phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)	+= phy-s5pv210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
> +obj-$(CONFIG_PHY_EXYNOS_PCIE)	+= phy-exynos-pcie.o
>  obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
>  obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>  obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2)	+= phy-rockchip-inno-usb2.o
> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
> new file mode 100644
> index 0000000..b57f49b
> --- /dev/null
> +++ b/drivers/phy/phy-exynos-pcie.c
> @@ -0,0 +1,280 @@
> +/*
> + * Samsung EXYNOS SoC series PCIe PHY driver
> + *
> + * Phy provider for PCIe controller on Exynos SoC series
> + *
> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.

2017?
> + * Jaehoon Chung <jh80.chung@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +
> +/* PCIe Purple registers */
> +#define PCIE_PHY_GLOBAL_RESET		0x000
> +#define PCIE_PHY_COMMON_RESET		0x004
> +#define PCIE_PHY_CMN_REG		0x008
> +#define PCIE_PHY_MAC_RESET		0x00c
> +#define PCIE_PHY_PLL_LOCKED		0x010
> +#define PCIE_PHY_TRSVREG_RESET		0x020
> +#define PCIE_PHY_TRSV_RESET		0x024

Please use BIT() macro for bit definitions.
> +
> +/* PCIe PHY registers */
> +#define PCIE_PHY_IMPEDANCE		0x004
> +#define PCIE_PHY_PLL_DIV_0		0x008
> +#define PCIE_PHY_PLL_BIAS		0x00c
> +#define PCIE_PHY_DCC_FEEDBACK		0x014
> +#define PCIE_PHY_PLL_DIV_1		0x05c
> +#define PCIE_PHY_COMMON_POWER		0x064
> +#define PCIE_PHY_COMMON_PD_CMN		BIT(3)
> +#define PCIE_PHY_TRSV0_EMP_LVL		0x084
> +#define PCIE_PHY_TRSV0_DRV_LVL		0x088
> +#define PCIE_PHY_TRSV0_RXCDR		0x0ac
> +#define PCIE_PHY_TRSV0_POWER		0x0c4
> +#define PCIE_PHY_TRSV0_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV0_LVCC		0x0dc
> +#define PCIE_PHY_TRSV1_EMP_LVL		0x144
> +#define PCIE_PHY_TRSV1_RXCDR		0x16c
> +#define PCIE_PHY_TRSV1_POWER		0x184
> +#define PCIE_PHY_TRSV1_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV1_LVCC		0x19c
> +#define PCIE_PHY_TRSV2_EMP_LVL		0x204
> +#define PCIE_PHY_TRSV2_RXCDR		0x22c
> +#define PCIE_PHY_TRSV2_POWER		0x244
> +#define PCIE_PHY_TRSV2_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV2_LVCC		0x25c
> +#define PCIE_PHY_TRSV3_EMP_LVL		0x2c4
> +#define PCIE_PHY_TRSV3_RXCDR		0x2ec
> +#define PCIE_PHY_TRSV3_POWER		0x304
> +#define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
> +#define PCIE_PHY_TRSV3_LVCC		0x31c
> +
> +struct exynos_pcie_phy_data {
> +	struct phy_ops	*ops;
> +};
> +
> +/* For Exynos pcie phy */
> +struct exynos_pcie_phy {
> +	const struct exynos_pcie_phy_data *drv_data;
> +	void __iomem *phy_base;
> +	void __iomem *blk_base; /* For exynos5440 */
> +};
> +
> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
> +{
> +	writel(val, base + offset);
> +}
> +
> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
> +{
> +	return readl(base + offset);
> +}
> +
> +/* For Exynos5440 specific functions */
> +static int exynos5440_pcie_phy_init(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	/* DCC feedback control off */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
> +
> +	/* set TX/RX impedance */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
> +
> +	/* set 50Mhz PHY clock */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
> +
> +	/* set TX Differential output for lane 0 */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
> +
> +	/* set TX Pre-emphasis Level Control for lane 0 to minimum */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
> +
> +	/* set RX clock and data recovery bandwidth */
> +	exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
> +
> +	/* change TX Pre-emphasis Level Control for lanes */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
> +	exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
> +
> +	/* set LVCC */
> +	exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
> +	exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);

I'm starting to dis-like all this hard-coded hw params. All this should come
from dt. Define a dt binding like this for all hw params..
	phy,tx-differential = <val, reg-offset, mask>

and have one API in phy-core to do all these settings.
> +
> +	/* pulse for common reset */
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
> +	udelay(500);

how did you get this delay value? Adding a comment might help.
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val &= ~PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val &= ~PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val &= ~PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val &= ~PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val &= ~PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +	u32 val;
> +
> +	while (exynos_pcie_phy_readl(ep->phy_base,
> +				PCIE_PHY_PLL_LOCKED) == 0) {
> +		val = exynos_pcie_phy_readl(ep->blk_base,
> +				PCIE_PHY_PLL_LOCKED);
> +		dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
> +	}
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
> +	val |= PCIE_PHY_COMMON_PD_CMN;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
> +	val |= PCIE_PHY_TRSV0_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
> +	val |= PCIE_PHY_TRSV1_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
> +	val |= PCIE_PHY_TRSV2_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
> +
> +	val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
> +	val |= PCIE_PHY_TRSV3_PD_TSV;
> +	exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
> +
> +	return 0;
> +}
> +
> +static int exynos5440_pcie_phy_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
> +
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
> +	exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
> +
> +	return 0;
> +}
> +
> +static struct phy_ops exynos5440_phy_ops = {
> +	.init	= exynos5440_pcie_phy_init,
> +	.power_on = exynos5440_pcie_phy_power_on,
> +	.power_off = exynos5440_pcie_phy_power_off,
> +	.reset	= exynos5440_pcie_phy_reset,

add .owner
> +};
> +
> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
> +	.ops		= &exynos5440_phy_ops,

why do you need a wrapper for phy_ops?
> +};
> +
> +static const struct of_device_id exynos_pcie_phy_match[] = {
> +	{
> +		.compatible = "samsung,exynos5440-pcie-phy",
> +		.data = &exynos5440_pcie_phy_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
> +
> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct exynos_pcie_phy *exynos_phy;
> +	struct phy *generic_phy;
> +	struct phy_provider *phy_provider;
> +	struct resource *res;
> +	const struct exynos_pcie_phy_data *drv_data;
> +
> +	drv_data = of_device_get_match_data(dev);
> +	if (!drv_data)
> +		return -ENODEV;
> +
> +	exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
> +	if (!exynos_phy)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	exynos_phy->phy_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	exynos_phy->blk_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(exynos_phy->phy_base))
> +		return PTR_ERR(exynos_phy->phy_base);
> +
> +	exynos_phy->drv_data = drv_data;
> +
> +	generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
> +	if (IS_ERR(generic_phy)) {
> +		dev_err(dev, "failed to create PHY\n");
> +		return PTR_ERR(generic_phy);
> +	}
> +
> +	phy_set_drvdata(generic_phy, exynos_phy);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static struct platform_driver exynos_pcie_phy_driver = {
> +	.probe	= exynos_pcie_phy_probe,
> +	.driver = {
> +		.of_match_table	= exynos_pcie_phy_match,
> +		.name		= "exynos_pcie_phy",
> +	}
> +};
> +module_platform_driver(exynos_pcie_phy_driver);
> 
Thanks
Kishon

^ permalink raw reply

* Re: [PATCH V7 1/4] Documentation/devicetree/bindings: b850v3_lvds_dp
From: Peter Senna Tschudin @ 2017-01-16  8:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, Peter Senna Tschudin, Rob Herring, Mark Rutland,
	Daniel Vetter, Peter Senna Tschudin, Takashi Iwai, Yakir Yang,
	Jiri Slaby, Martyn Welch, Ian Campbell, Russell King,
	Javier Martinez Canillas, Thierry Reding, Guenter Roeck,
	martin.donnelly, devicetree@vger.kernel.org, Pawel Moll,
	Mauro Carvalho Chehab, enric.balletb
In-Reply-To: <6276161.johxDync2u@avalon>

On Tue, Jan 10, 2017 at 11:04:58PM +0200, Laurent Pinchart wrote:
> Hi Peter,

Laurent!

> 
> On Saturday 07 Jan 2017 01:29:52 Peter Senna Tschudin wrote:
> > On 04 January, 2017 21:39 CET, Rob Herring wrote:
> > > On Tue, Jan 3, 2017 at 5:34 PM, Peter Senna Tschudin wrote:
> > >> On 03 January, 2017 23:51 CET, Rob Herring <robh@kernel.org> wrote:
> > >>> On Sun, Jan 01, 2017 at 09:24:29PM +0100, Peter Senna Tschudin wrote:
> > >>>> Devicetree bindings documentation for the GE B850v3 LVDS/DP++
> > >>>> display bridge.
> > >>>> 
> > >>>> Cc: Martyn Welch <martyn.welch@collabora.co.uk>
> > >>>> Cc: Martin Donnelly <martin.donnelly@ge.com>
> > >>>> Cc: Javier Martinez Canillas <javier@dowhile0.org>
> > >>>> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > >>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > >>>> Cc: Rob Herring <robh@kernel.org>
> > >>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > >>>> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> > >>>> ---
> > >>>> There was an Acked-by from Rob Herring <robh@kernel.org> for V6, but
> > >>>> I changed the bindings to use i2c_new_secondary_device() so I
> > >>>> removed it from the commit message.
> > >>>> 
> > >>>>  .../devicetree/bindings/ge/b850v3-lvds-dp.txt      | 39 ++++++++++++++
> > >>> Generally, bindings are not organized by vendor. Put in
> > >>> bindings/display/bridge/... instead.
> > >> 
> > >> Will change that.
> > >> 
> > >>>>  1 file changed, 39 insertions(+)
> > >>>>  create mode 100644
> > >>>>  Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> > >>>> 
> > >>>> diff --git a/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> > >>>> b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt new file
> > >>>> mode 100644
> > >>>> index 0000000..1bc6ebf
> > >>>> --- /dev/null
> > >>>> +++ b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> > >>>> @@ -0,0 +1,39 @@
> > >>>> +Driver for GE B850v3 LVDS/DP++ display bridge
> > >>>> +
> > >>>> +Required properties:
> > >>>> +  - compatible : should be "ge,b850v3-lvds-dp".
> > >>> 
> > >>> Isn't '-lvds-dp' redundant? The part# should be enough.
> > >> 
> > >> b850v3 is the name of the product, this is why the proposed name. What
> > >> about, b850v3-dp2 dp2 indicating the second DP output?
> > >
> > > Humm, b850v3 is the board name? This node should be the name of the bridge
> > > chip.
> >
> > From the cover letter:
> > 
> > -- // --
> > There are two physical bridges on the video signal pipeline: a STDP4028(LVDS
> > to DP) and a STDP2690(DP to DP++).  The hardware and firmware made it
> > complicated for this binding to comprise two device tree nodes, as the
> > design goal is to configure both bridges based on the LVDS signal, which
> > leave the driver powerless to control the video processing pipeline. The
> > two bridges behaves as a single bridge, and the driver is only needed for
> > telling the host about EDID / HPD, and for giving the host powers to ack
> > interrupts. The video signal pipeline is as follows:
> > 
> >   Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> > -- // --
> 
> You forgot to prefix your patch series with [HACK] ;-)
> 
> How about fixing the issues that make the two DT nodes solution difficult ? 
> What are they ?

The Firmware and the hardware design. Both bridges, with stock firmware,
are fully capable of providig EDID information and handling interrupts.
But on this specific design, with this specific firmware, I need to read
EDID from one bridge, and handle interrupts on the other. Back when I
was starting the development I could not come up with a proper way to
split EDID and interrupts between two bridges in a way that would result
in a fully functional connector. Did I miss something?


> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

^ permalink raw reply

* [PATCH v3 1/1] iio: adc: tlc4541: add support for TI tlc4541 adc
From: Phil Reid @ 2017-01-16  8:38 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

This adds TI's tlc4541 16-bit ADC driver. Which is a single channel
ADC. Supports raw and trigger buffer access.
Also supports the tlc3541 14-bit device, which has not been tested.
Implementation of the tlc3541 is fairly straight forward thou.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---

Notes:
    Changes from v2:
    - Fix paste error in binding document.
    - Remove index from channel description. Note this break libiio.
      libiio will no longer recognise that the cahnnel is buffer enabled.
    - Add brackets to macro parameter bitshift
    - Make tcl3541 / tcl4541 order in code consistent.
    - Use spi_message_init_with_transfers
    - Cleanup whitespace lines
    - Add TLC3541 to kconfig description
    - Add Robs Ack.
    
    Changes from v1:
    - Add tlc3541 support and chan spec.
    - remove fields that where already 0 from TLC4541_V_CHAN macro
    - Increase rx_buf size in tlc4541_state to avoid copy in tlc4541_trigger_handle
    - Remove erroneous be16_to_cpu in tlc4541_trigger_handle
    - Docs/binding: spi -> SPI & add ti,tlc3541

 .../devicetree/bindings/iio/adc/ti-tlc4541.txt     |  17 ++
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-tlc4541.c                       | 271 +++++++++++++++++++++
 4 files changed, 301 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
 create mode 100644 drivers/iio/adc/ti-tlc4541.c

diff --git a/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
new file mode 100644
index 0000000..6b26927
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
@@ -0,0 +1,17 @@
+* Texas Instruments' TLC4541
+
+Required properties:
+ - compatible: Should be one of
+	* "ti,tlc4541"
+	* "ti,tlc3541"
+	- reg: SPI chip select number for the device
+ - vref-supply: The regulator supply for ADC reference voltage
+ - spi-max-frequency: Max SPI frequency to use (<= 200000)
+
+Example:
+adc@0 {
+	compatible = "ti,tlc4541";
+	reg = <0>;
+	vref-supply = <&vdd_supply>;
+	spi-max-frequency = <200000>;
+};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..fb9ede7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -525,6 +525,18 @@ config TI_AM335X_ADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called ti_am335x_adc.
 
+config TI_TLC4541
+	tristate "Texas Instruments TLC4541 ADC driver"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for Texas Instruments TLC4541 / TLC3541
+	  ADC chips.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-tlc4541.
+
 config TWL4030_MADC
 	tristate "TWL4030 MADC (Monitoring A/D Converter)"
 	depends on TWL4030_CORE
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04..9bf2377 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
 obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
+obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
 obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
 obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
 obj-$(CONFIG_VF610_ADC) += vf610_adc.o
diff --git a/drivers/iio/adc/ti-tlc4541.c b/drivers/iio/adc/ti-tlc4541.c
new file mode 100644
index 0000000..78d91a0
--- /dev/null
+++ b/drivers/iio/adc/ti-tlc4541.c
@@ -0,0 +1,271 @@
+/*
+ * TI tlc4541 ADC Driver
+ *
+ * Copyright (C) 2017 Phil Reid
+ *
+ * Datasheets can be found here:
+ * http://www.ti.com/lit/gpn/tlc3541
+ * http://www.ti.com/lit/gpn/tlc4541
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * The tlc4541 requires 24 clock cycles to start a transfer.
+ * Conversion then takes 2.94us to complete before data is ready
+ * Data is returned MSB first.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+struct tlc4541_state {
+	struct spi_device               *spi;
+	struct regulator                *reg;
+	struct spi_transfer             scan_single_xfer[3];
+	struct spi_message              scan_single_msg;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 * 2 bytes data + 6 bytes padding + 8 bytes timestamp when
+	 * call iio_push_to_buffers_with_timestamp.
+	 */
+	__be16                          rx_buf[8] ____cacheline_aligned;
+};
+
+struct tlc4541_chip_info {
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+};
+
+enum tlc4541_id {
+	TLC3541,
+	TLC4541,
+};
+
+#define TLC4541_V_CHAN(bits, bitshift) {                              \
+		.type = IIO_VOLTAGE,                                  \
+		.info_mask_separate       = BIT(IIO_CHAN_INFO_RAW),   \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+		.scan_type = {                                        \
+			.sign = 'u',                                  \
+			.realbits = (bits),                           \
+			.storagebits = 16,                            \
+			.shift = (bitshift),                          \
+			.endianness = IIO_BE,                         \
+		},                                                    \
+	}
+
+#define DECLARE_TLC4541_CHANNELS(name, bits, bitshift) \
+const struct iio_chan_spec name ## _channels[] = { \
+	TLC4541_V_CHAN(bits, bitshift), \
+	IIO_CHAN_SOFT_TIMESTAMP(1), \
+}
+
+static DECLARE_TLC4541_CHANNELS(tlc3541, 14, 2);
+static DECLARE_TLC4541_CHANNELS(tlc4541, 16, 0);
+
+static const struct tlc4541_chip_info tlc4541_chip_info[] = {
+	[TLC3541] = {
+		.channels = tlc3541_channels,
+		.num_channels = ARRAY_SIZE(tlc3541_channels),
+	},
+	[TLC4541] = {
+		.channels = tlc4541_channels,
+		.num_channels = ARRAY_SIZE(tlc4541_channels),
+	},
+};
+
+static irqreturn_t tlc4541_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tlc4541_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = spi_sync(st->spi, &st->scan_single_msg);
+	if (ret < 0)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
+					   iio_get_time_ns(indio_dev));
+
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static int tlc4541_get_range(struct tlc4541_state *st)
+{
+	int vref;
+
+	vref = regulator_get_voltage(st->reg);
+	if (vref < 0)
+		return vref;
+
+	vref /= 1000;
+
+	return vref;
+}
+
+static int tlc4541_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long m)
+{
+	int ret = 0;
+	struct tlc4541_state *st = iio_priv(indio_dev);
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+		ret = spi_sync(st->spi, &st->scan_single_msg);
+		iio_device_release_direct_mode(indio_dev);
+		if (ret < 0)
+			return ret;
+		*val = be16_to_cpu(st->rx_buf[0]);
+		*val = *val >> chan->scan_type.shift;
+		*val &= GENMASK(chan->scan_type.realbits - 1, 0);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		ret = tlc4541_get_range(st);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		*val2 = chan->scan_type.realbits;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info tlc4541_info = {
+	.read_raw = &tlc4541_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int tlc4541_probe(struct spi_device *spi)
+{
+	struct tlc4541_state *st;
+	struct iio_dev *indio_dev;
+	const struct tlc4541_chip_info *info;
+	int ret;
+	int8_t device_init = 0;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (indio_dev == NULL)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	spi_set_drvdata(spi, indio_dev);
+
+	st->spi = spi;
+
+	info = &tlc4541_chip_info[spi_get_device_id(spi)->driver_data];
+
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = info->channels;
+	indio_dev->num_channels = info->num_channels;
+	indio_dev->info = &tlc4541_info;
+
+	/* perform reset */
+	spi_write(spi, &device_init, 1);
+
+	/* Setup default message */
+	st->scan_single_xfer[0].rx_buf = &st->rx_buf[0];
+	st->scan_single_xfer[0].len = 3;
+	st->scan_single_xfer[1].delay_usecs = 3;
+	st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
+	st->scan_single_xfer[2].len = 2;
+
+	spi_message_init_with_transfers(&st->scan_single_msg,
+					st->scan_single_xfer, 3);
+
+	st->reg = devm_regulator_get(&spi->dev, "vref");
+	if (IS_ERR(st->reg))
+		return PTR_ERR(st->reg);
+
+	ret = regulator_enable(st->reg);
+	if (ret)
+		return ret;
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+			&tlc4541_trigger_handler, NULL);
+	if (ret)
+		goto error_disable_reg;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error_cleanup_buffer;
+
+	return 0;
+
+error_cleanup_buffer:
+	iio_triggered_buffer_cleanup(indio_dev);
+error_disable_reg:
+	regulator_disable(st->reg);
+
+	return ret;
+}
+
+static int tlc4541_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct tlc4541_state *st = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+	regulator_disable(st->reg);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id tlc4541_dt_ids[] = {
+	{ .compatible = "ti,tlc3541", },
+	{ .compatible = "ti,tlc4541", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, tlc4541_dt_ids);
+#endif
+
+static const struct spi_device_id tlc4541_id[] = {
+	{"tlc3541", TLC3541},
+	{"tlc4541", TLC4541},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, tlc4541_id);
+
+static struct spi_driver tlc4541_driver = {
+	.driver = {
+		.name   = "tlc4541",
+		.of_match_table = of_match_ptr(tlc4541_dt_ids),
+	},
+	.probe          = tlc4541_probe,
+	.remove         = tlc4541_remove,
+	.id_table       = tlc4541_id,
+};
+module_spi_driver(tlc4541_driver);
+
+MODULE_AUTHOR("Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>");
+MODULE_DESCRIPTION("Texas Instruments TLC4541 ADC");
+MODULE_LICENSE("GPL v2");
-- 
1.8.3.1

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

^ permalink raw reply related

* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-16  8:40 UTC (permalink / raw)
  To: Marek Vasut, Rob Herring, Mark Rutland
  Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
	Richard Weinberger, Cyrille Pitchen, Russell King,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e7fa1542-f0e2-0e45-23b3-25d6491ae40d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Sun, 15 Jan 2017 01:23:48 +0100
Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On 01/14/2017 09:29 AM, Boris Brezillon wrote:
> > On Fri, 13 Jan 2017 18:33:40 +0100
> > Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >   
> >> On 01/13/2017 05:56 PM, Boris Brezillon wrote:  
> >>> On Fri, 13 Jan 2017 17:44:12 +0100
> >>> Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>     
> >>>> On 01/13/2017 05:28 PM, Boris Brezillon wrote:    
> >>>>> On Fri, 13 Jan 2017 17:13:55 +0100
> >>>>> Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>>       
> >>>>>> On 01/13/2017 04:12 PM, Matthias Brugger wrote:      
> >>>>>>>
> >>>>>>>
> >>>>>>> On 13/01/17 15:17, Boris Brezillon wrote:        
> >>>>>>>> On Fri, 13 Jan 2017 15:13:29 +0800
> >>>>>>>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>>>>>>>        
> >>>>>>>>> Add Mediatek nor flash node.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>>>>>> ---
> >>>>>>>>>  arch/arm/boot/dts/mt2701-evb.dts |   25 +++++++++++++++++++++++++
> >>>>>>>>>  arch/arm/boot/dts/mt2701.dtsi    |   12 ++++++++++++
> >>>>>>>>>  2 files changed, 37 insertions(+)
> >>>>>>>>>
> >>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
> >>>>>>>>> b/arch/arm/boot/dts/mt2701-evb.dts
> >>>>>>>>> index 082ca88..85e5ae8 100644
> >>>>>>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
> >>>>>>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> >>>>>>>>> @@ -24,6 +24,31 @@
> >>>>>>>>>      };
> >>>>>>>>>  };
> >>>>>>>>>
> >>>>>>>>> +&nor_flash {
> >>>>>>>>> +    pinctrl-names = "default";
> >>>>>>>>> +    pinctrl-0 = <&nor_pins_default>;
> >>>>>>>>> +    status = "okay";
> >>>>>>>>> +    flash@0 {
> >>>>>>>>> +        compatible = "jedec,spi-nor";
> >>>>>>>>> +        reg = <0>;
> >>>>>>>>> +    };
> >>>>>>>>> +};
> >>>>>>>>> +
> >>>>>>>>> +&pio {
> >>>>>>>>> +    nor_pins_default: nor {
> >>>>>>>>> +        pins1 {
> >>>>>>>>> +            pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
> >>>>>>>>> +                 <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
> >>>>>>>>> +                 <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
> >>>>>>>>> +                 <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
> >>>>>>>>> +                 <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
> >>>>>>>>> +                 <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
> >>>>>>>>> +            drive-strength = <MTK_DRIVE_4mA>;
> >>>>>>>>> +            bias-pull-up;
> >>>>>>>>> +        };
> >>>>>>>>> +    };
> >>>>>>>>> +};
> >>>>>>>>> +
> >>>>>>>>>  &uart0 {
> >>>>>>>>>      status = "okay";
> >>>>>>>>>  };
> >>>>>>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
> >>>>>>>>> b/arch/arm/boot/dts/mt2701.dtsi
> >>>>>>>>> index bdf8954..1eefce4 100644
> >>>>>>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
> >>>>>>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
> >>>>>>>>> @@ -227,6 +227,18 @@
> >>>>>>>>>          status = "disabled";
> >>>>>>>>>      };
> >>>>>>>>>
> >>>>>>>>> +    nor_flash: spi@11014000 {
> >>>>>>>>> +        compatible = "mediatek,mt2701-nor",
> >>>>>>>>> +                 "mediatek,mt8173-nor";        
> >>>>>>>>
> >>>>>>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
> >>>>>>>> subset of the features supported by "mediatek,mt2701-nor"?
> >>>>>>>>        
> >>>>>>>
> >>>>>>> I think even if the ip block is the same, we should provide both
> >>>>>>> bindings, just in case in the future we find out that mt2701 has some
> >>>>>>> hidden bug, feature or bug-feature. This way even if we update the
> >>>>>>> driver, we stay compatible with older device tree blobs in the wild.
> >>>>>>>
> >>>>>>> We can drop the mt2701-nor in the bindings definition if you want.       
> >>>>>
> >>>>> Oh, sorry, I misunderstood. What I meant is that if you want to
> >>>>> list/support all possible compatibles, maybe you should just put one
> >>>>> compatible in your DT and patch your driver (+ binding doc) to define
> >>>>> all of them.      
> >>>>
> >>>> Uh, what ? I lost you here :-)  
> > 
> > I mean adding a new entry in the mtk_nor_of_ids table (in
> > mtk-quadspi.c) so that the mediatek,mt2701-nor compatible string can be
> > matched directly, and you won't need to define 2 compatible strings in
> > your device tree.  
> 
> But then you grow the table in the driver, is that what we want if we
> can avoid that ?

The space you save by not growing the mtk_nor_of_ids table is lost in
your dtbs, so I'm not sure the size argument is relevant here. Also,
note that distros are shipping a lot of dtbs, and you're likely to have
several boards based on the mt2701 SoC, so, for this specific use case,
it's better to make the in-driver of-id table grow than specifying 2
compatibles in the DT. But as I said, I'm not sure we should rely on
this argument to decide which approach to choose (we're talking about a
few bytes here).

> 
> >>>>    
> >>>>>> This exactly. We should have a DT compat in the form:
> >>>>>> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
> >>>>>> Then if we find a problem in the future, we can match on the
> >>>>>> "vendor,<soc>-block" and still support the old DTs.      
> >>>>>
> >>>>> Not sure it's only in term of whose IP appeared first. My understanding
> >>>>> is that it's a way to provide inheritance. For example:
> >>>>>
> >>>>> 	"<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
> >>>>>
> >>>>> or
> >>>>>
> >>>>> 	"<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
> >>>>>
> >>>>> BTW, which one is the oldest between mt8173 and mt2701? :-)      
> >>>>
> >>>> And that's another thing and I agree with you, but I don't think that's
> >>>> what we're discussing in this thread. But (!), OT, I think we should
> >>>> codify the rules in Documentation/ . This discussion came up multiple
> >>>> times recently.
> >>>>
> >>>> And my question still stands, what do we put into the DT here, IMO
> >>>> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";    
> >>>
> >>> I'd say
> >>>
> >>> 	compatible = "mediatek,mt8173-nor";
> >>>
> >>> because both compatible are referring to very specific IP version. It's
> >>> not the same as    
> >>
> >> But then you don't have the ability to handle a block in this particular
> >> SoC in case there's a bug found in it in the future,
> >> so IMO it should be:
> >>
> >> compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";  
> > 
> > Sorry again, I meant
> > 
> > 	compatible = "mediatek,mt2701-nor";
> >   
> >>  
> >>> 	compatible = "mediatek,mt8173-nor", "mediatek,mt81xx-nor";    
> >>
> >> This doesn't look right, since here we add two new compatibles ...  
> > 
> > That was just an example to describe how compatible inheritance works
> > (at least that's my understanding of it), it does not apply to this
> > particular use case.  
> 
> Well this is OK I guess, but then you can also use "mediatek,mt8173-nor"
> as the oldest supported compatible and be done with it, no ? It looks a
> bit crappy though, I admit that ...
> 

Let's stop bikeshedding and wait for DT maintainers feedback
before taking a decision ;-).

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

^ permalink raw reply

* Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
From: Kishon Vijay Abraham I @ 2017-01-16  8:45 UTC (permalink / raw)
  To: Vivek Gautam, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: mark.rutland-5wv7dgnIgG8, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
	bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484045519-19030-3-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> PHY transceiver driver for QUSB2 phy controller that provides
> HighSpeed functionality for DWC3 controller present on
> Qualcomm chipsets.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> 
> Changes since v3:
>  - Added 'Reviewed-by' from Stephen.
>  - Fixed debug message for qusb2_phy_set_tune2_param().
>  - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
>    since we are requesting only one reset.
>  - Updated devm_nvmem_cell_get() with a NULL cell id.
>  - Made error labels more idiomatic.
>  - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
>    base address and register offset as two separate arguments.
> 
> Changes since v2:
>  - Removed selecting 'RESET_CONTROLLER' config.
>  - Added error handling for clk_prepare_enable paths.
>  - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
>    do that since 'xo' is modeled as parent to this clock.
>  - Removed 'ref_clk_src' handling. Driver doesn't need to request and
>    handle this clock.
>  - Moved nvmem_cell_get() to probe function.
>  - Simplified phy pll status handling.
>  - Using of_device_get_match_data() to get match data.
>  - Uniformly using lowercase for hex numbers.
>  - Fixed sparse warnings.
>  - Using shorter variable names in structure and in functions.
>  - Handling various comment style shortcomings.
> 
> Changes since v1:
>  - removed reference to clk_enabled/pwr_enabled.
>  - moved clock and regulator enable code to phy_power_on/off() callbacks.
>  - fixed return on EPROBE_DEFER in qusb2_phy_probe().
>  - fixed phy create and phy register ordering.
>  - removed references to non-lkml links from commit message.
>  - took care of other minor nits.
>  - Fixed coccinelle warnings -
>    'PTR_ERR applied after initialization to constant'
>  - Addressed review comment, regarding qfprom access for tune2 param value.
>    This driver is now based on qfprom patch[1] that allows byte access now.
> 
>  drivers/phy/Kconfig          |  10 +
>  drivers/phy/Makefile         |   1 +
>  drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 550 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-qusb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f225a88..0ed53d018b23 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,16 @@ config PHY_STIH407_USB
>  	  Enable this support to enable the picoPHY device used by USB2
>  	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>  
> +config PHY_QCOM_QUSB2
> +	tristate "Qualcomm QUSB2 PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
> +	  controllers on Qualcomm chips. This driver supports the high-speed
> +	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
> +	  USB IPs on MSM SOCs.
> +
>  config PHY_QCOM_UFS
>  	tristate "Qualcomm UFS PHY driver"
>  	depends on OF && ARCH_QCOM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f436a41..dad1682b80e3 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>  obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
> +obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
> new file mode 100644
> index 000000000000..c69118610164
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qusb2.c
> @@ -0,0 +1,539 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define QUSB2PHY_PLL_TEST		0x04
> +#define CLK_REF_SEL			BIT(7)
> +
> +#define QUSB2PHY_PLL_TUNE		0x08
> +#define QUSB2PHY_PLL_USER_CTL1		0x0c
> +#define QUSB2PHY_PLL_USER_CTL2		0x10
> +#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
> +#define QUSB2PHY_PLL_PWR_CTRL		0x18
> +
> +#define QUSB2PHY_PLL_STATUS		0x38
> +#define PLL_LOCKED			BIT(5)
> +
> +#define QUSB2PHY_PORT_TUNE1		0x80
> +#define QUSB2PHY_PORT_TUNE2		0x84
> +#define QUSB2PHY_PORT_TUNE3		0x88
> +#define QUSB2PHY_PORT_TUNE4		0x8c
> +#define QUSB2PHY_PORT_TUNE5		0x90
> +#define QUSB2PHY_PORT_TEST2		0x9c
> +
> +#define QUSB2PHY_PORT_POWERDOWN		0xb4
> +#define CLAMP_N_EN			BIT(5)
> +#define FREEZIO_N			BIT(1)
> +#define POWER_DOWN			BIT(0)
> +
> +#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
> +
> +#define PHY_CLK_SCHEME_SEL		BIT(0)
> +
> +struct qusb2_phy_init_tbl {
> +	unsigned int offset;
> +	unsigned int val;
> +};
> +#define QUSB2_PHY_INIT_CFG(o, v) \
> +	{			\
> +		.offset = o,	\
> +		.val = v,	\
> +	}
> +
> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> +};

I wish all this data comes from device tree and one API in phy-core can do all
these settings. Your other driver qcom-qmp also seems to have a bunch of
similar settings.

The problem is every vnedor driver adds a bunch of code to perform the same
thing again and again when all of these settings can be done by a single phy API.

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

^ permalink raw reply

* Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy
From: Kishon Vijay Abraham I @ 2017-01-16  8:49 UTC (permalink / raw)
  To: Vivek Gautam, robh+dt, linux-kernel, devicetree
  Cc: mark.rutland, sboyd, bjorn.andersson, srinivas.kandagatla,
	linux-arm-msm
In-Reply-To: <1484045519-19030-4-git-send-email-vivek.gautam@codeaurora.org>

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> Qualcomm chipsets have QMP phy controller that provides
> support to a number of controller, viz. PCIe, UFS, and USB.
> Adding dt binding information for the same.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes since v3:
>  - Added #clock-cells = <1>, indicating that phy is a clock provider.
> 
> Changes since v2:
>  - Removed binding for "ref_clk_src" since we don't request this
>    clock in the driver.
>  - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>  - Using 'phy' for the node name.
> 
> Changes since v1:
>  - New patch, forked out of the original driver patch:
>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>  - Added 'Acked-by' from Rob.
>  - Updated bindings to include mem resource as a list of
>    offset - length pair for serdes block and for each lane.
>  - Added a new binding for 'lane-offsets' that contains offsets
>    to tx, rx and pcs blocks from each lane base address.
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> new file mode 100644
> index 000000000000..6f510fe48f46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -0,0 +1,76 @@
> +Qualcomm QMP PHY controller
> +===========================
> +
> +QMP phy controller supports physical layer functionality for a number of
> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> +
> +Required properties:
> + - compatible: compatible list, contains:
> +	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
> +	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
> + - reg: list of offset and length pair of the PHY register sets.
> +	at index 0: offset and length of register set for PHY common
> +		    serdes block.
> +	from index 1 - N: offset and length of register set for each lane,
> +			  for N number of phy lanes (ports).
> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
> + - #phy-cells: must be 1
> +    - Cell after phy phandle should be the port (lane) number.
> + - #clock-cells: must be 1
> +    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
> +      interface (for pipe based PHYs). These clock are then gate-controlled
> +      by gcc.
> + - clocks: a list of phandles and clock-specifier pairs,
> +	   one for each entry in clock-names.
> + - clock-names: must be "cfg_ahb" for phy config clock,
> +			"aux" for phy aux clock,
> +			"ref" for 19.2 MHz ref clk,
> +			"pipe<port-number>" for pipe clock specific to
> +			each port/lane (Optional).
> + - resets: a list of phandles and reset controller specifier pairs,
> +	   one for each entry in reset-names.
> + - reset-names: must be "phy" for reset of phy block,
> +			"common" for phy common block reset,
> +			"cfg" for phy's ahb cfg block reset (Optional).
> +			"port<port-number>" for reset specific to
> +			each port/lane (Optional).
> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +Optional properties:
> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
> +			pll block.
> +
> +Example:
> +	pcie_phy: phy@34000 {
> +		compatible = "qcom,msm8996-qmp-pcie-phy";
> +		reg = <0x034000 0x48f>,
> +			<0x035000 0x5bf>,
> +			<0x036000 0x5bf>,
> +			<0x037000 0x5bf>;
> +				/* tx, rx, pcs */
> +		lane-offsets = <0x0 0x200 0x400>;
> +		#phy-cells = <1>;
> +		#clock-cells = <1>;
> +
> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
> +			<&gcc GCC_PCIE_CLKREF_CLK>,
> +			<&gcc GCC_PCIE_0_PIPE_CLK>,
> +			<&gcc GCC_PCIE_1_PIPE_CLK>,
> +			<&gcc GCC_PCIE_2_PIPE_CLK>;
> +		clock-names = "aux", "cfg_ahb", "ref",
> +				"pipe0", "pipe1", "pipe2";
> +
> +		vdda-phy-supply = <&pm8994_l28>;
> +		vdda-pll-supply = <&pm8994_l12>;
> +
> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
> +			<&gcc GCC_PCIE_0_PHY_BCR>,
> +			<&gcc GCC_PCIE_1_PHY_BCR>,
> +			<&gcc GCC_PCIE_2_PHY_BCR>;
> +		reset-names = "phy", "common", "cfg",
> +				"lane0", "lane1", "lane2";

Each lane has a separate clock, separate reset.. why not create sub-nodes for
each lane?

Thanks
Kishon

^ 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