Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] dt-binding: remoteproc: Introduce ADSP loader binding
From: Rob Herring @ 2016-11-15 22:08 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Ohad Ben-Cohen, Mark Rutland,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Sarangdhar Joshi
In-Reply-To: <1478757009-11522-1-git-send-email-bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Wed, Nov 09, 2016 at 09:50:09PM -0800, Bjorn Andersson wrote:
> This document defines the binding for a component that loads firmware
> and control the life cycle of the Qualcomm ADSP Hexagon core.
> 
> Cc: Sarangdhar Joshi <spjoshi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> 
> Changes since v2:
> - Added the required "xo" clock, from Sarangdhar
> - Added smd-edge node
> - Corrected example
> 
> Changes since v1:
> - Added platform names to compatible
> 
>  .../devicetree/bindings/remoteproc/qcom,adsp.txt   | 98 ++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,adsp.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Device Tree track at Linux Plumbers 2016 -- notes, slides
From: Frank Rowand @ 2016-11-15 21:39 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Notes and slides from the Device Tree track at the Linux Plumbers 2016
conference are available at:

   http://elinux.org/Device_tree_future#Linux_Plumbers_2016_Device_Tree_Track
--
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: specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-11-15 21:39 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
	Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161115213510.GY1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>

On Tue, Nov 15, 2016 at 1:35 PM, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> On Tue, Nov 15, 2016 at 12:27:53PM -0800, Tim Harvey wrote:
>> On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
>> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
>> > So, someone merged a patch which makes mmcblk devices follow the
>> > host controller numbering.
>> >
>> > Now my cubox-i fails to boot correctly because the SD card in the
>> > _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
>> >
>> > USDHC1 is wired to the on-microsom WiFi, and never has anything
>> > remotely near a SD card or eMMC present.  So, this change is
>> > confusing on these platforms.
>> >
>> > Moreover, this is _going_ to break SolidRun distros if people upgrade
>> > their kernels.
>> >
>> > It may be appropriate for eMMC, but it's not appropriate everywhere.
>> >
>> > This is a user visible _regression_ in 4.9-rc.  Whoever did this,
>> > please revert whatever change caused this, and next time limit it
>> > to only eMMC.
>> >
>> > Thanks.
>>
>> I see the same thing on newer kernels, which is why I asked the
>> question. I didn't expect (or even want honestly) a non mmcblk0 boot
>> device and was looking for a way to control that via dt. Now I'm
>> understanding that to avoid this kind of bootloader/kernel dependence
>> issue I should be using UUID's to identify the boot device.
>>
>> >From my testing it looks like the change your looking for occurred
>> some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
>> regression specifically.
>
> That depends how you look at it.  Yes, there's a change in 4.5 to 4.6
> which ties the block device number to the host device index, but that's
> really only part of the story here.
>
> 4.8 definitely identifies the SD card in iMX6 usdhc2 as "mmcblk0".
> 4.9-rc identifies the SD card as "mmcblk1".  This makes it a 4.9 change
> of behaviour - there can be no argument about that.
>
> Now, digging further into this today, it appears that:
>
> v4.8: usdhc2 was probed first, and is given mmc0.
>       usdhc1 is probed second, and is given mmc1.
>
> v4.9-rc: usdhc1 is probed first, and is given mmc0.
>          usdhc2 is probed second, and is given mmc1.
>
> I haven't yet been able to figure out why there's been this change
> of probe order.  There's no change that I can see in the iMX6 DT
> files that would account for this.
>

I bisected it and the commit your looking for is
9aaf3437aa72ed5370bf32c99580a3fa2c330e3d

Tim
--
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: specifying order of /dev/mmcblk devices via device-tree?
From: Russell King - ARM Linux @ 2016-11-15 21:35 UTC (permalink / raw)
  To: Tim Harvey
  Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
	Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAJ+vNU38XQBHt709S_z6Quvce6wcqLzRHFuN_bVZ_0eARa3=fw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Tue, Nov 15, 2016 at 12:27:53PM -0800, Tim Harvey wrote:
> On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
> <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> > So, someone merged a patch which makes mmcblk devices follow the
> > host controller numbering.
> >
> > Now my cubox-i fails to boot correctly because the SD card in the
> > _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
> >
> > USDHC1 is wired to the on-microsom WiFi, and never has anything
> > remotely near a SD card or eMMC present.  So, this change is
> > confusing on these platforms.
> >
> > Moreover, this is _going_ to break SolidRun distros if people upgrade
> > their kernels.
> >
> > It may be appropriate for eMMC, but it's not appropriate everywhere.
> >
> > This is a user visible _regression_ in 4.9-rc.  Whoever did this,
> > please revert whatever change caused this, and next time limit it
> > to only eMMC.
> >
> > Thanks.
> 
> I see the same thing on newer kernels, which is why I asked the
> question. I didn't expect (or even want honestly) a non mmcblk0 boot
> device and was looking for a way to control that via dt. Now I'm
> understanding that to avoid this kind of bootloader/kernel dependence
> issue I should be using UUID's to identify the boot device.
> 
> >From my testing it looks like the change your looking for occurred
> some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
> regression specifically.

That depends how you look at it.  Yes, there's a change in 4.5 to 4.6
which ties the block device number to the host device index, but that's
really only part of the story here.

4.8 definitely identifies the SD card in iMX6 usdhc2 as "mmcblk0".
4.9-rc identifies the SD card as "mmcblk1".  This makes it a 4.9 change
of behaviour - there can be no argument about that.

Now, digging further into this today, it appears that:

v4.8: usdhc2 was probed first, and is given mmc0.
      usdhc1 is probed second, and is given mmc1.

v4.9-rc: usdhc1 is probed first, and is given mmc0.
         usdhc2 is probed second, and is given mmc1.

I haven't yet been able to figure out why there's been this change
of probe order.  There's no change that I can see in the iMX6 DT
files that would account for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
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 4/4] ARM: dts: da850: Add the usb otg device nodeg
From: Bin Liu @ 2016-11-15 21:19 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Alexandre Bailon, khilman-rdvid1DuHRBWk0Htik3J/w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <0c94c5e0-9d2c-fff5-1213-2fb2adb20ab0-l0cyMroinI0@public.gmane.org>

On Tue, Nov 15, 2016 at 04:16:02PM +0530, Sekhar Nori wrote:
> On Thursday 03 November 2016 09:29 PM, Alexandre Bailon wrote:
> > This adds the device tree node for the usb otg
> > controller present in the da850 family of SoC's.
> > This also enables the otg usb controller for the lcdk board.
> > 
> > Signed-off-by: Alexandre Bailon <abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/da850-lcdk.dts |  8 ++++++++
> >  arch/arm/boot/dts/da850.dtsi     | 15 +++++++++++++++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
> > index 7b8ab21..9f5040c 100644
> > --- a/arch/arm/boot/dts/da850-lcdk.dts
> > +++ b/arch/arm/boot/dts/da850-lcdk.dts
> > @@ -158,6 +158,14 @@
> >  	rx-num-evt = <32>;
> >  };
> >  
> > +&usb_phy {
> > +	status = "okay";
> > +	};
> 
> As mentioned by David already, this node needs to be removed. Please
> rebase this on top of latest linux-davinci/master when ready for merging
> (driver changes accepted).

Dropped this patch due to this comment.

Regards,
-Bin.

> 
> > +
> > +&usb0 {
> > +	status = "okay";
> > +};
> > +
> >  &aemif {
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&nand_pins>;
> > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> > index f79e1b9..322a31a 100644
> > --- a/arch/arm/boot/dts/da850.dtsi
> > +++ b/arch/arm/boot/dts/da850.dtsi
> > @@ -372,6 +372,21 @@
> >  					>;
> >  			status = "disabled";
> >  		};
> > +		usb_phy: usb-phy {
> > +			compatible = "ti,da830-usb-phy";
> > +			#phy-cells = <1>;
> > +			status = "disabled";
> > +		};
> > +		usb0: usb@200000 {
> > +			compatible = "ti,da830-musb";
> > +			reg = <0x200000 0x10000>;
> > +			interrupts = <58>;
> > +			interrupt-names = "mc";
> > +			dr_mode = "otg";
> > +			phys = <&usb_phy 0>;
> > +			phy-names = "usb-phy";
> > +			status = "disabled";
> > +		};
> 
> Can you separate out the soc specific changes from board changes? Please
> place the usb0 node above the mdio node. I am trying to get to a rough
> ordering based on reg property.
> 
> Thanks,
> Sekhar
> 
--
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 0/4] Add DT support for DA8xx
From: Bin Liu @ 2016-11-15 21:12 UTC (permalink / raw)
  To: Alexandre Bailon
  Cc: khilman-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	nsekhar-l0cyMroinI0
In-Reply-To: <1478188752-22447-1-git-send-email-abailon-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

On Thu, Nov 03, 2016 at 04:59:08PM +0100, Alexandre Bailon wrote:
> Changes in v2:
> * Remove unrelated changes in patch 3
> * Rename the device node in patch 4
> 
> Changes in v3:
> * Fix few mistakes in DT binding sample
> * Only build the device table if DT is enabled
> 
> Change in v4:
> * Fix a nit
> 
> Alexandre Bailon (1):
>   ARM: dts: da850: Add the usb otg device node
> 
> Petr Kulhavy (3):
>   dt/bindings: Add binding for the DA8xx MUSB driver
>   usb: musb: core: added helper function for parsing DT
>   usb: musb: da8xx: Add DT support for the DA8xx driver

Applied. Thanks.
-Bin.

> 
>  .../devicetree/bindings/usb/da8xx-usb.txt          | 43 ++++++++++++++++++++
>  arch/arm/boot/dts/da850-lcdk.dts                   |  8 ++++
>  arch/arm/boot/dts/da850.dtsi                       | 15 +++++++
>  drivers/usb/musb/da8xx.c                           | 46 ++++++++++++++++++++++
>  drivers/usb/musb/musb_core.c                       | 19 +++++++++
>  drivers/usb/musb/musb_core.h                       |  6 +++
>  6 files changed, 137 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/da8xx-usb.txt
> 
> -- 
> 2.7.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Marek Vasut @ 2016-11-15 20:52 UTC (permalink / raw)
  To: Shawn Lin, Rob Herring, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478855766-151673-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On 11/11/2016 10:16 AM, Shawn Lin wrote:
> Add rockchip serial flash controller driver
> 
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>


[...]

> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 4a682ee..48c5e0e 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -65,6 +65,13 @@ config SPI_HISI_SFC
>  	help
>  	  This enables support for hisilicon SPI-NOR flash controller.
>  
> +config SPI_ROCKCHIP_SFC

Keep this list sorted please.

> +	tristate "Rockchip Serial Flash Controller(SFC)"
> +	depends on ARCH_ROCKCHIP || COMPILE_TEST
> +	depends on HAS_IOMEM && HAS_DMA
> +	help
> +	  This enables support for rockchip serial flash controller.
> +
>  config SPI_NXP_SPIFI
>  	tristate "NXP SPI Flash Interface (SPIFI)"
>  	depends on OF && (ARCH_LPC18XX || COMPILE_TEST)


[...]

> +/* Interrypt mask */

Interrupt :)

> +#define SFC_IMR				0x4
> +#define  SFC_IMR_RX_FULL		BIT(0)
> +#define  SFC_IMR_RX_UFLOW		BIT(1)
> +#define  SFC_IMR_TX_OFLOW		BIT(2)
> +#define  SFC_IMR_TX_EMPTY		BIT(3)
> +#define  SFC_IMR_TRAN_FINISH		BIT(4)
> +#define  SFC_IMR_BUS_ERR		BIT(5)
> +#define  SFC_IMR_NSPI_ERR		BIT(6)
> +#define  SFC_IMR_DMA			BIT(7)


[...]

> +enum rockchip_sfc_iftype {
> +	IF_TYPE_STD,
> +	IF_TYPE_DUAL,
> +	IF_TYPE_QUAD,
> +};
> +
> +struct rockchip_sfc {
> +	struct device *dev;
> +	struct mutex lock;
> +	void __iomem *regbase;
> +	struct clk *hclk;
> +	struct clk *clk;
> +	void *buffer;
> +	dma_addr_t dma_buffer;

The naming (buffer) could use some improvement or comment for clarification.

> +	struct completion cp;
> +	struct spi_nor	*nor[SFC_MAX_CHIP_NUM];

Should be MAX_CHIPSELECT_NUM , for clarity.

> +	u32 num_chip;

u8

> +	bool use_dma;
> +	bool negative_edge;

Negative edge ... of what ?

> +};
> +
> +struct rockchip_sfc_priv {
> +	u32 cs;

Doesn't this board support only 4 CS ? Use u8 :-)

> +	u32 clk_rate;
> +	struct rockchip_sfc *sfc;
> +};
> +
> +static int get_if_type(enum read_mode flash_read)
> +{
> +	enum rockchip_sfc_iftype if_type;
> +
> +	switch (flash_read) {
> +	case SPI_NOR_DUAL:
> +		if_type = IF_TYPE_DUAL;
> +		break;
> +	case SPI_NOR_QUAD:
> +		if_type = IF_TYPE_QUAD;
> +		break;
> +	case SPI_NOR_NORMAL:
> +	case SPI_NOR_FAST:
> +	default:

Should the default case really fall back to 1-bit mode or should it
rather report error ?

> +		if_type = IF_TYPE_STD;
> +		break;
> +	}
> +
> +	return if_type;
> +}
> +
> +static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
> +{
> +	unsigned long timeout = jiffies + HZ;
> +	int err = -ETIMEDOUT;
> +	u32 status;
> +
> +	writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
> +
> +	while (time_before(jiffies, timeout)) {

Would readl_poll_*() from include/linux/iopoll.h help here ?

> +		status = readl_relaxed(sfc->regbase + SFC_RCVR);
> +		if (!(status & SFC_RCVR_RESET)) {
> +			err = 0;
> +			break;
> +		}
> +		msleep(20);
> +	}
> +
> +	if (err)
> +		dev_err(sfc->dev, "SFC reset never finished\n");

Should the writel() below be executed if an error happened ?

> +	writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> +		       SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> +		       SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> +		       SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> +		       sfc->regbase + SFC_ICLR);
> +	return err;
> +}
> +
> +static int rockchip_sfc_init(struct rockchip_sfc *sfc)
> +{
> +	int err;
> +
> +	err = rockchip_sfc_reset(sfc);
> +	if (err)
> +		return err;
> +
> +	/* Mask all eight interrupts */
> +	writel_relaxed(0xff, sfc->regbase + SFC_IMR);
> +	/* Phase configure */

What phase ? Please clarify the comment. Also, don't you have to
configure the register if sfc->negative_edge == 0 too ?

> +	if (sfc->negative_edge)
> +		writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE <<
> +			       SFC_CTRL_PHASE_SEL_SHIFT,
> +			       sfc->regbase + SFC_CTRL);
> +	return 0;
> +}
> +
> +static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	int ret;
> +
> +	mutex_lock(&sfc->lock);
> +
> +	ret = clk_set_rate(sfc->clk, priv->clk_rate);
> +	if (ret)
> +		goto out;
> +
> +	ret = clk_prepare_enable(sfc->clk);
> +	if (ret)
> +		goto out;
> +
> +	return 0;
> +
> +out:
> +	mutex_unlock(&sfc->lock);
> +	return ret;
> +}
> +
> +static void rockchip_sfc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +
> +	clk_disable_unprepare(sfc->clk);
> +	mutex_unlock(&sfc->lock);
> +}
> +
> +static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc)
> +{
> +	unsigned long timeout = jiffies + 2 * HZ;
> +	int err = -ETIMEDOUT;
> +	u32 status;
> +
> +	/*
> +	 * Note: tx and rx share the same fifo, so the rx's water level
> +	 * is the same as rx's, which means this function could be reused
> +	 * for checking the read operations as well.
> +	 */
> +	while (time_before(jiffies, timeout)) {

readl_poll_*() ?

> +		status = readl_relaxed(sfc->regbase + SFC_FSR);
> +		if (((status >> SFC_FSR_TX_EMPTY_SHIFT) &
> +		     SFC_FSR_TX_EMPTY_MASK) == SFC_FSR_TX_IS_EMPTY) {
> +			err = 0;
> +			break;
> +		}
> +		msleep(20);
> +	}
> +
> +	if (err)
> +		dev_err(sfc->dev, "SFC tx never empty\n");
> +
> +	return err;
> +}
> +
> +static int rockchip_sfc_op_reg(struct spi_nor *nor,
> +				u8 opcode, int len, u8 optype)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	u32 reg;
> +
> +	if (((readl_relaxed(sfc->regbase + SFC_FSR) >> SFC_FSR_TX_EMPTY_SHIFT) &
> +	      SFC_FSR_TX_EMPTY_MASK) != SFC_FSR_TX_IS_EMPTY ||
> +	     ((readl_relaxed(sfc->regbase + SFC_FSR) >>
> +	       SFC_FSR_RX_EMPTY_SHIFT) &
> +	      SFC_FSR_RX_EMPTY_MASK) != SFC_FSR_RX_IS_EMPTY ||
> +	     (readl_relaxed(sfc->regbase + SFC_SR) == SFC_SR_IS_BUSY))
> +		rockchip_sfc_reset(sfc);

This is chaos, please fix this condition so it's actually readable. You
can ie. read the FSR into a variable, do your shifting/anding magic and
then do if (var1 || var2 || var3) {} .

> +	reg = (opcode & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT;
> +	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> +	reg |= (optype & SFC_CMD_DIR_MASK) << SFC_CMD_DIR_SHIFT;
> +
> +	writel_relaxed(reg, sfc->regbase + SFC_CMD);
> +
> +	return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode,
> +				 u8 *buf, int len)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	int ret;
> +	u32 tmp;
> +	u32 i;
> +
> +	ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD);
> +	if (ret)
> +		return ret;
> +
> +	while (len > 0) {
> +		tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> +		for (i = 0; i < len; i++)
> +			*buf++ = (u8)((tmp >> (i * 8)) & 0xff);

Won't this fail for len > 4 ?

Also, you can use ioread32_rep() here, but (!) that won't work for
unaligned reads, which I dunno if they can happen here, but please do
double-check.

> +		len = len - 4;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode,
> +				  u8 *buf, int len)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	u32 words, i;
> +
> +	/* Align bytes to words */
> +	words = (len + 3) >> 2;
> +
> +	for (i = 0; i < words; i++)
> +		writel_relaxed(*(buf + 4 * i), sfc->regbase + SFC_DATA);

See above about the ioread32_rep()/iowrite32_rep(), but careful about
unaligned (len % 4 != 0) case.

> +	return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR);
> +}
> +
> +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to,
> +				     dma_addr_t dma_buf, size_t len, u8 op_type)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	u32 reg;
> +	u8 if_type = 0;
> +
> +	init_completion(&sfc->cp);
> +
> +	writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> +		       SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> +		       SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> +		       SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> +		       sfc->regbase + SFC_ICLR);
> +
> +	/* Enable transfer complete interrupt */
> +	reg = readl_relaxed(sfc->regbase + SFC_IMR);
> +	reg &= ~SFC_IMR_TRAN_FINISH;
> +	writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> +	if (op_type == SFC_CMD_DIR_WR)
> +		reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
> +		      ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT);
> +	else
> +		reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
> +		      ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT);

reg = nor->read_opcode & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT;
reg |= op_type << SFC_CMD_DIR_SHIFT;

> +	reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
> +		: SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;

Why don't you just define those SFC_CMD_ADDR_24BITS and co. with the
shift in those bitfields already ? Then you wouldn't have to riddle this
driver with FOO << BAR, but you'd only have FOO all over the place.

> +	if_type = get_if_type(nor->flash_read);
> +	writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
> +		       if_type << SFC_CTRL_ADDR_BITS_SHIFT |
> +		       if_type << SFC_CTRL_CMD_BITS_SHIFT |

Parenthesis missing around the statements ,
(if_type << FOO) | (... << bar)

> +		       sfc->negative_edge ?
> +		       SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
> +		       SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
> +		       sfc->regbase + SFC_CTRL);
> +
> +	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> +	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +
> +	if (op_type == SFC_CMD_DIR_RD)
> +		reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
> +			SFC_CMD_DUMMY_SHIFT;

Just define SFC_CMD_DUMMY(x) \
 (((x) & SFC_CMD_DUMMY_MASK) << SFC_CMD_DUMMY_SHIFT)

And then use it ... reg |= SFC_CMD_DUMMY(nor->read_dummy);

> +	/* Should minus one as 0x0 means 1 bit flash address */
> +	writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
> +	writel_relaxed(reg, sfc->regbase + SFC_CMD);
> +	writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
> +	writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR);

I hope the DMA buffer management is implemented correctly and you're not
running into any weird cache issues.

> +	/* Start dma */
> +	writel_relaxed(0x1, sfc->regbase + SFC_DMA_TRIGGER);
> +
> +	/* Wait for the interrupt. */
> +	if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
> +		dev_err(sfc->dev, "DMA wait for transfer finish timeout.");
> +		return -ETIMEDOUT;
> +	}
> +
> +	/* Disable transfer finish interrupt */
> +	reg = readl_relaxed(sfc->regbase + SFC_IMR);
> +	reg |= SFC_IMR_TRAN_FINISH;
> +	writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> +	return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_write(struct rockchip_sfc *sfc, u_char *buf,
> +					 size_t len)
> +{
> +	u32 words, tx_wl, count, i;
> +	unsigned long timeout;
> +	int ret = 0;
> +	u32 *tbuf = (u32 *)buf;
> +
> +	/* Align bytes to words */
> +	words = (len + 3) >> 2;
> +
> +	while (words) {

See iowrite32_rep() above, but I suspect you'll run into problems with
$len which is not multiple of 4 .

> +		tx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_TX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_TX_WATER_LVL_MASK;
> +
> +		if (tx_wl > 0) {
> +			count = min_t(u32, words, tx_wl);
> +			for (i = 0; i < count; i++) {
> +				writel_relaxed(*tbuf++,
> +					       sfc->regbase + SFC_DATA);
> +				words--;
> +			}
> +
> +			if (words == 0)
> +				break;
> +			timeout = 0;
> +		} else {
> +			mdelay(1);
> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (ret)
> +		return ret;
> +	else
> +		return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
> +					size_t len)
> +{
> +	u32 words, rx_wl, count, i;
> +	unsigned long timeout;
> +	int ret = 0;
> +	u32 tmp;
> +	u32 *tbuf = (u32 *)buf;
> +	u_char *tbuf2;
> +
> +	words = len >> 2;
> +	/* Get the remained bytes */
> +	len = len & 0x3;

See above.

> +	while (words) {
> +		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_RX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_RX_WATER_LVL_MASK;
> +
> +		if (rx_wl > 0) {
> +			count = min_t(u32, words, rx_wl);
> +			for (i = 0; i < count; i++) {
> +				*tbuf++ = readl_relaxed(sfc->regbase +
> +							SFC_DATA);
> +				words--;
> +			}
> +
> +			if (words == 0)
> +				break;
> +			timeout = 0;
> +		} else {
> +			mdelay(1);
> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	/* Read the remained bytes */
> +	timeout = 0;
> +	tbuf2 = (u_char *)tbuf;
> +	while (len) {
> +		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_RX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_RX_WATER_LVL_MASK;
> +		if (rx_wl > 0) {
> +			tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> +			for (i = 0; i < len; i++)
> +				tbuf2[i] = (u8)((tmp >> (i * 8)) & 0xff);
> +			goto done;
> +		} else {
> +			mdelay(1);
> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}
> +done:
> +	if (ret)
> +		return ret;
> +	else
> +		return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to,
> +				     size_t len, u_char *buf, u8 op_type)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	u32 reg;
> +	u8 if_type = 0;
> +
> +	if (op_type == SFC_CMD_DIR_WR)
> +		reg = (SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT) |
> +		      ((nor->program_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT);
> +	else
> +		reg = (SFC_CMD_DIR_RD << SFC_CMD_DIR_SHIFT) |
> +		      ((nor->read_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT);

See above regarding this condition. I think you can factor out this
common code too. Also nuke the bitshifts , see my comments on
rockchip_sfc_dma_transfer .

> +	reg |= ((nor->addr_width == 4) ? SFC_CMD_ADDR_32BITS
> +		: SFC_CMD_ADDR_24BITS) << SFC_CMD_ADDR_SHIFT;
> +
> +	if_type = get_if_type(nor->flash_read);
> +	writel_relaxed(if_type << SFC_CTRL_DATA_BITS_SHIFT |
> +		       if_type << SFC_CTRL_ADDR_BITS_SHIFT |
> +		       if_type << SFC_CTRL_CMD_BITS_SHIFT |
> +		       sfc->negative_edge ?
> +		       SFC_CTRL_PHASE_SEL_NEGETIVE << SFC_CTRL_PHASE_SEL_SHIFT :
> +		       SFC_CTRL_PHASE_SEL_POSITIVE << SFC_CTRL_PHASE_SEL_SHIFT,
> +		       sfc->regbase + SFC_CTRL);
> +
> +	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> +	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +
> +	if (op_type == SFC_CMD_DIR_RD)
> +		reg |= (nor->read_dummy & SFC_CMD_DUMMY_MASK) <<
> +			SFC_CMD_DUMMY_SHIFT;
> +
> +	/* Should minus one as 0x0 means 1 bit flash address */
> +	writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
> +	writel_relaxed(reg, sfc->regbase + SFC_CMD);
> +	writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
> +
> +	if (op_type == SFC_CMD_DIR_WR)
> +		return rockchip_sfc_pio_write(sfc, buf, len);
> +	else
> +		return rockchip_sfc_pio_read(sfc, buf, len);
> +}
> +
> +static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from, size_t len,
> +				 u_char *read_buf)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	size_t offset;
> +	int ret;
> +	dma_addr_t dma_addr = 0;
> +
> +	if (!sfc->use_dma)
> +		goto no_dma;
> +
> +	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> +		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> +		dma_addr = dma_map_single(NULL, (void *)read_buf,
> +					  trans, DMA_FROM_DEVICE);
> +		if (dma_mapping_error(sfc->dev, dma_addr))
> +			dma_addr = 0;
> +
> +		/* Fail to map dma, use pre-allocated area instead */
> +		ret = rockchip_sfc_dma_transfer(nor, from + offset,
> +						dma_addr ? dma_addr :
> +						sfc->dma_buffer,
> +						trans, SFC_CMD_DIR_RD);
> +		if (ret) {
> +			dev_warn(nor->dev, "DMA read timeout\n");
> +			return ret;
> +		}
> +		if (!dma_addr)
> +			memcpy(read_buf + offset, sfc->buffer, trans);
> +	}
> +
> +	return len;
> +
> +no_dma:
> +	ret = rockchip_sfc_pio_transfer(nor, from, len,
> +					read_buf, SFC_CMD_DIR_RD);
> +	if (ret) {
> +		dev_warn(nor->dev, "PIO read timeout\n");
> +		return ret;
> +	}
> +	return len;
> +}
> +
> +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
> +				  size_t len, const u_char *write_buf)
> +{
> +	struct rockchip_sfc_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	size_t offset;
> +	int ret;
> +	dma_addr_t dma_addr = 0;
> +
> +	if (!sfc->use_dma)
> +		goto no_dma;

Seems like there's a lot of similarity between read/write .

> +	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> +		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> +		dma_addr = dma_map_single(NULL, (void *)write_buf,
> +					  trans, DMA_TO_DEVICE);
> +		if (dma_mapping_error(sfc->dev, dma_addr)) {
> +			dma_addr = 0;
> +			memcpy(sfc->buffer, write_buf + offset, trans);
> +		}
> +
> +		/* Fail to map dma, use pre-allocated area instead */
> +		ret = rockchip_sfc_dma_transfer(nor, to + offset,
> +						dma_addr ? dma_addr :
> +						sfc->dma_buffer,
> +						trans, SFC_CMD_DIR_WR);
> +		if (dma_addr)
> +			dma_unmap_single(NULL, dma_addr,
> +					 trans, DMA_TO_DEVICE);
> +		if (ret) {
> +			dev_warn(nor->dev, "DMA write timeout\n");
> +			return ret;
> +		}
> +	}
> +
> +	return len;
> +no_dma:
> +	ret = rockchip_sfc_pio_transfer(nor, to, len,
> +					(u_char *)write_buf, SFC_CMD_DIR_WR);
> +	if (ret) {
> +		dev_warn(nor->dev, "PIO write timeout\n");
> +		return ret;
> +	}
> +	return len;
> +}
> +
> +/**
> + * Get spi flash device information and register it as a mtd device.
> + */
> +static int rockchip_sfc_register(struct device_node *np,
> +				 struct rockchip_sfc *sfc)
> +{
> +	struct device *dev = sfc->dev;
> +	struct spi_nor *nor;
> +	struct rockchip_sfc_priv *priv;
> +	struct mtd_info *mtd;
> +	int ret;
> +
> +	nor = devm_kzalloc(dev, sizeof(*nor), GFP_KERNEL);
> +	if (!nor)
> +		return -ENOMEM;

You can embed struct spi_nor in struct rockchip_sfc_priv and drop this
allocation . Also it'd be a good idea to rename rockchip_sfc_priv to
something like rockchip_sfc_chip_priv to make it explicit this is a
per-chip private data -- which you can even pre-allocate in rockchi_sfc
structure as a static array of (four) such structures (see cadence qspi
driver for how this is done there).

> +	nor->dev = dev;
> +	spi_nor_set_flash_node(nor, np);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(np, "reg", &priv->cs);
> +	if (ret) {
> +		dev_err(dev, "No reg property for %s\n",
> +			np->full_name);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "spi-max-frequency",
> +			&priv->clk_rate);
> +	if (ret) {
> +		dev_err(dev, "No spi-max-frequency property for %s\n",
> +			np->full_name);
> +		return ret;
> +	}
> +
> +	priv->sfc = sfc;
> +	nor->priv = priv;
> +
> +	nor->prepare = rockchip_sfc_prep;
> +	nor->unprepare = rockchip_sfc_unprep;
> +	nor->read_reg = rockchip_sfc_read_reg;
> +	nor->write_reg = rockchip_sfc_write_reg;
> +	nor->read = rockchip_sfc_read;
> +	nor->write = rockchip_sfc_write;
> +	nor->erase = NULL;
> +	ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> +	if (ret)
> +		return ret;
> +
> +	mtd = &nor->mtd;
> +	mtd->name = np->name;
> +	ret = mtd_device_register(mtd, NULL, 0);
> +	if (ret)
> +		return ret;
> +
> +	sfc->nor[sfc->num_chip] = nor;
> +	sfc->num_chip++;
> +	return 0;
> +}
> +
> +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc)
> +{
> +	int i;
> +
> +	for (i = 0; i < sfc->num_chip; i++)
> +		mtd_device_unregister(&sfc->nor[i]->mtd);
> +}
> +
> +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc)
> +{
> +	struct device *dev = sfc->dev;
> +	struct device_node *np;
> +	int ret;
> +
> +	for_each_available_child_of_node(dev->of_node, np) {
> +		ret = rockchip_sfc_register(np, sfc);
> +		if (ret)
> +			goto fail;
> +
> +		if (sfc->num_chip == SFC_MAX_CHIP_NUM) {
> +			dev_warn(dev, "Exceeds the max cs limitation\n");
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +
> +fail:
> +	dev_err(dev, "Failed to register all chip\n");
> +	rockchip_sfc_unregister_all(sfc);

See cadence qspi where we only unregister the registered flashes.
Implement it the same way here.

> +	return ret;
> +}
> +
> +static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
> +{
> +	struct rockchip_sfc *sfc = dev_id;
> +	u32 reg;
> +
> +	reg = readl_relaxed(sfc->regbase + SFC_RISR);
> +	dev_dbg(sfc->dev, "Get irq: 0x%x\n", reg);
> +
> +	/* Clear interrupt */
> +	writel_relaxed(reg, sfc->regbase + SFC_ICLR);
> +
> +	if (reg & SFC_IRQ_TRAN_FINISH)
> +		complete(&sfc->cp);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rockchip_sfc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct rockchip_sfc *sfc;
> +	int ret;
> +
> +	sfc = devm_kzalloc(dev, sizeof(*sfc), GFP_KERNEL);
> +	if (!sfc)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, sfc);
> +	sfc->dev = dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	sfc->regbase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(sfc->regbase))
> +		return PTR_ERR(sfc->regbase);
> +
> +	sfc->clk = devm_clk_get(&pdev->dev, "sfc");
> +	if (IS_ERR(sfc->clk)) {
> +		dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
> +		return PTR_ERR(sfc->clk);
> +	}
> +
> +	sfc->hclk = devm_clk_get(&pdev->dev, "hsfc");
> +	if (IS_ERR(sfc->hclk)) {
> +		dev_err(&pdev->dev, "Failed to get sfc ahp clk\n");
> +		return PTR_ERR(sfc->hclk);
> +	}
> +
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_warn(dev, "Unable to set dma mask\n");
> +		return ret;
> +	}
> +
> +	sfc->buffer = dmam_alloc_coherent(dev, SFC_DMA_MAX_LEN,
> +			&sfc->dma_buffer, GFP_KERNEL);
> +	if (!sfc->buffer)
> +		return -ENOMEM;
> +
> +	mutex_init(&sfc->lock);
> +
> +	ret = clk_prepare_enable(sfc->hclk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable hclk\n");
> +		goto err_hclk;
> +	}
> +
> +	ret = clk_prepare_enable(sfc->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable clk\n");
> +		goto err_clk;
> +	}
> +
> +	if (of_property_read_bool(sfc->dev->of_node, "rockchip,sfc-no-dma"))
> +		sfc->use_dma = false;
> +	else
> +		sfc->use_dma = true;

sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
                                      "rockchip,sfc-no-dma");

> +	if (of_device_is_compatible(sfc->dev->of_node,
> +				    "rockchip,rk1108-sfc"))
> +		sfc->negative_edge = true;
> +	else
> +		sfc->negative_edge = false;

See above

> +	/* Find the irq */
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get the irq\n");
> +		goto err_irq;
> +	}
> +
> +	ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
> +			       0, pdev->name, sfc);
> +	if (ret) {
> +		dev_err(dev, "Failed to request irq\n");
> +		goto err_irq;
> +	}
> +
> +	ret = rockchip_sfc_init(sfc);
> +	if (ret)
> +		goto err_init;
> +
> +	ret = rockchip_sfc_register_all(sfc);
> +	if (ret)
> +		goto err_init;
> +
> +	clk_disable_unprepare(sfc->clk);
> +	return 0;
> +
> +err_irq:
> +err_init:

Drop the err_irq: label unless you plan to handle the error (which you
should).

> +	clk_disable_unprepare(sfc->clk);
> +err_clk:
> +	clk_disable_unprepare(sfc->hclk);
> +err_hclk:
> +	mutex_destroy(&sfc->lock);
> +	return ret;
> +}
> +
> +static int rockchip_sfc_remove(struct platform_device *pdev)
> +{
> +	struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
> +
> +	rockchip_sfc_unregister_all(sfc);
> +	mutex_destroy(&sfc->lock);
> +	clk_disable_unprepare(sfc->clk);
> +	clk_disable_unprepare(sfc->hclk);
> +	return 0;
> +}
> +
> +static const struct of_device_id rockchip_sfc_dt_ids[] = {
> +	{ .compatible = "rockchip,sfc"},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
> +
> +static struct platform_driver rockchip_sfc_driver = {
> +	.driver = {
> +		.name	= "rockchip-sfc",
> +		.of_match_table = rockchip_sfc_dt_ids,
> +	},
> +	.probe	= rockchip_sfc_probe,
> +	.remove	= rockchip_sfc_remove,
> +};
> +module_platform_driver(rockchip_sfc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");

MODULE_AUTHOR is missing



-- 
Best regards,
Marek Vasut
--
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 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers
From: Jyri Sarha @ 2016-11-15 20:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-devicetree, Benoit Cousson, Kevin Hilman, linux-drm,
	Tomi Valkeinen, Laurent Pinchart
In-Reply-To: <CAMpxmJVcMa-wfXf1BUXvkZ78v_yWorEka1mkGd3wZ88oyL58Kw@mail.gmail.com>

On 11/15/16 19:36, Bartosz Golaszewski wrote:
> 2016-11-14 17:54 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
>> Adds drm bride support for attaching drm bridge drivers to tilcdc. The
>> decision whether a video port leads to an external encoder or bridge
>> is made simply based on remote device's compatible string. The code
>> has been tested with BeagleBone-Black with and without BeagleBone
>> DVI-D Cape Rev A3 using ti-tfp410 driver.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
> 
> Hi Jyri,
> 
> thanks a lot for doing this.
> 
> One issue I see with this patch is that tilcdc doesn't seem to support
> deferred probe correctly (if modules are built-in). The following
> happens on my setup:
> 
> The dump-vga-dac module is loaded first, but the i2c0 is not ready yet
> - probe returns EPROBE_DEFER and it's propagated to tilcdc probe.
> 
>     [drm] Initialized
>     dumb-vga-dac vga_bridge: Couldn't retrieve i2c bus
> 
> Then the i2c bus is initialized and dump-vga-dac probe succeeds, but
> the second probe of tilcdc gives me:
> 
>     [drm:drm_debugfs_init] *ERROR* Cannot create /sys/kernel/debug/dri/64
>     [drm:drm_minor_register] *ERROR* DRM: Failed to initialize
> /sys/kernel/debug/dri.
>     tilcdc: probe of da8xx_lcdc.0 failed with error -1
> 
> I was able to work around this issue by loading modules in correct order.
> 

Did you have any conflicts when applying my patch? I have done quite a
few changes lately and especially the initialization sequence and back
off from deferred probe may get broken easily broken if the source base
is not correct. I try to come up with a pull-request candidate branch
soon (hopefully tomorrow) for you to test.

> I then tried testing the patch with a da850-lcdk, but I don't get
> anything on the display (no signal), even though the LCDC seems to
> work fine (modetest and dmesg messages work just like when using the
> tilcdc panel). Also: I see the EDID info is correctly retrieved from
> the display.
> 
> Could you take a look at my DT[1] and see if you find it correct?
> 

It is hard to follow the dts diff, but if it probes and tilcdc is able
to read EDID modes, there should not be anything more to it.

Cheers,
Jyri

> Best regards,
> Bartosz Golaszewski
> 
> [1] http://pastebin.com/dfUX7PyL
> 

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

^ permalink raw reply

* Re: specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-11-15 20:27 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Ulf Hansson, Mark Rutland, Javier Martinez Canillas,
	Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161114190839.GS1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>

On Mon, Nov 14, 2016 at 11:08 AM, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> So, someone merged a patch which makes mmcblk devices follow the
> host controller numbering.
>
> Now my cubox-i fails to boot correctly because the SD card in the
> _only_ SD card slot now gets called "mmcblk1" and not "mmcblk0".
>
> USDHC1 is wired to the on-microsom WiFi, and never has anything
> remotely near a SD card or eMMC present.  So, this change is
> confusing on these platforms.
>
> Moreover, this is _going_ to break SolidRun distros if people upgrade
> their kernels.
>
> It may be appropriate for eMMC, but it's not appropriate everywhere.
>
> This is a user visible _regression_ in 4.9-rc.  Whoever did this,
> please revert whatever change caused this, and next time limit it
> to only eMMC.
>
> Thanks.

I see the same thing on newer kernels, which is why I asked the
question. I didn't expect (or even want honestly) a non mmcblk0 boot
device and was looking for a way to control that via dt. Now I'm
understanding that to avoid this kind of bootloader/kernel dependence
issue I should be using UUID's to identify the boot device.

>From my testing it looks like the change your looking for occurred
some time ago and is somewhere between 4.5 and 4.6 and not a 4.9
regression specifically.

Regards,

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

^ permalink raw reply

* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Rob Herring @ 2016-11-15 20:10 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: devicetree@vger.kernel.org, Laurent Pinchart, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
	linux-media@vger.kernel.org
In-Reply-To: <4350940.43ZWQivOUU@avalon>

On Mon, Nov 14, 2016 at 8:11 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
>> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
>> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>>>> Multiple incompatible data link layers have been used over time to
>> >>>>> transmit image data to LVDS panels. This binding supports display
>> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>>>> specifications.

[...]

>> >>> Furthermore, LVDS data organization is controlled by the combination of
>> >>> both data-mapping and data-mirror. It makes little sense from my point
>> >>> of view to handle one as part of the compatible string and the other one
>> >>> as a separate property.
>> >>>
>> >>>> > +Optional properties:
>> >>>> > +- label: a symbolic name for the panel
>> >>>>
>> >>>> Could be for any panel or display connector.
>> >>>
>> >>> Yes, but I'm not sure to understand how that's relevant :-)
>> >>
>> >> Meaning it should be a common property.
>> >
>> > Sure. So you expect me to reorganize all the panels and connectors DT
>> > bindings in order to get this one merged ? :-)
>>
>> No, because I don't think label is widely defined. Just put it in a
>> common place and reference it. Any other panels can be fixed later.
>> Really, the "simple panel" binding should probably morph into the
>> common binding.
>
> The "label" property is actually defined in the "Devicetree Specification,

Yes, so is reg, interrupts, etc. but we still list when those are used.

> Release 0.1" as recently published on devicetree.org. Where would you like me
> to define it in the bindings ?

Just split things into 2 files. Move everything that's used in other
panel bindings (label, width-mm, ports, etc.) to a panel/common.txt.
Then in lvds-panel.txt, it just refers to common.txt and is just the
LVDS specific things. Bonus points if simple-panel (should just go
away), panel-dpi, panel-dsi-cm are converted.

>> >>>>> +- avdd-supply: reference to the regulator that powers the panel
>> >>>>> analog supply
>> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>>>> digital supply

I would not be against these being common either. It's somewhat better
than simple-panel's "power-supply" property.

>> >>>>
>> >>>> Which one has to be powered on first, what voltage, and with what time
>> >>>> in between? This is why "generic" or "simple" bindings don't work.
>> >>>
>> >>> The above-mentioned specifications also define connectors, pinouts and
>> >>> power supplies, but many LVDS panels compatible with the LVDS physical
>> >>> and data layers use a different connector with small differences in
>> >>> power supplies.
>> >>>
>> >>> I believe the voltage is irrelevant here, it doesn't need to be
>> >>> controlled by the operating system. Power supplies order and timing is
>> >>> relevant, I'll investigate the level of differences between panels. I'm
>> >>> also fine with dropping those properties for now.
>> >>
>> >> Whether you have control of the supplies is dependent on the board.
>> >> Dropping them is just puts us in the simple binding trap. The simple
>> >> bindings start out that way and then people keep adding to them.
>> >
>> > Damn, you can't be fooled easily ;-)
>>
>> I guess you can count all the simple bindings to see how many times I
>> can be fooled. :)
>>
>> > On a more serious note, I'd like to design the bindings in a way that
>> > wouldn't require adding device-specific code in the driver for each panel
>> > model, given that in most cases power supply handling will be generic.
>> > What's your opinion about a generic power supply model that would be used
>> > in the default case, with the option to override it with device-specific
>> > code when needed ?
>>
>> I don't agree. Read Thierry's post on the subject[1].
>
> I'm not sure you understood me correctly (and writing a clarification at
> 4:00am might not help :-)). My point here is that a fair number of panels
> don't care about power sequencing and have no control GPIOs (that's certainly
> the case of the two Mitsubishi panels I use here that have a single power
> supply - good luck trying to convince me that this needs to be sequenced :-) -
> and no enable or reset control pin). This kind of panel should obviously be
> modelled in DT with both a specific and a generic ("panel-lvds") compatible
> string, to ensure that we'll have enough information available on the
> operating system side to control the panel properly. The power supply(ies)
> should be documented in relation to the specific compatible string and not
> mentioned by the generic bindings (whether that should go in one or multiple
> text files is bikeshedding).

Let me rephrase, I don't agree with generic bindings, but I fully
support having a generic driver. So you can have a generic driver
function that can handle your case. Even slightly more complicated can
be handled: turn on all the supplies listed, de-assert any reset
gpios, assert any enable gpios (this is why I push for common naming
of "enable-gpios"). If this is enough for a panel, then the generic
compatible will work. If not, sorry, provide your own power sequencing
code and match on the more specific compatible.

> This being said, on the driver side, I don't see a reason to list the specific
> compatible strings explicitly for those panels when a generic implementation
> matching the generic compatible string can handle them fine. This has no
> impact on the bindings and is an Linux implementation decision.

I think we're in agreement.

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

^ permalink raw reply

* [PATCH v2] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 19:59 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Christian Lamparter, linux-kernel, devicetree, linux-usb,
	linuxppc-dev, Mark Rutland, Rob Herring, Greg Kroah-Hartman,
	Felipe Balbi, John Youn

From: Christian Lamparter <chunkeey@googlemail.com>

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:

commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands
board")

but without any driver support as the dwc2 driver wasn't available at
that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---

v2 [johnyoun]:
* Removed params struct
* Minor commit message formatting

 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 drivers/usb/dwc2/params.c                      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index ad8f7ff..6c7c2bce 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 513556a..7991c21 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
 	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
 	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
 	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
+	{ .compatible = "amcc,dwc-otg", .data = &params_amcc_dwc_otg },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.0

^ permalink raw reply related

* Re: [PATCH 0/3] usb: dwc2: params revert and rework
From: John Youn @ 2016-11-15 19:37 UTC (permalink / raw)
  To: Felipe Balbi, John Youn, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <87polxugfo.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 11/15/2016 3:33 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
>> Hi Felipe,
>>
>> This reverts and fixes a few commits that are queued on your
>> testing/next, removing the previously added DT bindings, and the code
>> that reads them in.
>>
>> The feedback was that IP validation is not reason enough to add these.
>> So we'll leave them out for now.
> 
> these are still in testing/next, so I can actually still drop
> them. I'm applying only patch 3/3, please make sure it all looks good on
> testing/next.
> 

Ok thanks. There were merge issues when I did that locally so I
figured I'd give you the reverts.

Checked and looks good.

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

^ permalink raw reply

* Re: [PATCH v7 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Stephen Boyd @ 2016-11-15 19:27 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: adrian.hunter, ulf.hansson, linux-mmc, shawn.lin, andy.gross,
	devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
	alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd, kdorfman,
	david.griego, stummala, venkatg, rnayak, pramod.gurav
In-Reply-To: <3c1a7c72-0ac1-8ed0-87fc-238331f0645b@codeaurora.org>

On 11/15, Ritesh Harjani wrote:
> On 11/15/2016 1:07 AM, Stephen Boyd wrote:
> >On 11/14, Ritesh Harjani wrote:
> >
> >>+}
> >>+
> >>+/* sdhci_msm_set_clock - Called with (host->lock) spinlock held. */
> >>+static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> >>+{
> >>+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> >>+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> >>+	int rc;
> >>+
> >>+	if (!clock) {
> >>+		msm_host->clk_rate = clock;
> >>+		goto out;
> >>+	}
> >>+
> >>+	spin_unlock_irq(&host->lock);
> >>+	if (clock != msm_host->clk_rate) {
> >
> >Why do we need to check here? Can't we call clk_set_rate()
> >Unconditionally?
> Since it may so happen that above layers may call for ->set_clock
> function with same requested clock more than once, hence we cache
> the host->clock here.
> Also, since requested clock (host->clock) can be say 400Mhz but the
> actual pltfm supported clock would be say 384MHz.

clk_set_rate() detects the same rate being set even after it
internally rounds the rate. We're not going to touch the clk
hardware if 400 is requested once but 384 is what's set and then
400 is requested again. Caching the rate here in the driver can
lead to problems too if the driver is out of sync with the clk
hardware state, so it's best to avoid doing anything fancy here
and just let the framework handle duplicates.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support
From: John Youn @ 2016-11-15 19:08 UTC (permalink / raw)
  To: Christian Lamparter, John Youn
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Mark Rutland, Rob Herring, Greg Kroah-Hartman, Felipe Balbi
In-Reply-To: <f9be1906-3d94-e9c6-841a-d7baa2572f75-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On 11/14/2016 3:00 PM, John Youn wrote:
> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>
>>>>>> The device definition was added by:
>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>> available at that time.
>>>>>>
>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>> because of the special ahbcfg configuration. The default
>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> I don't want to add any more of these param structures to the driver
>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>> of using auto-detected defaults and device properties to override
>>>>> them.
>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>
>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>> thinking of making the default INCR.
>>>> Is that actually possible to change the default still? This would
>>>> require to re-evaluate all existing archs/platforms that use 
>>>> "snps,dwc2" for INCR16 compatibility. 
>>>
>>> INCR, not INCR16, but you're right, so we may not change it even
>>> though though INCR is usually the right choice over INCR4.
>> What about making a device-tree property?
> 
> Yes, that's what I meant. I'll send a change for this shortly.
> 
>>
>> Recommended properties:
>>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>>    the safer but inefficient "INCR4" is used. The optimal setting is
>>    "INCRx".
>>
>> Would this work? If so, I can make a patch over the weekend.
>>> Anyways, with the binding, can't you just set the compatible string to
>>> snps,dwc2?
>>
>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>> a while back about device-tree bindings.
>>
>> They made it very clear to me, that they don't want any generic "catch all
>> compatible" strings:
>>
>> "Bindings should be for hardware (either specific device models, or for
>> classes), and not for Linux drivers. The latter is subject to arbitrary
>> changes while the former is not, as old hardware continues to exist and
>> does not change while drivers get completely reworked." [0]
>>
>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>> and this binding can't be easily changed. Rob Herring explained this in
>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>> to make them work with the changes I made:
>>
>> "You can't remove the old drivers as they are needed to work with 
>> old dtbs, so there is no gain.
>>
>> You would need to match on existing compatibles such as
>> moxa,moxart-gpio and provide a match data struct that has all the info
>> you are adding here (e.g. data register offset). Then additionally you
>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>> additional data associated with it. But it has to be new properties,
>> not changing properties. Changing the reg values doesn't work."
>>
>> So, for this to work with the existing canyonlands.dts, I need to have
>> the "amcc,dwc-otg" compatible string.
> 
> Ok, if that's the case. But still a bit confused as to what driver was
> working with it before since the binding was not defined for dwc2.
> 
>>
>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>> about this case.
>>
>> Regards,
>> Christian
>>
>> [0] <https://patchwork.kernel.org/patch/8976221/>
>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>
>>  
>>>>
>>>> From what I can tell based would be:
>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>
>>>>> If that's all you need then a devicetree binding should be enough
>>>>> right?
>>>> Yes. The device is working fine so far.
>>>>
>>>> Regards,
>>>> Christian
>>>>
>>>> ---
>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>
>>>> This patch adds support for the "amcc,usb-otg" device
>>>> which is found in the PowerPC Canyonlands' dts.
>>>>
>>>> The device definition was added by:
>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>> but without any driver support as the dwc2 driver wasn't
>>>> available at that time.
>>>>
>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>> because of the special ahbcfg configuration. The default
>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>> when the USB and SATA is used concurrently.
>>>>
>>>> Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>> Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> v1->v2:
>>>> 	- moved definitons to params.c
>>>> 	- removed dma_enable / host_dma parameter
>>>> 	- added dma_desc_fs_enable parameter
>>>> v2->v3:
>>>> 	- removed parameters
>>>>
>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>> for ahbcfg.
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 10a2a4b..6ccfe85 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> @@ -12,6 +12,7 @@ Required properties:
>>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>>  - reg : Should contain 1 register range (address and length)
>>>>  - interrupts : Should contain 1 interrupt
>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>> index 64d5c66..9506ab0 100644
>>>> --- a/drivers/usb/dwc2/params.c
>>>> +++ b/drivers/usb/dwc2/params.c
>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>>  	{},
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>
> 
> For dwc2 part:
> 
> Acked-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> 

Hi Felipe,

Can you drop this from your testing/next?

I meant for the 2nd version to be applied, without the params
structure.

I can send you a clean version to apply later today.

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

^ permalink raw reply

* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Stephen Boyd @ 2016-11-15 18:56 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rob Herring, Mark Brown, Rafael Wysocki, nm, Viresh Kumar,
	linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, d-gerlach,
	devicetree
In-Reply-To: <20161115033151.GG4178@vireshk-i7>

On 11/15, Viresh Kumar wrote:
> On 14-11-16, 18:13, Stephen Boyd wrote:
> > On 11/14, Rob Herring wrote:
> > > On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> > > > On 10-11-16, 14:51, Stephen Boyd wrote:
> > > > > 
> > > > > No. The supply names (and also clock names/index) should be left
> > > > > up to the consumer of the OPP table. We don't want to encode any
> > > > > sort of details like this between the OPP table and the consumer
> > > > > of it in DT because then it seriously couples the OPP table to
> > > > > the consumer device. "The binding" in this case that needs to be
> > > > > updated is the consumer binding, to indicate that it correlated
> > > > > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > > > > voltages.
> > > > 
> > > > Are you saying that we shall have a property like this then?
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > > > index ee91cbdd95ee..733946df2fb8 100644
> > > > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > > > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > > > @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
> > > >                         compatible = "arm,cortex-a7";
> > > >                         ...
> > > >  
> > > > -                       cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > > > +                       vcc0-supply = <&cpu_supply0>;
> > > > +                       vcc1-supply = <&cpu_supply1>;
> > > > +                       vcc2-supply = <&cpu_supply2>;
> > > > +                       opp-supply-names = "vcc0", "vcc1", "vcc2";
> > > 
> > > Uh, no. You already have the names in the *-supply properties. Yes, they 
> > > are a PIA to retrieve compared to a *-names property, but that is the 
> > > nature of this style of binding.
> 
> Its not just PIA, but impossible AFAICT.
> 
> There are two important pieces of information we need for multiple
> regulator support:
> - Which regulator in the consumer node corresponds to which entry in
>   the OPP table. As Mark mentioned earlier, DT should be able to get
>   us this.

This is also possible from C code though. Or is there some case
where it isn't possible if we're sharing the same table with two
devices? I'm lost on when this would ever happen.

It feels like trying to keep the OPP table agnostic of the
consuming device and the device's binding is more trouble than
it's worth. Especially considering we have opp-shared and *-name
now.

> - The order in which the supplies need to be programmed. We have all
>   agreed to do this in code instead of inferring it from DT and this
>   patch series already does that.

Agreed. Encoding a sequence into DT doesn't sound very feasible.
How is this going to be handled though? I don't see any users of
the code we're reviewing here, so it's hard to grasp how things
will work. It would be really useful if we had some user of the
code included in the patch series to get the big picture.

> 
> I want to solve the first problem here and I don't see how it can be
> solved using such entries:
> 
> 	cpus {
> 		cpu@0 {
> 			compatible = "arm,cortex-a7";
> 			...
> 
>                         vcc0-supply = <&cpu_supply0>;
>                         vcc1-supply = <&cpu_supply1>;
>                         vcc2-supply = <&cpu_supply2>;
> 			operating-points-v2 = <&cpu0_opp_table>;
>                 };
>         };
> 
> 	cpu0_opp_table: opp_table0 {
> 		compatible = "operating-points-v2";
> 		opp-shared;
> 
> 		opp@1000000000 {
> 			opp-hz = /bits/ 64 <1000000000>;
> 			opp-microvolt = <970000>, /* Supply 0 */
> 					<960000>, /* Supply 1 */
> 					<960000>; /* Supply 2 */
> 		};
>         };
> 
> The code can't figure out which of vcc0, vcc1, vcc2 is added first in
> the CPU node and so we need to get the order somehow. A separate
> binding as I mentioned earlier is a probably (ugly) solution.
> 
> > I think the problem is that Viresh wants the binding to be "self
> > describing" so that the OPP can be used without a driver knowing
> > that a supply corresponds to a particular column in the voltage
> > table.
> 
> Right, and that's what Mark suggested as well.
> 
> > I don't understand that though. Can't we set the supply
> > names from C code somewhere based on the consumer of the OPPs?
> 
> That's what this patch series is doing right now.
> 
> So, are you saying that the way this patchset does it is fine with you
> ?

That's just to handle the ordering of operations? I need to take
a minute and understand what's changing. You may have spent
plenty of time developing/updating, but I haven't spent near
enough time understanding what's going on in these patches to
give a thorough review.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH V2 1/2] pinctrl: tegra: Add DT binding for io pads control
From: Jon Hunter @ 2016-11-15 18:48 UTC (permalink / raw)
  To: Laxman Dewangan, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w
  Cc: gnurou-Re5JQEeQqe8AvxtiuMwx3w,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478696782-11657-2-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>


On 09/11/16 13:06, Laxman Dewangan wrote:
> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
> sources. When IO interfaces are not used then IO pads can be
> configure in low power state to reduce the power consumption from
> that IO pads.
> 
> On Tegra124, the voltage level of IO power rail source is auto
> detected by hardware(SoC) and hence it is only require to configure
> in low power mode if IO pads are not used.
> 
> On T210 onwards, the auto-detection of voltage level from IO power
> rail is removed from SoC and hence SW need to configure the PMC
> register explicitly to set proper voltage in IO pads based on
> IO rail power source voltage.
> 
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> ---
> Changes from V1:
>  The DT binding document is modified to explain the regulator handle
>  for different IOs and how can it be passed from the DT.
> ---
>  .../bindings/pinctrl/nvidia,tegra-io-pad.txt       | 126 +++++++++++++++++++++
>  1 file changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> new file mode 100644
> index 0000000..6ca961f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> @@ -0,0 +1,126 @@
> +NVIDIA Tegra PMC IO pad controller
> +
> +NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> +low power state of some of its IO pads. When IO interface are not
> +used then IO pads can be configure in low power state to reduce
> +the power from that IO pads. The IO pads can work in the voltage
> +of the 1.8V and 3.3V of IO voltage from power rail sources.

The last sentence is a bit unclear and does not sound correct. I am not
sure if you are missing the word 'range' somewhere or if you are trying
to say it must be either 1.8V or 3.3V. Looks like you have the same
sentence on the changelog too.

Cheers
Jon

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

^ permalink raw reply

* [PATCH v4 3/3] ASoC: atmel: tse850: add ASoC driver for the Axentia TSE-850
From: Peter Rosin @ 2016-11-15 18:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	devicetree
In-Reply-To: <1479235095-13441-1-git-send-email-peda@axentia.se>

The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 MAINTAINERS                      |   1 +
 sound/soc/atmel/Kconfig          |  10 +
 sound/soc/atmel/Makefile         |   2 +
 sound/soc/atmel/tse850-pcm5142.c | 472 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 485 insertions(+)
 create mode 100644 sound/soc/atmel/tse850-pcm5142.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e6bf00e53bf3..59baed3fd0ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2323,6 +2323,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/sound/axentia,*
+F:	sound/soc/atmel/tse850-pcm5142.c
 
 AZ6007 DVB DRIVER
 M:	Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
index 22aec9a1e9a4..4a56f3dfba51 100644
--- a/sound/soc/atmel/Kconfig
+++ b/sound/soc/atmel/Kconfig
@@ -78,4 +78,14 @@ config SND_ATMEL_SOC_PDMIC
 	help
 	  Say Y if you want to add support for Atmel ASoC driver for boards using
 	  PDMIC.
+
+config SND_ATMEL_SOC_TSE850_PCM5142
+	tristate "ASoC driver for the Axentia TSE-850"
+	depends on ARCH_AT91 && OF
+	depends on ATMEL_SSC && I2C
+	select SND_ATMEL_SOC_SSC_DMA
+	select SND_SOC_PCM512x_I2C
+	help
+	  Say Y if you want to add support for the ASoC driver for the
+	  Axentia TSE-850 with a PCM5142 codec.
 endif
diff --git a/sound/soc/atmel/Makefile b/sound/soc/atmel/Makefile
index a2b127bd9c87..67e10cbd4ed7 100644
--- a/sound/soc/atmel/Makefile
+++ b/sound/soc/atmel/Makefile
@@ -13,9 +13,11 @@ snd-atmel-soc-wm8904-objs := atmel_wm8904.o
 snd-soc-sam9x5-wm8731-objs := sam9x5_wm8731.o
 snd-atmel-soc-classd-objs := atmel-classd.o
 snd-atmel-soc-pdmic-objs := atmel-pdmic.o
+snd-atmel-soc-tse850-pcm5142-objs := tse850-pcm5142.o
 
 obj-$(CONFIG_SND_AT91_SOC_SAM9G20_WM8731) += snd-soc-sam9g20-wm8731.o
 obj-$(CONFIG_SND_ATMEL_SOC_WM8904) += snd-atmel-soc-wm8904.o
 obj-$(CONFIG_SND_AT91_SOC_SAM9X5_WM8731) += snd-soc-sam9x5-wm8731.o
 obj-$(CONFIG_SND_ATMEL_SOC_CLASSD) += snd-atmel-soc-classd.o
 obj-$(CONFIG_SND_ATMEL_SOC_PDMIC) += snd-atmel-soc-pdmic.o
+obj-$(CONFIG_SND_ATMEL_SOC_TSE850_PCM5142) += snd-atmel-soc-tse850-pcm5142.o
diff --git a/sound/soc/atmel/tse850-pcm5142.c b/sound/soc/atmel/tse850-pcm5142.c
new file mode 100644
index 000000000000..ac6a814c8ecf
--- /dev/null
+++ b/sound/soc/atmel/tse850-pcm5142.c
@@ -0,0 +1,472 @@
+/*
+ * TSE-850 audio - ASoC driver for the Axentia TSE-850 with a PCM5142 codec
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * 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.
+ */
+
+/*
+ *               loop1 relays
+ *   IN1 +---o  +------------+  o---+ OUT1
+ *            \                /
+ *             +              +
+ *             |   /          |
+ *             +--o  +--.     |
+ *             |  add   |     |
+ *             |        V     |
+ *             |      .---.   |
+ *   DAC +----------->|Sum|---+
+ *             |      '---'   |
+ *             |              |
+ *             +              +
+ *
+ *   IN2 +---o--+------------+--o---+ OUT2
+ *               loop2 relays
+ *
+ * The 'loop1' gpio pin controlls two relays, which are either in loop
+ * position, meaning that input and output are directly connected, or
+ * they are in mixer position, meaning that the signal is passed through
+ * the 'Sum' mixer. Similarly for 'loop2'.
+ *
+ * In the above, the 'loop1' relays are inactive, thus feeding IN1 to the
+ * mixer (if 'add' is active) and feeding the mixer output to OUT1. The
+ * 'loop2' relays are active, short-cutting the TSE-850 from channel 2.
+ * IN1, IN2, OUT1 and OUT2 are TSE-850 connectors and DAC is the PCB name
+ * of the (filtered) output from the PCM5142 codec.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+
+#include <sound/soc.h>
+#include <sound/pcm_params.h>
+
+#include "atmel_ssc_dai.h"
+
+struct tse850_priv {
+	int ssc_id;
+
+	struct gpio_desc *add;
+	struct gpio_desc *loop1;
+	struct gpio_desc *loop2;
+
+	struct regulator *ana;
+
+	int add_cache;
+	int loop1_cache;
+	int loop2_cache;
+};
+
+static int tse850_get_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->loop1_cache;
+
+	return 0;
+}
+
+static int tse850_put_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(tse850->loop1, val);
+	tse850->loop1_cache = val;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static int tse850_get_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->loop2_cache;
+
+	return 0;
+}
+
+static int tse850_put_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(tse850->loop2, val);
+	tse850->loop2_cache = val;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+int tse850_get_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->add_cache;
+
+	return 0;
+}
+
+int tse850_put_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int connect = !!ucontrol->value.integer.value[0];
+
+	if (tse850->add_cache == connect)
+		return 0;
+
+	/*
+	 * Hmmm, this gpiod_set_value_cansleep call should probably happen
+	 * inside snd_soc_dapm_mixer_update_power in the loop.
+	 */
+	gpiod_set_value_cansleep(tse850->add, connect);
+	tse850->add_cache = connect;
+
+	snd_soc_dapm_mixer_update_power(dapm, kctrl, connect, NULL);
+	return 1;
+}
+
+int tse850_get_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = regulator_get_voltage(tse850->ana);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Map regulator output values like so:
+	 *      -11.5V to "Low" (enum 0)
+	 * 11.5V-12.5V to "12V" (enum 1)
+	 * 12.5V-13.5V to "13V" (enum 2)
+	 *     ...
+	 * 18.5V-19.5V to "19V" (enum 8)
+	 * 19.5V-      to "20V" (enum 9)
+	 */
+	if (ret < 11000000)
+		ret = 11000000;
+	else if (ret > 20000000)
+		ret = 20000000;
+	ret -= 11000000;
+	ret = (ret + 500000) / 1000000;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+int tse850_put_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int uV = ucontrol->value.enumerated.item[0];
+	int ret;
+
+	if (uV >= e->items)
+		return -EINVAL;
+
+	/*
+	 * Map enum zero (Low) to 2 volts on the regulator, do this since
+	 * the ana regulator is supplied by the system 12V voltage and
+	 * requesting anything below the system voltage causes the system
+	 * voltage to be passed through the regulator. Also, the ana
+	 * regulator induces noise when requesting voltages near the
+	 * system voltage. So, by mapping Low to 2V, that noise is
+	 * eliminated when all that is needed is 12V (the system voltage).
+	 */
+	if (uV)
+		uV = 11000000 + (1000000 * uV);
+	else
+		uV = 2000000;
+
+	ret = regulator_set_voltage(tse850->ana, uV, uV);
+	if (ret < 0)
+		return ret;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static const char * const mux_text[] = { "Mixer", "Loop" };
+
+static const struct soc_enum mux_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 2, mux_text);
+
+static const struct snd_kcontrol_new mux1 =
+	SOC_DAPM_ENUM_EXT("MUX1", mux_enum, tse850_get_mux1, tse850_put_mux1);
+
+static const struct snd_kcontrol_new mux2 =
+	SOC_DAPM_ENUM_EXT("MUX2", mux_enum, tse850_get_mux2, tse850_put_mux2);
+
+#define TSE850_DAPM_SINGLE_EXT(xname, reg, shift, max, invert, xget, xput) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.get = xget, \
+	.put = xput, \
+	.private_value = SOC_SINGLE_VALUE(reg, shift, max, invert, 0) }
+
+static const struct snd_kcontrol_new mix[] = {
+	TSE850_DAPM_SINGLE_EXT("IN Switch", SND_SOC_NOPM, 0, 1, 0,
+			       tse850_get_mix, tse850_put_mix),
+};
+
+static const char * const ana_text[] = {
+	"Low", "12V", "13V", "14V", "15V", "16V", "17V", "18V", "19V", "20V"
+};
+
+static const struct soc_enum ana_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 9, ana_text);
+
+static const struct snd_kcontrol_new out =
+	SOC_DAPM_ENUM_EXT("ANA", ana_enum, tse850_get_ana, tse850_put_ana);
+
+static const struct snd_soc_dapm_widget tse850_dapm_widgets[] = {
+	SND_SOC_DAPM_LINE("OUT1", NULL),
+	SND_SOC_DAPM_LINE("OUT2", NULL),
+	SND_SOC_DAPM_LINE("IN1", NULL),
+	SND_SOC_DAPM_LINE("IN2", NULL),
+	SND_SOC_DAPM_INPUT("DAC"),
+	SND_SOC_DAPM_AIF_IN("AIFINL", "Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIFINR", "Playback", 1, SND_SOC_NOPM, 0, 0),
+	SOC_MIXER_ARRAY("MIX", SND_SOC_NOPM, 0, 0, mix),
+	SND_SOC_DAPM_MUX("MUX1", SND_SOC_NOPM, 0, 0, &mux1),
+	SND_SOC_DAPM_MUX("MUX2", SND_SOC_NOPM, 0, 0, &mux2),
+	SND_SOC_DAPM_OUT_DRV("OUT", SND_SOC_NOPM, 0, 0, &out, 1),
+};
+
+/*
+ * These connections are not entirely correct, since both IN1 and IN2
+ * are always fed to MIX (if the "IN switch" is set so), i.e. without
+ * regard to the loop1 and loop2 relays that according to this only
+ * control MUX1 and MUX2 but in fact also control how the input signals
+ * are routed.
+ * But, 1) I don't know how to do it right, and 2) it doesn't seem to
+ * matter in practice since nothing is powered in those sections anyway.
+ */
+static const struct snd_soc_dapm_route tse850_intercon[] = {
+	{ "OUT1", NULL, "MUX1" },
+	{ "OUT2", NULL, "MUX2" },
+
+	{ "MUX1", "Loop",  "IN1" },
+	{ "MUX1", "Mixer", "OUT" },
+
+	{ "MUX2", "Loop",  "IN2" },
+	{ "MUX2", "Mixer", "OUT" },
+
+	{ "OUT", NULL, "MIX" },
+
+	{ "MIX", NULL, "DAC" },
+	{ "MIX", "IN Switch", "IN1" },
+	{ "MIX", "IN Switch", "IN2" },
+
+	/* connect board input to the codec left channel output pin */
+	{ "DAC", NULL, "OUTL" },
+};
+
+static struct snd_soc_dai_link tse850_dailink = {
+	.name = "TSE-850",
+	.stream_name = "TSE-850-PCM",
+	.codec_dai_name = "pcm512x-hifi",
+	.dai_fmt = SND_SOC_DAIFMT_I2S
+		 | SND_SOC_DAIFMT_NB_NF
+		 | SND_SOC_DAIFMT_CBM_CFS,
+};
+
+static struct snd_soc_card tse850_card = {
+	.name = "TSE-850-ASoC",
+	.owner = THIS_MODULE,
+	.dai_link = &tse850_dailink,
+	.num_links = 1,
+	.dapm_widgets = tse850_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tse850_dapm_widgets),
+	.dapm_routes = tse850_intercon,
+	.num_dapm_routes = ARRAY_SIZE(tse850_intercon),
+	.fully_routed = true,
+};
+
+static int tse850_dt_init(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *codec_np, *cpu_np;
+	struct snd_soc_card *card = &tse850_card;
+	struct snd_soc_dai_link *dailink = &tse850_dailink;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	if (!np) {
+		dev_err(&pdev->dev, "only device tree supported\n");
+		return -EINVAL;
+	}
+
+	cpu_np = of_parse_phandle(np, "axentia,ssc-controller", 0);
+	if (!cpu_np) {
+		dev_err(&pdev->dev, "failed to get dai and pcm info\n");
+		return -EINVAL;
+	}
+	dailink->cpu_of_node = cpu_np;
+	dailink->platform_of_node = cpu_np;
+	tse850->ssc_id = of_alias_get_id(cpu_np, "ssc");
+	of_node_put(cpu_np);
+
+	codec_np = of_parse_phandle(np, "axentia,audio-codec", 0);
+	if (!codec_np) {
+		dev_err(&pdev->dev, "failed to get codec info\n");
+		return -EINVAL;
+	}
+	dailink->codec_of_node = codec_np;
+	of_node_put(codec_np);
+
+	return 0;
+}
+
+static int tse850_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &tse850_card;
+	struct device *dev = card->dev = &pdev->dev;
+	struct tse850_priv *tse850;
+	int ret;
+
+	tse850 = devm_kzalloc(dev, sizeof(*tse850), GFP_KERNEL);
+	if (!tse850)
+		return -ENOMEM;
+
+	snd_soc_card_set_drvdata(card, tse850);
+
+	ret = tse850_dt_init(pdev);
+	if (ret) {
+		dev_err(dev, "failed to init dt info\n");
+		return ret;
+	}
+
+	tse850->add = devm_gpiod_get(dev, "axentia,add", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->add)) {
+		if (PTR_ERR(tse850->add) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'add' gpio\n");
+		return PTR_ERR(tse850->add);
+	}
+	tse850->add_cache = 1;
+
+	tse850->loop1 = devm_gpiod_get(dev, "axentia,loop1", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop1)) {
+		if (PTR_ERR(tse850->loop1) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop1' gpio\n");
+		return PTR_ERR(tse850->loop1);
+	}
+	tse850->loop1_cache = 1;
+
+	tse850->loop2 = devm_gpiod_get(dev, "axentia,loop2", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop2)) {
+		if (PTR_ERR(tse850->loop2) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop2' gpio\n");
+		return PTR_ERR(tse850->loop2);
+	}
+	tse850->loop2_cache = 1;
+
+	tse850->ana = devm_regulator_get(dev, "axentia,ana");
+	if (IS_ERR(tse850->ana)) {
+		if (PTR_ERR(tse850->ana) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'ana' regulator\n");
+		return PTR_ERR(tse850->ana);
+	}
+
+	ret = regulator_enable(tse850->ana);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable the 'ana' regulator\n");
+		return ret;
+	}
+
+	ret = atmel_ssc_set_audio(tse850->ssc_id);
+	if (ret != 0) {
+		dev_err(dev,
+			"failed to set SSC %d for audio\n", tse850->ssc_id);
+		goto err_disable_ana;
+	}
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(dev, "snd_soc_register_card failed\n");
+		goto err_put_audio;
+	}
+
+	return 0;
+
+err_put_audio:
+	atmel_ssc_put_audio(tse850->ssc_id);
+err_disable_ana:
+	regulator_disable(tse850->ana);
+	return ret;
+}
+
+static int tse850_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	snd_soc_unregister_card(card);
+	atmel_ssc_put_audio(tse850->ssc_id);
+	regulator_disable(tse850->ana);
+
+	return 0;
+}
+
+static const struct of_device_id tse850_dt_ids[] = {
+	{ .compatible = "axentia,tse850-pcm5142", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tse850_dt_ids);
+
+static struct platform_driver tse850_driver = {
+	.driver = {
+		.name = "axentia-tse850-pcm5142",
+		.of_match_table = of_match_ptr(tse850_dt_ids),
+	},
+	.probe = tse850_probe,
+	.remove = tse850_remove,
+};
+
+module_platform_driver(tse850_driver);
+
+/* Module information */
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_DESCRIPTION("ALSA SoC driver for TSE-850 with PCM5142 codec");
+MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related

* [PATCH v4 2/3] ASoC: tse850: document axentia,tse850-pcm5142 bindings
From: Peter Rosin @ 2016-11-15 18:38 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479235095-13441-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../bindings/sound/axentia,tse850-pcm5142.txt      | 88 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt

diff --git a/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
new file mode 100644
index 000000000000..5b9b38f578bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
@@ -0,0 +1,88 @@
+Devicetree bindings for the Axentia TSE-850 audio complex
+
+Required properties:
+  - compatible: "axentia,tse850-pcm5142"
+  - axentia,ssc-controller: The phandle of the atmel SSC controller used as
+    cpu dai.
+  - axentia,audio-codec: The phandle of the PCM5142 codec.
+  - axentia,add-gpios: gpio specifier that controls the mixer.
+  - axentia,loop1-gpios: gpio specifier that controls loop relays on channel 1.
+  - axentia,loop2-gpios: gpio specifier that controls loop relays on channel 2.
+  - axentia,ana-supply: Regulator that supplies the output amplifier. Must
+    support voltages in the 2V - 20V range, in 1V steps.
+
+The schematics explaining the gpios are as follows:
+
+               loop1 relays
+   IN1 +---o  +------------+  o---+ OUT1
+            \                /
+             +              +
+             |   /          |
+             +--o  +--.     |
+             |  add   |     |
+             |        V     |
+             |      .---.   |
+   DAC +----------->|Sum|---+
+             |      '---'   |
+             |              |
+             +              +
+
+   IN2 +---o--+------------+--o---+ OUT2
+               loop2 relays
+
+The 'loop1' gpio pin controlls two relays, which are either in loop position,
+meaning that input and output are directly connected, or they are in mixer
+position, meaning that the signal is passed through the 'Sum' mixer. Similarly
+for 'loop2'.
+
+In the above, the 'loop1' relays are inactive, thus feeding IN1 to the mixer
+(if 'add' is active) and feeding the mixer output to OUT1. The 'loop2' relays
+are active, short-cutting the TSE-850 from channel 2. IN1, IN2, OUT1 and OUT2
+are TSE-850 connectors and DAC is the PCB name of the (filtered) output from
+the PCM5142 codec.
+
+Example:
+
+	&i2c {
+		codec: pcm5142@4c {
+			compatible = "ti,pcm5142";
+
+			reg = <0x4c>;
+
+			AVDD-supply = <&reg_3v3>;
+			DVDD-supply = <&reg_3v3>;
+			CPVDD-supply = <&reg_3v3>;
+
+			clocks = <&sck>;
+
+			pll-in = <3>;
+			pll-out = <6>;
+		};
+	};
+
+	ana: ana-reg {
+		compatible = "pwm-regulator";
+
+		regulator-name = "ANA";
+
+		pwms = <&pwm0 2 1000 PWM_POLARITY_INVERTED>;
+		pwm-dutycycle-unit = <1000>;
+		pwm-dutycycle-range = <100 1000>;
+
+		regulator-min-microvolt = <2000000>;
+		regulator-max-microvolt = <20000000>;
+		regulator-ramp-delay = <1000>;
+	};
+
+	sound {
+		compatible = "axentia,tse850-pcm5142";
+
+		axentia,ssc-controller = <&ssc0>;
+		axentia,audio-codec = <&codec>;
+
+		axentia,add-gpios = <&pioA 8 GPIO_ACTIVE_LOW>;
+		axentia,loop1-gpios = <&pioA 10 GPIO_ACTIVE_LOW>;
+		axentia,loop2-gpios = <&pioA 11 GPIO_ACTIVE_LOW>;
+
+		axentia,ana-supply = <&ana>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 851b89b9edcb..e6bf00e53bf3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2318,6 +2318,12 @@ F:	include/uapi/linux/ax25.h
 F:	include/net/ax25.h
 F:	net/ax25/
 
+AXENTIA ASOC DRIVERS
+M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+L:	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/sound/axentia,*
+
 AZ6007 DVB DRIVER
 M:	Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
 M:	Mauro Carvalho Chehab <mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
-- 
2.1.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 related

* [PATCH v4 1/3] ASoC: atmel_ssc_dai: if not provided, default to sensible dividers
From: Peter Rosin @ 2016-11-15 18:38 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	devicetree
In-Reply-To: <1479235095-13441-1-git-send-email-peda@axentia.se>

When this driver masters BCLK and/or LRCLK, and noone has stated
differently, assume that all the bits of a frame are used.

This relieves the cpu dai users from the duty to fill in the dividers for
the common case.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 sound/soc/atmel/atmel_ssc_dai.c | 83 +++++++++++++++++++++++++++++++++++++----
 sound/soc/atmel/atmel_ssc_dai.h |  1 +
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 16e459aedffe..a1e2c5682dcd 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -380,6 +380,7 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
 		ssc_writel(ssc_p->ssc->regs, CR, SSC_BIT(CR_SWRST));
 		/* Clear the SSC dividers */
 		ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
+		ssc_p->forced_divider = 0;
 	}
 	spin_unlock_irq(&ssc_p->lock);
 
@@ -426,14 +427,17 @@ static int atmel_ssc_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
 		else
 			if (div != ssc_p->cmr_div)
 				return -EBUSY;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_CMR_DIV);
 		break;
 
 	case ATMEL_SSC_TCMR_PERIOD:
 		ssc_p->tcmr_period = div;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_TCMR_PERIOD);
 		break;
 
 	case ATMEL_SSC_RCMR_PERIOD:
 		ssc_p->rcmr_period = div;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_RCMR_PERIOD);
 		break;
 
 	default:
@@ -443,6 +447,28 @@ static int atmel_ssc_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
 	return 0;
 }
 
+/* Is the cpu-dai master of the frame clock? */
+static int atmel_ssc_cfs(struct atmel_ssc_info *ssc_p)
+{
+	switch (ssc_p->daifmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFS:
+	case SND_SOC_DAIFMT_CBS_CFS:
+		return 1;
+	}
+	return 0;
+}
+
+/* Is the cpu-dai master of the bit clock? */
+static int atmel_ssc_cbs(struct atmel_ssc_info *ssc_p)
+{
+	switch (ssc_p->daifmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFM:
+	case SND_SOC_DAIFMT_CBS_CFS:
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Configure the SSC.
  */
@@ -459,6 +485,9 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	u32 tfmr, rfmr, tcmr, rcmr;
 	int ret;
 	int fslen, fslen_ext;
+	u32 cmr_div;
+	u32 tcmr_period;
+	u32 rcmr_period;
 
 	/*
 	 * Currently, there is only one set of dma params for
@@ -470,6 +499,46 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	else
 		dir = 1;
 
+	/*
+	 * If the cpu dai should provide BCLK, but noone has provided the
+	 * divider needed for that to work, fall back to something sensible.
+	 */
+	cmr_div = ssc_p->cmr_div;
+	if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_CMR_DIV)) &&
+	    atmel_ssc_cbs(ssc_p)) {
+		int bclk_rate = snd_soc_params_to_bclk(params);
+
+		if (bclk_rate < 0) {
+			dev_err(dai->dev, "unable to calculate cmr_div: %d\n",
+				bclk_rate);
+			return bclk_rate;
+		}
+
+		cmr_div = DIV_ROUND_CLOSEST(ssc_p->mck_rate, 2 * bclk_rate);
+	}
+
+	/*
+	 * If the cpu dai should provide LRCLK, but noone has provided the
+	 * dividers needed for that to work, fall back to something sensible.
+	 */
+	tcmr_period = ssc_p->tcmr_period;
+	rcmr_period = ssc_p->rcmr_period;
+	if (atmel_ssc_cfs(ssc_p)) {
+		int frame_size = snd_soc_params_to_frame_size(params);
+
+		if (frame_size < 0) {
+			dev_err(dai->dev,
+				"unable to calculate tx/rx cmr_period: %d\n",
+				frame_size);
+			return frame_size;
+		}
+
+		if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_TCMR_PERIOD)))
+			tcmr_period = frame_size / 2 - 1;
+		if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_RCMR_PERIOD)))
+			rcmr_period = frame_size / 2 - 1;
+	}
+
 	dma_params = ssc_p->dma_params[dir];
 
 	channels = params_channels(params);
@@ -524,7 +593,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		fslen_ext = (bits - 1) / 16;
 		fslen = (bits - 1) % 16;
 
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, START_DELAY)
 			| SSC_BF(RCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -540,7 +609,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, START_DELAY)
 			| SSC_BF(TCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -606,7 +675,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		fslen_ext = (bits - 1) / 16;
 		fslen = (bits - 1) % 16;
 
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, START_DELAY)
 			| SSC_BF(RCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -623,7 +692,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, START_DELAY)
 			| SSC_BF(TCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -650,7 +719,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		 * MCK divider, and the BCLK signal is output
 		 * on the SSC TK line.
 		 */
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, 1)
 			| SSC_BF(RCMR_START, SSC_START_RISING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -665,7 +734,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, 1)
 			| SSC_BF(TCMR_START, SSC_START_RISING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -760,7 +829,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* set SSC clock mode register */
-	ssc_writel(ssc_p->ssc->regs, CMR, ssc_p->cmr_div);
+	ssc_writel(ssc_p->ssc->regs, CMR, cmr_div);
 
 	/* set receive clock mode and format */
 	ssc_writel(ssc_p->ssc->regs, RCMR, rcmr);
diff --git a/sound/soc/atmel/atmel_ssc_dai.h b/sound/soc/atmel/atmel_ssc_dai.h
index 80b153857a88..75194f582131 100644
--- a/sound/soc/atmel/atmel_ssc_dai.h
+++ b/sound/soc/atmel/atmel_ssc_dai.h
@@ -113,6 +113,7 @@ struct atmel_ssc_info {
 	unsigned short cmr_div;
 	unsigned short tcmr_period;
 	unsigned short rcmr_period;
+	unsigned int forced_divider;
 	struct atmel_pcm_dma_params *dma_params[2];
 	struct atmel_ssc_state ssc_state;
 	unsigned long mck_rate;
-- 
2.1.4

^ permalink raw reply related

* [PATCH v4 0/3] ASoC driver for the TSE-850
From: Peter Rosin @ 2016-11-15 18:38 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi!

v3 -> v4 changes:
- changed heading of the devicetree bindings as requested by Rob
- add ack from Rob on patch 2/3
- rebased onto v4.9-rc5

v2 -> v3 changes:
- update the preparatory atmel_ssc_dai patch with error checking and
  fallback handling of the CMR_DIV divider in addition to the new
  fallback handling of TCMR_PERIOD/RCMR_PERIOD from v2.

v1 -> v2 changes:
- new subject for the bindings patch to make it easier to find
- move the driver to the sound/soc/atmel directory
- add cached values for add/loop1/loop2 to avoid gets from output gpios
- use _cansleep when updating gpios
- add comment on how the regulator voltage is mapped to the ana enum
- drop the .hw_params hook that did set the cpu dai divider and...
- ...add a preparatory patch that does this in the cpu dai driver instead
- drop .init and set the dapm routes directly in the card struct instead


The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

This adds a driver for the "sound" bits of the device (quoted since it
is normally not used for normal sound output, but that works too of
course). 

I have not provided a patch to add axentia as a devicetree vendor prefix,
since such a patch is already pending in an IIO series [1] that is on its
merry way into -next.

However, there are a couple of points that I'm not 100% satisfied with
for this driver.

First, I do not know how to describe the relays that control if the
IN1/IN2 signals are directly routed towards OUT1/OUT2 or if they are
routed to the "add" switch. The dapm routing treats this as if the
IN1/IN2 signals are always routed to both the "add" switch and to
the muxes feeding OUT1/OUT2. This is fine with me since nothing is
powered in those sections anyway, so what dapm thinks does not really
matter. But it is a wart all the same.

Second, there's my comment in tse850_put_mix() when the "add" switch
is updated. I believe this update should really happen as a side
effect of the call to snd_soc_dapm_mixer_update_power(), so that it
happens at the right point compared to other stuff that is powered.
But I do not know how to hook that up and instead I flip the switch
before the call since it doesn't really matter. I.e., any noise
resulting from this badness is negligeble in practice.

Cheers,
Peter

[1] http://lwn.net/Articles/705931

Peter Rosin (3):
  ASoC: atmel_ssc_dai: if not provided, default to sensible dividers
  ASoC: tse850: document axentia,tse850-pcm5142 bindings
  ASoC: atmel: tse850: add ASoC driver for the Axentia TSE-850

 .../bindings/sound/axentia,tse850-pcm5142.txt      |  88 ++++
 MAINTAINERS                                        |   7 +
 sound/soc/atmel/Kconfig                            |  10 +
 sound/soc/atmel/Makefile                           |   2 +
 sound/soc/atmel/atmel_ssc_dai.c                    |  83 +++-
 sound/soc/atmel/atmel_ssc_dai.h                    |   1 +
 sound/soc/atmel/tse850-pcm5142.c                   | 472 +++++++++++++++++++++
 7 files changed, 656 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
 create mode 100644 sound/soc/atmel/tse850-pcm5142.c

-- 
2.1.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 2/2] ARM: zynq: Fix W=1 dtc 1.4 warnings
From: Michal Simek @ 2016-11-15 18:01 UTC (permalink / raw)
  To: Julia Cartwright, Michal Simek
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	S?ren Brinkmann, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Steffen Trumtrar, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Peter Crosthwaite, Rob Herring, Rob Herring, Mark Rutland,
	Russell King
In-Reply-To: <20161115154608.GH30999-ew3lsbMjNqt5wtABiV/Xjqyly8cj88Ttqxv4g6HH51o@public.gmane.org>


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

Hi,

On 15.11.2016 16:46, Julia Cartwright wrote:
> On Tue, Nov 15, 2016 at 03:07:27PM +0100, Michal Simek wrote:
>> The patch removes these warnings reported by dtc 1.4:
>> Warning (unit_address_vs_reg): Node /pmu has a reg or ranges property,
>> but no unit name
>> Warning (unit_address_vs_reg): Node /fixedregulator@0 has a unit name,
>> but no reg property
>> Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>> property, but no unit name
>>
>> Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> 
> Reviewed-by: Julia Cartwright <julia-acOepvfBmUk@public.gmane.org>
>>
>> ---
>>
>>  arch/arm/boot/dts/zynq-7000.dtsi      | 4 ++--
>>  arch/arm/boot/dts/zynq-parallella.dts | 2 +-
>>  arch/arm/boot/dts/zynq-zc702.dts      | 2 +-
>>  arch/arm/boot/dts/zynq-zc706.dts      | 2 +-
>>  arch/arm/boot/dts/zynq-zed.dts        | 2 +-
>>  arch/arm/boot/dts/zynq-zybo.dts       | 2 +-
>>  6 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index f47a6c1cc752..402b5bbe3b5b 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -42,14 +42,14 @@
>>  		};
>>  	};
>>  
>> -	pmu {
>> +	pmu@f8891000 {
>>  		compatible = "arm,cortex-a9-pmu";
>>  		interrupts = <0 5 4>, <0 6 4>;
>>  		interrupt-parent = <&intc>;
>>  		reg = < 0xf8891000 0x1000 0xf8893000 0x1000 >;
> 
> Style nit: we should drop the space before/after '<' and '>'; and,
> perhaps separate the entries to be a bit more readable:
> 
>    reg = <0xf8891000 0x1000>,
>          <0xf8893000 0x1000>;

yes we can do it. Do you want to send separate patch for it?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs



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

^ permalink raw reply

* Re: [PATCH 09/11] ARM: shmobile: document SK-RZG1M board
From: Simon Horman @ 2016-11-15 17:55 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3956722.6l6VXXKMOL-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>

On Sat, Oct 29, 2016 at 01:23:59AM +0300, Sergei Shtylyov wrote:
> Document the SK-RZG1E device tree bindings, listing it as a supported board.
> 
> This allows to use checkpatch.pl to validate .dts files referring to the
> SK-RZG1E board.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

I have queued this up after with s/SK-RZG1M/SK-RZG1E/

> ---
>  Documentation/devicetree/bindings/arm/shmobile.txt |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: renesas/Documentation/devicetree/bindings/arm/shmobile.txt
> ===================================================================
> --- renesas.orig/Documentation/devicetree/bindings/arm/shmobile.txt
> +++ renesas/Documentation/devicetree/bindings/arm/shmobile.txt
> @@ -77,6 +77,8 @@ Boards:
>      compatible = "renesas,salvator-x", "renesas,r8a7796";
>    - SILK (RTP0RC7794LCB00011S)
>      compatible = "renesas,silk", "renesas,r8a7794"
> +  - SK-RZG1E (YR8A77450S000BE)
> +    compatible = "renesas,sk-rzg1e", "renesas,r8a7745"
>    - SK-RZG1M (YR8A77430S000BE)
>      compatible = "renesas,sk-rzg1m", "renesas,r8a7743"
>    - Wheat
> 
--
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 06/11] ARM: dts: r8a7745: add [H]SCIF{A|B} support
From: Simon Horman @ 2016-11-15 17:55 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, linux-lFZ/pmaqli7XmaaqVzeoHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <2366203.iKt2DUneo7-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>

On Sat, Oct 29, 2016 at 01:19:40AM +0300, Sergei Shtylyov wrote:
> Describe [H]SCIF{|A|B} ports in the R8A7745 device tree.
> 
> Based on the original (and large) patch by Dmitry Shifrin
> <dmitry.shifrin-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

I have queued this up after updating the indentation to use
tabs where possible.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 0/11] Add R8A7745/SK-RZG1E board support
From: Simon Horman @ 2016-11-15 17:55 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-renesas-soc, robh+dt, mark.rutland, devicetree, magnus.damm,
	linux, linux-arm-kernel
In-Reply-To: <2368353.xfo5beGC5E@wasted.cogentembedded.com>

On Sat, Nov 05, 2016 at 12:42:33AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
>    Here's the set of 11 patches against Simon Horman's 'renesas.git' repo's
> 'renesas-devel-201611104-v4.9-rc3' tag plus the R8A7743/SK-RZG1M patch series
> posted earlier. I'm adding the device tree support for the R8A7745-based
> SK-RZG1E board. The SoC is close to R8A7794 and the board seems identical to the
> R8A7794/SILK board. The device tree patches depend on the R8A7745 CPG/MSSR
> driver series in order to compile and work.

Thanks, I have queued this up.

^ permalink raw reply

* Re: [PATCH v6 3/7] ARM: dts: r8a7743: add [H]SCIF{A|B} support
From: Simon Horman @ 2016-11-15 17:54 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, linux-lFZ/pmaqli7XmaaqVzeoHQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <29562553.JmvzVUkhx3-gHKXc3Y1Z8zGSmamagVegGFoWSdPRAKMAL8bYrjMMd8@public.gmane.org>

On Mon, Oct 31, 2016 at 10:55:39PM +0300, Sergei Shtylyov wrote:
> Describe [H]SCIF{|A|B} ports in the R8A7743 device tree.
> 
> Based on the original (and large) patch by Dmitry Shifrin
> <dmitry.shifrin-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

I have queued this up after updating the indentation to use
tabs where possible.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


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