Devicetree
 help / color / mirror / Atom feed
* [PATCHv3 RESEND 5/8] ARM: dts: stm32: enable RTC on stm32f429-disco
From: Amelie Delaunay @ 2017-01-16 13:29 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
	Alexandre Torgue
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Amelie Delaunay
In-Reply-To: <1484573388-590-1-git-send-email-amelie.delaunay-qxv4g6HH51o@public.gmane.org>

This patch enables RTC on stm32f429-disco with LSI as clock source because
X2 crystal for LSE is not fitted by default.

Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
---
 arch/arm/boot/dts/stm32f429-disco.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
index 7d0415e..9222b9f 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -94,6 +94,12 @@
 	clock-frequency = <8000000>;
 };
 
+&rtc {
+	assigned-clocks = <&rcc 1 CLK_RTC>;
+	assigned-clock-parents = <&rcc 1 CLK_LSI>;
+	status = "okay";
+};
+
 &usart1 {
 	pinctrl-0 = <&usart1_pins_a>;
 	pinctrl-names = "default";
-- 
1.9.1

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

^ permalink raw reply related

* [PATCHv3 RESEND 6/8] ARM: dts: stm32: enable RTC on stm32f469-disco
From: Amelie Delaunay @ 2017-01-16 13:29 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
	Alexandre Torgue
  Cc: devicetree, linux-arm-kernel, linux-kernel, Amelie Delaunay
In-Reply-To: <1484573388-590-1-git-send-email-amelie.delaunay@st.com>

This patch enables RTC on stm32f469-disco with default LSE clock source.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 arch/arm/boot/dts/stm32f469-disco.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index fda12a4..f52b9f6 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -78,6 +78,10 @@
 	clock-frequency = <8000000>;
 };
 
+&rtc {
+	status = "okay";
+};
+
 &usart3 {
 	pinctrl-0 = <&usart3_pins_a>;
 	pinctrl-names = "default";
-- 
1.9.1

^ permalink raw reply related

* [PATCHv3 RESEND 7/8] ARM: dts: stm32: enable RTC on stm32429i-eval
From: Amelie Delaunay @ 2017-01-16 13:29 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Russell King, Maxime Coquelin,
	Alexandre Torgue
  Cc: devicetree, linux-arm-kernel, linux-kernel, Amelie Delaunay
In-Reply-To: <1484573388-590-1-git-send-email-amelie.delaunay@st.com>

This patch enables RTC on stm32429i-eval with default LSE clock source.

Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 2181220..eedb27d 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -157,6 +157,10 @@
 	};
 };
 
+&rtc {
+	status = "okay";
+};
+
 &usart1 {
 	pinctrl-0 = <&usart1_pins_a>;
 	pinctrl-names = "default";
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 1/3] mtd: name the mtd device with an optional label property
From: Boris Brezillon @ 2017-01-16 13:32 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Marek Vasut, Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <1484573225-19095-2-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

On Mon, 16 Jan 2017 14:27:03 +0100
Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:

> This can be used to easily identify a specific chip on a system with
> multiple chips.
> 
> Suggested-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>

Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

> ---
>  drivers/mtd/mtdcore.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 052772f7caef..bf61557b599d 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -654,6 +654,13 @@ static int mtd_add_device_partitions(struct mtd_info *mtd,
>   */
>  static void mtd_set_dev_defaults(struct mtd_info *mtd)
>  {
> +	/*
> +	 * If DT support is enabled and we have a valid of_node pointer, try to
> +	 * extract the MTD name from the label property.
> +	 */
> +	if (IS_ENABLED(CONFIG_OF) && mtd->dev.of_node)
> +		of_property_read_string(mtd->dev.of_node, "label", &mtd->name);
> +
>  	if (mtd->dev.parent) {
>  		if (!mtd->owner && mtd->dev.parent->driver)
>  			mtd->owner = mtd->dev.parent->driver->owner;

--
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] mtd: spi-nor: add dt support for Everspin MRAMs
From: Uwe Kleine-König @ 2017-01-16 13:35 UTC (permalink / raw)
  To: Masahiko Iwamoto, Jagan Teki, Marek Vasut, Cyrille Pitchen
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Rafał Miłecki,
	Geert Uytterhoeven, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The MR25 family doesn't support JEDEC, so they need explicit mentioning
in the list of supported spi IDs. This makes it possible to add these
using for example:

	compatible = "everspin,mr25h40";

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Hello,

independent of the decision if -nonjedec is ok for m25p, this is needed
to make Everspin's MRAMs work.

Best regards
Uwe

 drivers/mtd/devices/m25p80.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{"m25p40-nonjedec"},	{"m25p80-nonjedec"},	{"m25p16-nonjedec"},
 	{"m25p32-nonjedec"},	{"m25p64-nonjedec"},	{"m25p128-nonjedec"},
 
+	/* Everspin MRAMs without JEDEC support */
+	{ "mr25h256" }, /* 256 kib, 40 MHz */
+	{ "mr25h10" },  /*   1 Mib, 40 MHz */
+	{ "mr25h40" },  /*   4 Mib, 40 MHz */
+
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
-- 
2.11.0

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

^ permalink raw reply

* Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files
From: Joao Pinto @ 2017-01-16 13:38 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Joao Pinto, Bjorn Helgaas, Jingoo Han,
	Arnd Bergmann
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, linux-arm-kernel-VrBV9hrLPhE,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0
In-Reply-To: <587CAEBD.1010803-l0cyMroinI0@public.gmane.org>

Às 11:30 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
> Hi Joao,
> 
> On Monday 16 January 2017 03:57 PM, Joao Pinto wrote:
>>
>> Hi,
>>
>> Às 5:21 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
>>> Hi Joao,
>>>
>>> On Friday 13 January 2017 10:19 PM, Joao Pinto wrote:
>>>> Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
>>>>> Split pcie-designware.c into pcie-designware-host.c that contains
>>>>> the host specific parts of the driver and pcie-designware.c that
>>>>> contains the parts used by both host driver and endpoint driver.
>>>>>
>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>>>>> ---
>>>>>  drivers/pci/dwc/Makefile               |    2 +-
>>>>>  drivers/pci/dwc/pcie-designware-host.c |  619 ++++++++++++++++++++++++++++++++
>>>>>  drivers/pci/dwc/pcie-designware.c      |  613 +------------------------------
>>>>>  drivers/pci/dwc/pcie-designware.h      |    8 +
>>>>>  4 files changed, 634 insertions(+), 608 deletions(-)
>>>>>  create mode 100644 drivers/pci/dwc/pcie-designware-host.c
>>>>>
>>>>> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
>>>>> index 7d27c14..3b57e55 100644
>>>>> --- a/drivers/pci/dwc/Makefile
>>>>> +++ b/drivers/pci/dwc/Makefile
>>>>> @@ -1,4 +1,4 @@
>>>>
>>>> (snip...)
>>>>
>>>>> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>>> -				      int type, u64 cpu_addr, u64 pci_addr,
>>>>> -				      u32 size)
>>>>> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>>>> +			       u64 cpu_addr, u64 pci_addr, u32 size)
>>>>>  {
>>>>>  	u32 retries, val;
>>>>>  
>>>>> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>>>  	dev_err(pci->dev, "iATU is not being enabled\n");
>>>>>  }
>>>>
>>>> Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
>>>> the pcie-designware-host.
>>>
>>> That is not true. Outbound ATU should be programmed to access host side buffers
>>> and inbound ATU should be programmed for the host to access EP mem space.
>>
>> Sorry, I was not clear enough. What I was trying to suggest is, since the ATU
>> programming is done by the host, wouldn't be better to include it in the
>> pcie-designware-host? It is just an architectural detail.
> 
> ATU programming is required in EP mode. See "[PATCH 24/37] PCI: dwc:
> designware: Add EP mode support" in this patch series.
> 
> Anything that's required by both EP mode and RC mode, I've placed in
> pcie-designware.c

Agreed!

> 
> Thanks
> Kishon
> 

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

^ permalink raw reply

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


Hi,

Às 10:13 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
> + Joao, Jingoo
> 
> Hi,
> 
> On Monday 16 January 2017 03:01 PM, Lukasz Majewski wrote:
>> Hi Kishon,
>>
>>> Hi Łukasz,
>>>
>>> On Monday 16 January 2017 12:19 PM, Lukasz Majewski wrote:
>>>> Hi Kishon,
>>>>
>>>>> Hi,
>>>>>
>>>>> On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote:
>>>>>> Some devices (due to e.g. bad PCIe signal integrity) require to
>>>>>> run with forced GEN1 speed on PCIe bus.
>>>>>>
>>>>>> This patch changes the speed explicitly on dra7 based devices when
>>>>>> proper device tree attribute is defined for the PCIe controller.
>>>>>>
>>>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>>>>>
>>>>> Bjorn has already queued a patch to do the same thing
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_cgit_linux_kernel_git_helgaas_pci.git_log_-3Fh-3Dpci_host-2Ddra7xx&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=zD82T5n4WcL7Ga-NSY2NI7KE75xQ99hN-mW2yX46wQk&s=E8zk1CbKxGH-f3fw_WpXxFU-A8BLkgA8NusCaxk1SvA&e= 
>>>>
>>>> It seems like Bjorn only modifies CAP registers.
>>>
>>> The patch also modifies the LNKCTL2 register.
>>>>
>>>> He also needs to change register with 0x080C offset to actually
>>>> ( PCIECTRL_PL_WIDTH_SPEED_CTL )
>>>
>>> This bit is used to initiate speed change (after the link is
>>> initialized in GEN1). Resetting the bit (like what you have done
>>> here) prevents speed change.
>>
>> This is strange, but e2e advised me to do things as I did in the patch
>> to _force_ GEN1 operation on PCIe2 port [1] (AM5728)
>>
>> Link:
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__e2e.ti.com_support_arm_sitara-5Farm_f_791_t_566421&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=zD82T5n4WcL7Ga-NSY2NI7KE75xQ99hN-mW2yX46wQk&s=uXLwglyRYqKpwp1JSxkOWmKpQ2wjfhgofpm8DCfquNw&e= 
>>
>> Both patches modify 0x5180 007C register to set GEN1 capability
>> (PCI_EXP_LNKCAP_SLS_2_5GB)
>>
>> The problem is with second register (in your patch):
>>
>> From SPRUHZ6G TRM:
>>
>> PCIECTRL_EP_DBICS_LNK_CAS_2 (0x5180 00A0)
>> - TRGT_LINK_SPEED (Reset 0x1) - "Target Link Speed" - no more
>>   description in TRM
>>
>> It is set to PCI_EXP_LNKCAP_SLS_2_5GB = 0x1, which is the same as
>> default /reset value.
> 
> The default value is 0x2 (or else none of the cards would have enumerated in GEN2)
>>
>>
>> Could you clarify which way to _force_ PCIe GEN1 operation is correct?
>> Mine shows differences in lspci output (as posted in [1]).
> 
> You'll see the difference even with the patch in Bjorn's tree ;-)
> 
> I think these are 2 different approaches to keep the link at GEN1. Joao or
> Jingoo, do you have any suggestion here?

I am going to check this and come back soon with a fundamented opinion.

Thanks.

> 
>>
>>>
>>> IMO the better way is to set the LNKCTL2 to GEN1 instead of hacking
>>> the IP register.
>>
>> From the original patch description:
>>
>> "Add support to force Root Complex to work in GEN1 mode if so desired,
>> but don't force GEN1 mode on any board just yet."
>>
>> Are there any (floating around) patches allowing forcing GEN1 operation
>> on any board (I would like to reuse/port them to my current solution)?
> 
> For setting to GEN1 mode, "max-link-speed" should be set to 1 in dt with the
> patch in Bjorn's tree.
> 
> Thanks
> Kishon
> 

^ permalink raw reply

* Re: [PATCH v3 16/24] media: Add i.MX media core driver
From: Philipp Zabel @ 2017-01-16 13:47 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
	mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
	bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
	tiffany.lin, jean-christophe.trotin, horms+renesas,
	niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
	andrew-ct.chen, gregkh, devicetree, linux-kernel,
	linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <a94025b4-c4dd-de51-572e-d2615a7246e4@gmail.com>

On Sat, 2017-01-14 at 14:46 -0800, Steve Longerbeam wrote:
[...]
> >> +Unprocessed Video Capture:
> >> +--------------------------
> >> +
> >> +Send frames directly from sensor to camera interface, with no
> >> +conversions:
> >> +
> >> +-> ipu_smfc -> camif
> > I'd call this capture interface, this is not just for cameras. Or maybe
> > idmac if you want to mirror hardware names?
> 
> Camif is so named because it is the V4L2 user interface for video
> capture. I suppose it could be named "capif", but that doesn't role
> off the tongue quite as well.

Agreed, capif sounds weird. I find camif a bit confusing though, because
Samsung S3C has a camera interface that is actually called "CAMIF".

> >> +Note the ipu_smfc can do pixel reordering within the same colorspace.
> > That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).
> 
> yes, the doc is re-worded to make that more clear.
> 
> >> +For example, its sink pad can take UYVY2X8, but its source pad can
> >> +output YUYV2X8.
> > I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
> > in the CSI chapter, for 8-bit per component data, the internal format
> > between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
> > (or "bayer/generic data"). In either case, the internal format does not
> > change along the way.
> 
> these are pixels in memory buffers, not the IPU internal formats.

As long as we are talking about the CSI -> SMFC -> IDMAC path, these
should be IPU internal formats. How else would one choose between 8-bit
companded RGB, and 16-bit expanded RGB for a 10-bit per component input
signal? This is the same issue as in the next comment.

> >> +   media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
> >> +   # Configure pads for OV5640 pipeline
> >> +   media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
> > [...]
> >> +   media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
> > I agree this looks very intuitive, but technically correct for the
> > csi1:1 and camif1:0 pads would be a 32-bit YUV format.
> > (MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
> >
> > I think it would be better to use the correct format
> 
> I'm not sure I follow you here.

The ov5640 sends UYVY2X8 on the wire, so pads "ov5640_mipi 1-0040":0
up to "ipu1_csi1":0 are correct. But the CSI writes 32-bit YUV values
into the SMFC, so the CSI output pad and the IDMAC input pad should have
a YUV8_1X32 format.

Chapter 37.4.2.3 "FCW & FCR - Format converter write and read" in the
IDMAC chapter states that all internal submodules only work on 8-bit per
component formats with four components: YUVA or RGBA.

> > [...]
> > Is this a whole software buffer queue implementation? I thought the
> > whole point of putting the custom mem2mem framework into the capture
> > driver was to use the hardware FSU channel linking?
> 
>   see below.
> 
> > What is the purpose of this if the sink should be triggered by the FSU?
> 
> Ok, here is where I need to make an admission.
> 
> The only FSU links I have attempted (and which currently have entries
> in the fsu_link_info[] table), are the enc/vf/pp --> IRT links for rotation.

Which are not described as media entity links because the rotation units
do not have separate media entities. So me arguing against handling
mem2mem chaining via media entity links doesn't concern these implicit
links.

> There does not appear to be support in the FSU for linking a write channel
> to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
> is support for the direct link from CSI (which I am using), but that's 
> not an
> IDMAC channel link.
> 
> There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
> 0..2 (and 3? it's not clear, and not clear whether this includes channel 1).

As I read it, that is 0 and 2 only, no idea why. But since there are
only 2 CSIs, that shouldn't be a problem.

> But I think this links to channel 12, and not to channels 8,9,10 to the 
> VDIC.
> Or will it? It's worth experimenting. It would have helped if FSL listed 
> which
> IDMAC channels these FSU links correspond to, instead of making us guess
> at it.

I would have assumed that the FSU triggering only works on 1:1 channels
and the VDIC with its three input channels is different. But then
there's the alleged VDOA link to ch8/9/10 ro ch9, depending on
VDI_MOT_SEL.

This makes me more convinced that the CSI -> VDIC link should only
describe the direct path (real-time mode, single field).

> In any event, the docs are not clear enough to implement a real FSU
> link to the VDIC read channels, if it's even possible. And trying to get
> programming help from FSL can be difficult, and no coding examples
> for this link AFAIK.
> 
> So I ended resorted to linking to VDIC channels 8,9,10 with a software
> approach, instead of attempting a hardware FSU link.
> 
> The EOF interrupt handler for the SMFC channels informs the VDIC
> entity via a v4l2_subdev_ioctl() call that a buffer is available. The
> VDIC then manually kicks off its read channels to bring that buffer
> (and a previous buffer for F(n-1) field) into the VDIC.
> 
> There is a small amount of extra overhead going this route compared
> to a FSU hardware link: there is the EOF irq latency (a few usec), and
> the CPU overhead for the VDIC to manually start the read channels,
> which is also a few usec at most (see prepare_vdi_in_buffers() in
> imx-vdic.c). So in total at most ~10 usec of extra overhead (CPU
> use plus irq latency) under normal system load.

That the same low overhead could be reached by linking videobuf2 queues
of different video devices, that would be a lot more flexible.

> Of course, in order to implement this software link, I had to implement
> a straightforward FIFO dma buffer ring. The sink (VDIC) allocates the ring
> at stream on, and the source requests a pointer to this ring in its own
> stream on. Passing buffers from source to sink then follows a 
> straightforward
> FIFO queue/done/dequeue/queue model: sink queues buffers to src, src
> grabs queued buffers and makes them active, src signals completed
> buffers to sink, sink dequeues buffers in response, and sink queues
> buffers back when it is finished with them.

Thank you for the explanation.

[...]
> >> +static const u32 power_off_seq[] = {
> >> +	IMX_MEDIA_GRP_ID_IC_PP,
> >> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> >> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> >> +	IMX_MEDIA_GRP_ID_SMFC,
> >> +	IMX_MEDIA_GRP_ID_CSI,
> >> +	IMX_MEDIA_GRP_ID_VIDMUX,
> >> +	IMX_MEDIA_GRP_ID_SENSOR,
> >> +	IMX_MEDIA_GRP_ID_CSI2,
> >> +};
> > This seems somewhat arbitrary. Why is a power sequence needed?
> 
> The CSI-2 receiver must be powered up before the sensor, that's the
> only requirement IIRC. The others have no s_power requirement. So I
> can probably change this to power up in the frontend -> backend order
> (IC_PP to sensor). And vice-versa for power off.

Yes, I think that should work (see below).

> > [...]
> >> +/*
> >> + * Turn current pipeline power on/off starting from start_entity.
> >> + * Must be called with mdev->graph_mutex held.
> >> + */
> >> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
> >> +				 struct media_entity_graph *graph,
> >> +				 struct media_entity *start_entity, bool on)
> >> +{
> >> +	struct media_entity *entity;
> >> +	struct v4l2_subdev *sd;
> >> +	int i, ret = 0;
> >> +	u32 id;
> >> +
> >> +	for (i = 0; i < NUM_POWER_ENTITIES; i++) {
> >> +		id = on ? power_on_seq[i] : power_off_seq[i];
> >> +		entity = find_pipeline_entity(imxmd, graph, start_entity, id);
> >> +		if (!entity)
> >> +			continue;
> >> +
> >> +		sd = media_entity_to_v4l2_subdev(entity);
> >> +
> >> +		ret = v4l2_subdev_call(sd, core, s_power, on);
> >> +		if (ret && ret != -ENOIOCTLCMD)
> >> +			break;
> >> +	}
> >> +
> >> +	return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> > This should really be handled by v4l2_pipeline_pm_use.
> 
> I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
> doing some other stuff that bothered me, at least that's what I remember.
> I will revisit this.

I have used it with a tc358743 -> mipi-csi2 pipeline, it didn't cause
any problems. It would be better to reuse and, if necessary, fix the
existing infrastructure where available.

> >> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
> >> +				   struct imx_media_subdev *csi[4])
> >> +{
> >> +	int ret;
> >> +
> >> +	/* there must be at least one CSI in first IPU */
> > Why?
> 
> Well yeah, imx-media doesn't necessarily need a CSI if things
> like the VDIC or post-processor are being used by an output
> overlay pipeline, for example. I'll fix this.

I haven't even thought that far, but there could be boards with only a
parallel sensor connected to IPU2 CSI1 and IPU1 disabled for power
saving reasons.

regards
Philipp

^ permalink raw reply

* Re: [RFC 0/1] Platform driver support for 'amd5536udc' driver
From: Felipe Balbi @ 2017-01-16 13:51 UTC (permalink / raw)
  To: Arnd Bergmann, Raviteja Garimella
  Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, BCM Kernel Feedback,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, John Youn
In-Reply-To: <6422487.mhqDBHHcCr@wuerfel>

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


Hi,

Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
> On Friday, January 6, 2017 12:29:12 PM CET Raviteja Garimella wrote:
>> Hi Arnd,
>> 
>> On Fri, Jan 6, 2017 at 3:33 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > On Thursday, January 5, 2017 1:53:16 PM CET Raviteja Garimella wrote:
>> >> The UDC is based on Synopsys Designware core USB (2.0) Device controller
>> >> IP.
>> > ...
>> >> This is a request for comments from maintainers/others regarding approach
>> >> on whether to have 2 different drivers (one each for AMD and Broadcom)
>> >> with a common library (3 files in total), or have a single driver like
>> >> it's done in this patch and have the driver filename changed to some
>> >> common name based on ther underlying IP, like snps_udc.c.
>> >
>> > I have not looked at the code at all, so sorry for my ignorance, but
>> > isn't the IP block you describe the one that drivers/usb/dwc2/ is for?
>> > Could you add support for the Broadcom hardware there instead?
>> 
>> The current driver I submitted is for a different Synopsys IP (USB
>> Device Controller IP,
>> not the HS OTG). It's confirmed by John Youn (from Synopsys) earlier.
>> 
>
> Ok, sounds fine the. I'd suggest taking the current driver than and
> splitting out the pci_driver front-end into a separate module that
> calls exported symbols of the main driver, with the new platform
> driver in a third file that also calls the same exported symbols.

right, that's the best idea.

-- 
balbi

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

^ permalink raw reply

* Re: [PATCH 2/3] dt-bindings: mtd: add a common label property to all mtd devices
From: Boris Brezillon @ 2017-01-16 13:52 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Marek Vasut, Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <1484573225-19095-3-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

On Mon, 16 Jan 2017 14:27:04 +0100
Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:

> This can be used to easily identify a specific chip on a system with
> multiple chips.
> 
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mtd/common.txt | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/common.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
> new file mode 100644
> index 000000000000..a781641a3b00
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/common.txt
> @@ -0,0 +1,12 @@
> +* Common properties of all MTD devices
> +
> +Optional properties:
> +- label : name to assign to mtd. If omitted, the label is the MTD device name.

Hm, the description is not really clear. How about:

"
- label: user-defined MTD device name. Can be used to assign user
  friendly names to MTD devices (instead of the flash model or flash
  controller based name) in order to ease flash device identification
  and/or describe what they are used for.
"

Otherwise,

Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Thanks,

Boris

> +
> +Example:
> +
> +	flash@0 {
> +		label = "System-firmware";
> +
> +		/* flash type specific properties */
> +	};

--
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] mtd: spi-nor: add dt support for Everspin MRAMs
From: Rafał Miłecki @ 2017-01-16 13:55 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Masahiko Iwamoto, Jagan Teki, Marek Vasut, Cyrille Pitchen,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Sascha Hauer, Geert Uytterhoeven, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170116133503.13887-1-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On 16 January 2017 at 14:35, Uwe Kleine-König
<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> The MR25 family doesn't support JEDEC, so they need explicit mentioning
> in the list of supported spi IDs. This makes it possible to add these
> using for example:
>
>         compatible = "everspin,mr25h40";
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Hello,
>
> independent of the decision if -nonjedec is ok for m25p, this is needed
> to make Everspin's MRAMs work.
>
> Best regards
> Uwe
>
>  drivers/mtd/devices/m25p80.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -305,6 +305,11 @@ static const struct spi_device_id m25p_ids[] = {
>         {"m25p40-nonjedec"},    {"m25p80-nonjedec"},    {"m25p16-nonjedec"},
>         {"m25p32-nonjedec"},    {"m25p64-nonjedec"},    {"m25p128-nonjedec"},
>
> +       /* Everspin MRAMs without JEDEC support */
> +       { "mr25h256" }, /* 256 kib, 40 MHz */

A trivial thing: I think it should be Kib
https://en.wikipedia.org/wiki/Binary_prefix

-- 
Rafał
--
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: [RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and usable memory range
From: Andreas Färber @ 2017-01-16 14:02 UTC (permalink / raw)
  To: Neil Armstrong, carlo-KA+7E9HrN00dnm+yROfE0A
  Cc: Heinrich Schuchardt, khilman-rdvid1DuHRBWk0Htik3J/w,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9a1dbb0b-ad3d-7898-3b43-fd8f85892985-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Am 16.01.2017 um 11:26 schrieb Neil Armstrong:
> On 01/15/2017 04:44 PM, Andreas Färber wrote:
>> Am 23.12.2016 um 10:42 schrieb Heinrich Schuchardt:
>>> it really makes a difference if we write
>>>
>>>  	memory@0 {
>>>  		device_type = "memory";
>>>  		linux,usable-memory = <0x0 0x1000000 0x0 0x7f000000>;
>>>  	};
>>>
>>> or
>>>
>>>  	memory@0 {
>>>  		device_type = "memory";
>>>  		reg = <0x0 0x1000000 0x0 0x7f000000>;
>>>  	};
>>>
>>> The second version leads to failure of the Odroid C2.
>>>
>>> When I looked at /sys/firmware/fdt I saw this difference:
>>>
>>> --- fails
>>> +++ works
>>>
>>>         memory@0 {
>>> -               device_type = "memory";
>>>                 reg = <0x0 0x0 0x0 0x78000000>;
>>> +               device_type = "memory";
>>> +               linux,usable-memory = <0x0 0x1000000 0x0 0x7f000000>;
>>>         };
>>>
>>> I found the following sentence in the NXP forum:
>>> In case you want to overwrite the memory usage passed from u-boot, you
>>> can use "linux,usable-memory".
>>> https://community.nxp.com/thread/382284
>>
>> The Odroid-C2 is in mainline U-Boot. Please submit a patch to U-Boot
>> instead of forcing the creation of unnecessary new .dts files onto
>> everyone due to hardcoded linux,usable-memory properties. In fact, it
>> already reserves 0x1000000, so it seems you are merely using an older
>> U-Boot.
>>
>> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-meson/board.c;h=f159cbf849f75ab046e6f3a025bbc97c0bcfd59d;hb=HEAD#l39
>>
>> I would bet that the upper limit is unrelated here.
>>
>> Regards,
>> Andreas
>>
> 
> Hi Andreas,
> 
> I really disagree about relying on any work or properties added by any bootloader here, Amlogic SoCs has
> a lot of u-boot version in the field, and the Odroid-C2 is part of this.
> 
> Even if Odroid-c2 is in mainline U-Boot or not, the mainline Linux kernel should work using
> any U-boot version even with the one provided by Amlogic on their openlinux distribution channel.

That is not the position of the kernel maintainers though. They
deliberately rely on timers being enabled before entering Linux, which
broke my afboot-stm32 (which I could fix) as well as s5pv210 and vf610
based platforms by F+S (which remain broken to date).

And I documented how to chainload mainline U-Boot from downstream
Amlogic U-Boot, so it is easily fixable on Meson. The only thing missing
is Carlo resubmitting his MMC patches.

A bug somewhere does not justify breaking the whole Meson-gx* .dts
design for everyone, especially not without CC'ing me as the original
creator!

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Cyrille Pitchen @ 2017-01-16 14:03 UTC (permalink / raw)
  To: Cyrille Pitchen, Shawn Lin, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Marek Vasut,
	Rob Herring, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <e5852f21-7aa0-8091-dc85-774e306e6f73-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

Le 16/01/2017 à 12:10, Cyrille Pitchen a écrit :
> Hi Shawn,
> 
> Le 15/12/2016 à 10:27, Shawn Lin a écrit :
>> Add rockchip serial flash controller driver
>>
>> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> ---
>>
>> Changes in v4:
>> - simplify the code of get_if_type
>> - use dma_dir to simplify the code
>> - simplify the rockchip_sfc_do_rd_wr
>> - some minor improvements
>> - add reset controller when doing resume
>>
>> Changes in v3:
>> - use io{read32,write32}_rep to simplify the corner cases
>> - remove more unnecessary bit definitions
>> - some minor comment fixes and improvement
>> - fix wrong unregister function
>> - unify more code
>> - use nor to avoid constantly replicating the whole
>>   sfc->flash[sfc->num_chip].nor
>> - add email for MODULE_AUTHOR
>> - remove #if 1 --- #endif
>> - extract DMA code to imporve the code structure
>> - reset all when failing to do dma
>> - pass sfc to get_if_type
>> - rename sfc-no-dma to sfc-no-DMA
>>
>> Changes in v2:
>> - fix typos
>> - add some comment for buffer and others operations
>> - rename SFC_MAX_CHIP_NUM to MAX_CHIPSELECT_NUM
>> - use u8 for cs
>> - return -EINVAL for default case of get_if_type
>> - use readl_poll_*() to check timeout cases
>> - simplify and clarify some condition checks
>> - rework the bitshifts to simplify the code
>> - define SFC_CMD_DUMMY(x)
>> - fix ummap for dma read path and finish all the
>>   cache maintenance.
>> - rename to rockchip_sfc_chip_priv and embed struct spi_nor
>>   in it.
>> - add MODULE_AUTHOR
>> - add runtime PM and general PM support.
>> - Thanks for Marek's comments. Link:
>>   http://lists.infradead.org/pipermail/linux-mtd/2016-November/070321.html
>>
>>  MAINTAINERS                        |   8 +
>>  drivers/mtd/spi-nor/Kconfig        |   7 +
>>  drivers/mtd/spi-nor/Makefile       |   1 +
>>  drivers/mtd/spi-nor/rockchip-sfc.c | 872 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 888 insertions(+)
>>  create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1cd38a7..eb7e06d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10266,6 +10266,14 @@ L:	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>  S:	Odd Fixes
>>  F:	drivers/tty/serial/rp2.*
>>  
>> +ROCKCHIP SERIAL FLASH CONTROLLER DRIVER
>> +M:	Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> +L:	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> +L:	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
>> +F:	drivers/mtd/spi-nor/rockchip-sfc.c
>> +
>>  ROSE NETWORK LAYER
>>  M:	Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
>>  L:	linux-hams-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 4a682ee..bf783a8 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -76,4 +76,11 @@ config SPI_NXP_SPIFI
>>  	  Flash. Enable this option if you have a device with a SPIFI
>>  	  controller and want to access the Flash as a mtd device.
>>  
>> +config SPI_ROCKCHIP_SFC
>> +	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.
>> +
>>  endif # MTD_SPI_NOR
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 121695e..364d4c6 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -5,3 +5,4 @@ obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
>>  obj-$(CONFIG_SPI_HISI_SFC)	+= hisi-sfc.o
>>  obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
>>  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
>> +obj-$(CONFIG_SPI_ROCKCHIP_SFC)	+= rockchip-sfc.o
>> diff --git a/drivers/mtd/spi-nor/rockchip-sfc.c b/drivers/mtd/spi-nor/rockchip-sfc.c
>> new file mode 100644
>> index 0000000..102c08f
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/rockchip-sfc.c
>> @@ -0,0 +1,872 @@
>> +/*
>> + * Rockchip Serial Flash Controller Driver
>> + *
>> + * Copyright (c) 2016, Rockchip Inc.
>> + * Author: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <linux/bitops.h>
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/spi-nor.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/slab.h>
>> +
>> +/* System control */
>> +#define SFC_CTRL			0x0
>> +#define  SFC_CTRL_COMMON_BITS_1		0x0
>> +#define  SFC_CTRL_COMMON_BITS_2		0x1
>> +#define  SFC_CTRL_COMMON_BITS_4		0x2
>> +#define  SFC_CTRL_DATA_BITS_SHIFT	12
>> +#define  SFC_CTRL_ADDR_BITS_SHIFT	10
>> +#define  SFC_CTRL_CMD_BITS_SHIFT	8
>> +#define  SFC_CTRL_PHASE_SEL_NEGETIVE	BIT(1)
>> +
>> +/* Interrupt mask */
>> +#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)
>> +/* Interrupt clear */
>> +#define SFC_ICLR			0x8
>> +#define  SFC_ICLR_RX_FULL		BIT(0)
>> +#define  SFC_ICLR_RX_UFLOW		BIT(1)
>> +#define  SFC_ICLR_TX_OFLOW		BIT(2)
>> +#define  SFC_ICLR_TX_EMPTY		BIT(3)
>> +#define  SFC_ICLR_TRAN_FINISH		BIT(4)
>> +#define  SFC_ICLR_BUS_ERR		BIT(5)
>> +#define  SFC_ICLR_NSPI_ERR		BIT(6)
>> +#define  SFC_ICLR_DMA			BIT(7)
>> +/* FIFO threshold level */
>> +#define SFC_FTLR			0xc
>> +#define  SFC_FTLR_TX_SHIFT		0
>> +#define  SFC_FTLR_TX_MASK		0x1f
>> +#define  SFC_FTLR_RX_SHIFT		8
>> +#define  SFC_FTLR_RX_MASK		0x1f
>> +/* Reset FSM and FIFO */
>> +#define SFC_RCVR			0x10
>> +#define  SFC_RCVR_RESET			BIT(0)
>> +/* Enhanced mode */
>> +#define SFC_AX				0x14
>> +/* Address Bit number */
>> +#define SFC_ABIT			0x18
>> +/* Interrupt status */
>> +#define SFC_ISR				0x1c
>> +#define  SFC_ISR_RX_FULL_SHIFT		BIT(0)
>> +#define  SFC_ISR_RX_UFLOW_SHIFT		BIT(1)
>> +#define  SFC_ISR_TX_OFLOW_SHIFT		BIT(2)
>> +#define  SFC_ISR_TX_EMPTY_SHIFT		BIT(3)
>> +#define  SFC_ISR_TX_FINISH_SHIFT	BIT(4)
>> +#define  SFC_ISR_BUS_ERR_SHIFT		BIT(5)
>> +#define  SFC_ISR_NSPI_ERR_SHIFT		BIT(6)
>> +#define  SFC_ISR_DMA_SHIFT		BIT(7)
>> +/* FIFO status */
>> +#define SFC_FSR				0x20
>> +#define  SFC_FSR_TX_IS_FULL		BIT(0)
>> +#define  SFC_FSR_TX_IS_EMPTY		BIT(1)
>> +#define  SFC_FSR_RX_IS_EMPTY		BIT(2)
>> +#define  SFC_FSR_RX_IS_FULL		BIT(3)
>> +/* FSM status */
>> +#define SFC_SR				0x24
>> +#define  SFC_SR_IS_IDLE			0x0
>> +#define  SFC_SR_IS_BUSY			0x1
>> +/* Raw interrupt status */
>> +#define SFC_RISR			0x28
>> +#define  SFC_RISR_RX_FULL		BIT(0)
>> +#define  SFC_RISR_RX_UNDERFLOW		BIT(1)
>> +#define  SFC_RISR_TX_OVERFLOW		BIT(2)
>> +#define  SFC_RISR_TX_EMPTY		BIT(3)
>> +#define  SFC_RISR_TRAN_FINISH		BIT(4)
>> +#define  SFC_RISR_BUS_ERR		BIT(5)
>> +#define  SFC_RISR_NSPI_ERR		BIT(6)
>> +#define  SFC_RISR_DMA			BIT(7)
>> +/* Master trigger */
>> +#define SFC_DMA_TRIGGER			0x80
>> +/* Src or Dst addr for master */
>> +#define SFC_DMA_ADDR			0x84
>> +/* Command */
>> +#define SFC_CMD				0x100
>> +#define  SFC_CMD_IDX_SHIFT		0
>> +#define  SFC_CMD_DUMMY_SHIFT		8
>> +#define  SFC_CMD_DIR_RD			0
>> +#define  SFC_CMD_DIR_WR			1
>> +#define  SFC_CMD_DIR_SHIFT		12
>> +#define  SFC_CMD_ADDR_ZERO		(0x0 << 14)
>> +#define  SFC_CMD_ADDR_24BITS		(0x1 << 14)
>> +#define  SFC_CMD_ADDR_32BITS		(0x2 << 14)
>> +#define  SFC_CMD_ADDR_FRS		(0x3 << 14)
>> +#define  SFC_CMD_TRAN_BYTES_SHIFT	16
>> +#define  SFC_CMD_CS_SHIFT		30
>> +/* Address */
>> +#define SFC_ADDR			0x104
>> +/* Data */
>> +#define SFC_DATA			0x108
>> +
>> +#define SFC_MAX_CHIPSELECT_NUM		4
>> +#define SFC_DMA_MAX_LEN			0x4000
>> +#define SFC_CMD_DUMMY(x) \
>> +	((x) << SFC_CMD_DUMMY_SHIFT)
>> +
>> +enum rockchip_sfc_iftype {
>> +	IF_TYPE_STD,
>> +	IF_TYPE_DUAL,
>> +	IF_TYPE_QUAD,
>> +};
>> +
>> +struct rockchip_sfc;
>> +struct rockchip_sfc_chip_priv {
>> +	u8 cs;
>> +	u32 clk_rate;
>> +	struct spi_nor nor;
>> +	struct rockchip_sfc *sfc;
>> +};
>> +
>> +struct rockchip_sfc {
>> +	struct device *dev;
>> +	struct mutex lock;
>> +	void __iomem *regbase;
>> +	struct clk *hclk;
>> +	struct clk *clk;
>> +	/* virtual mapped addr for dma_buffer */
>> +	void *buffer;
>> +	dma_addr_t dma_buffer;
>> +	struct completion cp;
>> +	struct rockchip_sfc_chip_priv flash[SFC_MAX_CHIPSELECT_NUM];
>> +	u32 num_chip;
>> +	bool use_dma;
>> +	/* use negative edge of hclk to latch data */
>> +	bool negative_edge;
>> +};
>> +
>> +static int get_if_type(struct rockchip_sfc *sfc, enum read_mode flash_read)
>> +{
>> +	if (flash_read == SPI_NOR_DUAL)
>> +		return IF_TYPE_DUAL;
>> +	else if (flash_read == SPI_NOR_QUAD)
>> +		return IF_TYPE_QUAD;
>> +	else if (flash_read == SPI_NOR_NORMAL ||
>> +		 flash_read == SPI_NOR_FAST)
>> +		return IF_TYPE_STD;
>> +
>> +	dev_err(sfc->dev, "unsupported SPI read mode\n");
>> +	return -EINVAL;
>> +}
>> +
>> +static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
>> +{
>> +	int err;
>> +	u32 status;
>> +
>> +	writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
>> +
>> +	err = readl_poll_timeout(sfc->regbase + SFC_RCVR, status,
>> +				 !(status & SFC_RCVR_RESET), 20,
>> +				 jiffies_to_usecs(HZ));
>> +	if (err)
>> +		dev_err(sfc->dev, "SFC reset never finished\n");
>> +
>> +	/* Still need to clear the masked interrupt from RISR */
>> +	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 for sfc to latch data by using
>> +	 * ahb clock, and this configuration should be Soc
>> +	 * specific.
>> +	 */
>> +	if (sfc->negative_edge)
>> +		writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE,
>> +			       sfc->regbase + SFC_CTRL);
>> +	else
>> +		writel_relaxed(0, sfc->regbase + SFC_CTRL);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	int ret;
>> +
>> +	mutex_lock(&sfc->lock);
>> +	pm_runtime_get_sync(sfc->dev);
>> +
>> +	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_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +
>> +	clk_disable_unprepare(sfc->clk);
>> +	mutex_unlock(&sfc->lock);
>> +	pm_runtime_mark_last_busy(sfc->dev);
>> +	pm_runtime_put_autosuspend(sfc->dev);
>> +}
>> +
>> +static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc)
>> +{
>> +	int err;
>> +	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.
>> +	 */
>> +	err = readl_poll_timeout(sfc->regbase + SFC_FSR, status,
>> +				 status & SFC_FSR_TX_IS_EMPTY,
>> +				 20, jiffies_to_usecs(2 * HZ));
>> +	if (err)
>> +		dev_err(sfc->dev, "SFC fifo never empty\n");
>> +
>> +	return err;
>> +}
>> +
>> +static int rockchip_sfc_op_reg(struct spi_nor *nor,
>> +				u8 opcode, int len, u8 optype)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	u32 reg;
>> +	bool tx_no_empty, rx_no_empty, is_busy;
>> +	int err;
>> +
>> +	reg = readl_relaxed(sfc->regbase + SFC_FSR);
>> +	tx_no_empty = !(reg & SFC_FSR_TX_IS_EMPTY);
>> +	rx_no_empty = !(reg & SFC_FSR_RX_IS_EMPTY);
>> +
>> +	is_busy = readl_relaxed(sfc->regbase + SFC_SR);
>> +
>> +	if (tx_no_empty || rx_no_empty || is_busy) {
>> +		err = rockchip_sfc_reset(sfc);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>> +	reg = opcode << SFC_CMD_IDX_SHIFT;
>> +	reg |= len << SFC_CMD_TRAN_BYTES_SHIFT;
>> +	reg |= priv->cs << SFC_CMD_CS_SHIFT;
>> +	reg |= optype << SFC_CMD_DIR_SHIFT;
>> +
>> +	writel_relaxed(reg, sfc->regbase + SFC_CMD);
>> +
>> +	return rockchip_sfc_wait_op_finish(sfc);
>> +}
>> +
>> +static void rockchip_sfc_read_fifo(struct rockchip_sfc *sfc, u8 *buf, int len)
>> +{
>> +	u32 tmp, i;
>> +	int total_len = len;
>> +
>> +	/* 32-bit access only */
>> +	if (len >= 4 && !((u32)buf & 0x03)) {

	if (len >= sizeof(u32) && IS_ALIGNED(buf, sizeof(u32))) {
>> +		ioread32_rep(sfc->regbase + SFC_DATA, buf, len >> 2);
>> +		len %= 4;
>> +		buf += total_len - len;
>> +	}
>> +
>> +	/* read the rest bytes */
>> +	for (i = 0; i < len; i++) {
>> +		if (!(i & 0x03))
>> +			tmp = readl_relaxed(sfc->regbase + SFC_DATA);
>> +		buf[i] = (tmp >> ((i & 0x03) * 8)) & 0xff;
>> +	}
>> +}
>> +
>> +static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode,
>> +				 u8 *buf, int len)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	int ret;
>> +
>> +	ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD);
>> +	if (ret)
>> +		return ret;
>> +
>> +	rockchip_sfc_read_fifo(sfc, buf, len);
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode,
>> +				  u8 *buf, int len)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	u32 dwords;
>> +
>> +	/* Align bytes to dwords */
>> +	dwords = DIV_ROUND_UP(len, sizeof(u32));
>> +	iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords);

iowrite32_rep() is likely to be implemented with writesl() and writesl()
casts its "const void *buffer" argument into a "const u32 *buf" local
variable. Then this buf variable is used with __raw_writel(*buf++).

Hence if the u8 *buf argument of rockchip_sfc_write_reg() is not aligned to
32 bits, this function performs unaligned accesses to the memory pointed by
buf.

>> +
>> +	return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR);
>> +}
>> +
>> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor,
>> +					       loff_t from_to,
>> +					       size_t len, u8 op_type)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	u32 reg;
>> +	u8 if_type = 0;
>> +
>> +	if_type = get_if_type(sfc, 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 : 0),
>> +		       sfc->regbase + SFC_CTRL);
> 
> Marek rose an issue when commenting v3:
> 
> Looking at that code it seems that even if the hardware can support SPI
> 1-1-n protocols, this driver actually allows SPI n-n-n protocols.
> 
> However with the current spi-nor framework, the values of the enum
> read_mode must be understood this way:
> SPI_NOR_FAST or SPI_NOR_NORMAL: SPI 1-1-1 protocol
> SPI_NOR_DUAL: SPI 1-1-2 protocol
> SPI_NOR_QUAD: SPI 1-1-4 protocol
> 
> Support to other SPI protocols such as SPI 1-4-4 or SPI 4-4-4 as not been
> accepted in mainline yet.
> 
> Below in this driver, spi_nor_scan() is called with the value SPI_NOR_QUAD
> hence it asks the spi-nor framework for using the SPI 1-1-4 (and not SPI
> 4-4-4) *when supported by the QSPI memory*.
> 
> Also since the driver was tested with a Winbond w25q256 memory, let me warn
> you that currently the SPI_NOR_QUAD_READ flag is *NOT* set in the "w25q256"
> entry of the spi_nor_ids[] array. So this claims this memory is not capable
> of using the SPI 1-1-4 protocol even if the Winbond memory actually
> supports this protocol. Then the spi-nor framework selects the SPI 1-1-1
> protocol as a fallback.
> That's why you have succeeded in using your driver but it would have failed
> with another QSPI memory with its SPI_NOR_QUAD_READ flag due to a protocol
> mismatch.
> 
> So with the current spi-nor framework you must set
> SFC_CTRL_{DATA|ADDR}_BITS_SHIFT to IF_TYPE_STD.
> 
> Later, once the patch adding support to other SPI protocols would have been
> accepted in mainline, you could update your driver to tell the spi-nor
> framework that the rockchip controller also supports SPI 1-2-2 and SPI
> 1-4-4 protocols.
> 
> 
>> +
>> +	if (op_type == SFC_CMD_DIR_WR)
>> +		reg = nor->program_opcode << SFC_CMD_IDX_SHIFT;
>> +	else
>> +		reg = nor->read_opcode << SFC_CMD_IDX_SHIFT;
>> +
>> +	reg |= op_type << SFC_CMD_DIR_SHIFT;
>> +	reg |= (nor->addr_width == 4) ?
>> +		SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS;
>> +
>> +	reg |= priv->cs << SFC_CMD_CS_SHIFT;
>> +	reg |= len << SFC_CMD_TRAN_BYTES_SHIFT;
> 
> Looking at the definitions of SFC_CMD_TRAN_BYTES_SHIFT (16) and
> SFC_CMD_CS_SHIFT (30), I understand that the bitfield for the transfer
> length lays between bits 16 and 30 hence a 14 bit value at most.
> So what if len is greater than 16384? It overflows in the cs bitfield?
> 
> You should apply masks to avoid such overflows and also test the len value
> then report the actual number of transferred bytes.
> 
>> +
>> +	if (op_type == SFC_CMD_DIR_RD)
>> +		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);
>> +}
>> +
>> +static int rockchip_sfc_do_dma_transfer(struct spi_nor *nor, loff_t from_to,
>> +					dma_addr_t dma_buf, size_t len,
>> +					u8 op_type)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	u32 reg;
>> +	int err = 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);
>> +
>> +	rockchip_sfc_setup_transfer(nor, from_to, len, op_type);
>> +	writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR);
>> +
>> +	/*
>> +	 * Start dma but note that the sfc->dma_buffer is derived from
>> +	 * dmam_alloc_coherent so we don't actually need any sync operations
>> +	 * for coherent dma memory.
>> +	 */
>> +	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\n");
>> +		err = -ETIMEDOUT;
>> +	}
>> +
>> +	/* Disable transfer finish interrupt */
>> +	reg = readl_relaxed(sfc->regbase + SFC_IMR);
>> +	reg |= SFC_IMR_TRAN_FINISH;
>> +	writel_relaxed(reg, sfc->regbase + SFC_IMR);
>> +
>> +	if (err) {
>> +		rockchip_sfc_reset(sfc);
>> +		return err;
>> +	}
>> +
>> +	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 dwords;
>> +
>> +	/*
>> +	 * Align bytes to dwords, although we will write some extra
>> +	 * bytes to fifo but the transfer bytes number in SFC_CMD
>> +	 * register will make sure we just send out the expected
>> +	 * byte numbers and the extra bytes will be clean before
>> +	 * setting up the next transfer. We should always round up
>> +	 * to align to DWORD as the ahb for Rockchip Socs won't
>> +	 * support non-aligned-to-DWORD transfer.
>> +	 */
>> +	dwords = DIV_ROUND_UP(len, sizeof(u32));
>> +	iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords);
>> +

Here again the buf pointer might not be aligned to 4 bytes.

>> +	return rockchip_sfc_wait_op_finish(sfc);
>> +}
>> +
>> +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
>> +					size_t len)
>> +{
>> +	rockchip_sfc_read_fifo(sfc, buf, len);
>> +
>> +	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_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +
>> +	rockchip_sfc_setup_transfer(nor, from_to, len, op_type);
>> +
>> +	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 int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to,
>> +				     size_t len, u_char *buf, u8 op_type)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	size_t offset;
>> +	int ret;
>> +	dma_addr_t dma_addr = 0;
>> +	int dma_dir;
>> +
>> +	dma_dir = (op_type == SFC_CMD_DIR_RD) ?
>> +		   DMA_FROM_DEVICE : DMA_TO_DEVICE;
>> +
>> +	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 *)buf, trans, dma_dir);
> not good: buf may have been allocated with vmalloc() hence the pages of buf
> are not garanteed to be contiguous in physical memory.
> 
> Just write a ubifs image into your QSPI memory and try to mount it. You are
> very likely to notice some issues/crashes.
> 
>

This code can only work when:
- len <= SFC_DMA_MAX_LEN and (buf + len) points inside the very same page
as buf.
or
- dma_map_single() always fails so the driver always copy from/to buf data
to/from sfc->buffer and transfers from/to sfc->buffer_dma.

Otherwise the driver always tries to dma map from buf but buf is never
incremented by offset after a transfer. Hence the driver transfers the
right data during the first loop but starting from the 2nd loop it
transfers the very same data again and again till offset >= len.

I think you have forgotten a "+ offset" in the dma_map_single() call:

dma_map_single(NULL, (void *)buf + offset, trans, dma_dir);

               ^^ should be better to provide some dev, e.g. nor->dev

Anyway, like I've explained in my previous mail dma_map_single() is not
suited to buffers allocated by vmalloc().


>> +
>> +		if (dma_mapping_error(sfc->dev, dma_addr)) {
>> +			/*
>> +			 * If we use pre-allocated dma_buffer, we need to
>> +			 * do a copy here.
>> +			 */
>> +			if (op_type == SFC_CMD_DIR_WR)
>> +				memcpy(sfc->buffer, buf + offset, trans);
>> +
>> +			dma_addr = 0;
>> +		}
>> +
>> +		if (op_type == SFC_CMD_DIR_WR)
>> +			/*
>> +			 * Flush the write data from write_buf to dma_addr
>> +			 * if using dynamic allocated dma buffer before dma
>> +			 * moves data from dma_addr to fifo.
>> +			 */
>> +			dma_sync_single_for_device(sfc->dev, dma_addr,
>> +						   trans, DMA_TO_DEVICE);
>> +
>> +
>> +		/* If failing to map dma, use pre-allocated area instead */
>> +		ret = rockchip_sfc_do_dma_transfer(nor, from_to + offset,
>> +						dma_addr ? dma_addr :
>> +						sfc->dma_buffer,
>> +						trans, op_type);
>> +
>> +		if (dma_addr) {
>> +			/*
>> +			 * Invalidate the read data from dma_addr if using
>> +			 * dynamic allocated dma buffer after dma moves data
>> +			 * from fifo to dma_addr.
>> +			 */
>> +			if (op_type == SFC_CMD_DIR_RD)
>> +				dma_sync_single_for_cpu(sfc->dev, dma_addr,
>> +							trans, DMA_FROM_DEVICE);
>> +
>> +			dma_unmap_single(NULL, dma_addr,
>> +					 trans, dma_dir);
>> +		}
>> +
>> +		if (ret) {
>> +			dev_warn(nor->dev, "DMA read timeout\n");
>> +			return ret;
>> +		}
>> +		/*
>> +		 * If we use pre-allocated dma_buffer for read, we need to
>> +		 * do a copy here.
>> +		 */
>> +		if (!dma_addr && (op_type == SFC_CMD_DIR_RD))
>> +			memcpy(buf + offset, sfc->buffer, trans);
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t rockchip_sfc_do_rd_wr(struct spi_nor *nor, loff_t from_to,
>> +				     size_t len, u_char *buf, u32 op_type)
>> +{
>> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
>> +	struct rockchip_sfc *sfc = priv->sfc;
>> +	int ret;
>> +
>> +	if (likely(sfc->use_dma))
>> +		return rockchip_sfc_dma_transfer(nor, from_to, len,
>> +						 buf, op_type);
>> +
>> +	/* Fall back to PIO mode if DMA isn't present */
>> +	ret = rockchip_sfc_pio_transfer(nor, from_to, len,
>> +					(u_char *)buf, op_type);
>> +	if (ret) {
>> +		if (op_type == SFC_CMD_DIR_RD)
>> +			dev_warn(nor->dev, "PIO read timeout\n");
>> +		else
>> +			dev_warn(nor->dev, "PIO write timeout\n");
>> +		return ret;
>> +	}
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from,
>> +				 size_t len, u_char *read_buf)
>> +{
>> +	return rockchip_sfc_do_rd_wr(nor, from, len,
>> +				     read_buf, SFC_CMD_DIR_RD);
>> +}
>> +
>> +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
>> +				  size_t len, const u_char *write_buf)
>> +{
>> +	return rockchip_sfc_do_rd_wr(nor, to, len,
>> +				     (u_char *)write_buf,
>> +				     SFC_CMD_DIR_WR);
>> +}
>> +
>> +static int rockchip_sfc_register(struct device_node *np,
>> +				 struct rockchip_sfc *sfc)
>> +{
>> +	struct device *dev = sfc->dev;
>> +	struct mtd_info *mtd;
>> +	struct spi_nor *nor;
>> +	int ret;
>> +
>> +	nor = &sfc->flash[sfc->num_chip].nor;
>> +	nor->dev = dev;
>> +	spi_nor_set_flash_node(nor, np);
>> +
>> +	ret = of_property_read_u8(np, "reg", &sfc->flash[sfc->num_chip].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",
>> +			&sfc->flash[sfc->num_chip].clk_rate);
>> +	if (ret) {
>> +		dev_err(dev, "No spi-max-frequency property for %s\n",
>> +			np->full_name);
>> +		return ret;
>> +	}
>> +
>> +	sfc->flash[sfc->num_chip].sfc = sfc;
>> +	nor->priv = &(sfc->flash[sfc->num_chip]);
>> +
>> +	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->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->flash[i].nor.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_CHIPSELECT_NUM) {
>> +			dev_warn(dev, "Exceeds the max cs limitation\n");
>> +			break;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +fail:
>> +	dev_err(dev, "Failed to register all chips\n");
>> +	/* Unregister all the _registered_ nor flash */
>> +	rockchip_sfc_unregister_all(sfc);
>> +	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_RISR_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;
>> +	}
>> +
>> +	sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
>> +					      "rockchip,sfc-no-DMA");
>> +
>> +	sfc->negative_edge = of_device_is_compatible(sfc->dev->of_node,
>> +						     "rockchip,rk1108-sfc");
>> +	/* 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;
>> +	}
>> +
>> +	sfc->num_chip = 0;
>> +	ret = rockchip_sfc_init(sfc);
>> +	if (ret)
>> +		goto err_irq;
>> +
>> +	pm_runtime_get_noresume(&pdev->dev);
>> +	pm_runtime_set_active(&pdev->dev);
>> +	pm_runtime_enable(&pdev->dev);
>> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
>> +	pm_runtime_use_autosuspend(&pdev->dev);
>> +
>> +	ret = rockchip_sfc_register_all(sfc);
>> +	if (ret)
>> +		goto err_register;
>> +
>> +	clk_disable_unprepare(sfc->clk);
>> +	pm_runtime_put_autosuspend(&pdev->dev);
>> +	return 0;
>> +
>> +err_register:
>> +	pm_runtime_disable(&pdev->dev);
>> +	pm_runtime_set_suspended(&pdev->dev);
>> +	pm_runtime_put_noidle(&pdev->dev);
>> +err_irq:
>> +	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);
>> +
>> +	pm_runtime_get_sync(&pdev->dev);
>> +	pm_runtime_disable(&pdev->dev);
>> +	pm_runtime_put_noidle(&pdev->dev);
>> +
>> +	rockchip_sfc_unregister_all(sfc);
>> +	mutex_destroy(&sfc->lock);
>> +	clk_disable_unprepare(sfc->clk);
>> +	clk_disable_unprepare(sfc->hclk);
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +int rockchip_sfc_runtime_suspend(struct device *dev)
>> +{
>> +	struct rockchip_sfc *sfc = dev_get_drvdata(dev);
>> +
>> +	clk_disable_unprepare(sfc->hclk);
>> +	return 0;
>> +}
>> +
>> +int rockchip_sfc_runtime_resume(struct device *dev)
>> +{
>> +	struct rockchip_sfc *sfc = dev_get_drvdata(dev);
>> +
>> +	clk_prepare_enable(sfc->hclk);
>> +	return rockchip_sfc_reset(sfc);
>> +}
>> +#endif /* CONFIG_PM */
>> +
>> +static const struct of_device_id rockchip_sfc_dt_ids[] = {
>> +	{ .compatible = "rockchip,sfc"},
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
>> +
>> +static const struct dev_pm_ops rockchip_sfc_dev_pm_ops = {
>> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +				pm_runtime_force_resume)
>> +	SET_RUNTIME_PM_OPS(rockchip_sfc_runtime_suspend,
>> +			   rockchip_sfc_runtime_resume, NULL)
>> +};
>> +
>> +static struct platform_driver rockchip_sfc_driver = {
>> +	.driver = {
>> +		.name	= "rockchip-sfc",
>> +		.of_match_table = rockchip_sfc_dt_ids,
>> +		.pm = &rockchip_sfc_dev_pm_ops,
>> +	},
>> +	.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("Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>");
>>
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

--
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] reset: zx2967: add reset controller driver for ZTE's zx2967 family
From: Jun Nie @ 2017-01-16 14:28 UTC (permalink / raw)
  To: Baoyou Xie, p.zabel, robh+dt, mark.rutland
  Cc: devicetree, xie.baoyou, linux-kernel, chen.chaokai, wang.qiang01,
	shawnguo, linux-arm-kernel
In-Reply-To: <1484564194-18530-3-git-send-email-baoyou.xie@linaro.org>

On 2017年01月16日 18:56, Baoyou Xie wrote:
> This patch adds reset controller driver for ZTE's zx2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
>  drivers/reset/Kconfig        |   6 +++
>  drivers/reset/Makefile       |   1 +
>  drivers/reset/reset-zx2967.c | 125 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 132 insertions(+)
>  create mode 100644 drivers/reset/reset-zx2967.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 172dc96..f4cdfe9 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -86,6 +86,12 @@ config RESET_UNIPHIER
>  	  Say Y if you want to control reset signals provided by System Control
>  	  block, Media I/O block, Peripheral Block.
>
> +config RESET_ZX2967
> +	bool "ZTE ZX2967 Reset Driver"
> +	depends on ARCH_ZX || COMPILE_TEST
> +	help
> +	  This enables the reset controller driver for ZTE's zx2967 family.
> +
>  config RESET_ZYNQ
>  	bool "ZYNQ Reset Driver" if COMPILE_TEST
>  	default ARCH_ZYNQ
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 13b346e..2cd3f6c 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -13,4 +13,5 @@ obj-$(CONFIG_RESET_STM32) += reset-stm32.o
>  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> +obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
>  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
> new file mode 100644
> index 0000000..bc95261
> --- /dev/null
> +++ b/drivers/reset/reset-zx2967.c
> @@ -0,0 +1,125 @@
> +/*
> + * ZTE's zx2967 family reset controller driver
> + *
> + * Copyright (C) 2017 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +
> +struct zx2967_reset {
> +	void __iomem			*reg_base;
> +	spinlock_t			lock;
> +	struct reset_controller_dev	rcdev;
> +};
> +
> +static int zx2967_reset_act(struct reset_controller_dev *rcdev,
> +			    unsigned long id, bool assert)
> +{
> +	struct zx2967_reset *reset = NULL;
> +	u32 bank = id / 32;
> +	u32 offset = id % 32;
> +	u32 reg;
> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));

readl_relaxed is recommended.

> +	if (assert)
> +		reg &= ~BIT(offset);
> +	else
> +		reg |= BIT(offset);
> +	writel(reg, reset->reg_base + (bank * 4));

writel_relaxed is recommended.

> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	return zx2967_reset_act(rcdev, id, true);
> +}
> +
> +static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	return zx2967_reset_act(rcdev, id, false);
> +}
> +
> +static struct reset_control_ops zx2967_reset_ops = {
> +	.assert		= zx2967_reset_assert,
> +	.deassert	= zx2967_reset_deassert,
> +};
> +
> +static int zx2967_reset_probe(struct platform_device *pdev)
> +{
> +	struct zx2967_reset *reset;
> +	struct resource *res;
> +
> +	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> +	if (!reset)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(reset->reg_base))
> +		return PTR_ERR(reset->reg_base);
> +
> +	spin_lock_init(&reset->lock);
> +
> +	reset->rcdev.owner = THIS_MODULE;
> +	reset->rcdev.nr_resets = resource_size(res) * 8;
> +	reset->rcdev.ops = &zx2967_reset_ops;
> +	reset->rcdev.of_node = pdev->dev.of_node;
> +
> +	dev_info(&pdev->dev, "reset controller cnt:%d",
> +		  reset->rcdev.nr_resets);
> +
> +	return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> +}
> +
> +static int zx2967_reset_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +
> +static const struct of_device_id zx2967_reset_dt_ids[] = {
> +	 { .compatible = "zte,zx296718-reset", },
> +	 {},
> +};
> +MODULE_DEVICE_TABLE(of, zx2967_reset_dt_ids);
> +
> +static struct platform_driver zx2967_reset_driver = {
> +	.probe	= zx2967_reset_probe,
> +	.remove	= zx2967_reset_remove,
> +	.driver = {
> +		.name		= "zx2967-reset",
> +		.of_match_table	= zx2967_reset_dt_ids,
> +	},
> +};

This line can replace all below code if you do not have any other 
dependency in earlier stage.

builtin_platform_driver(zx2967_reset_driver);

> +
> +static int __init zx2967_reset_init(void)
> +{
> +	return platform_driver_register(&zx2967_reset_driver);
> +}
> +arch_initcall(zx2967_reset_init);
> +
> +static void __exit zx2967_reset_exit(void)
> +{
> +	platform_driver_unregister(&zx2967_reset_driver);
> +}
> +module_exit(zx2967_reset_exit);
> +
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
> +MODULE_DESCRIPTION("ZTE zx2967 Reset Controller Driver");
> +MODULE_LICENSE("GPL");
>


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

^ permalink raw reply

* Re: [PATCH 03/10] devicetree: bindings: add bindings for ahci-da850
From: Bartosz Golaszewski @ 2017-01-16 14:30 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: David Lechner, Kevin Hilman, Patrick Titiano, Michael Turquette,
	Tejun Heo, Rob Herring, Mark Rutland, Russell King,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, linux-devicetree, LKML, arm-soc
In-Reply-To: <d8406f53-b97d-ca9a-f7d1-94ce5b7064f7-l0cyMroinI0@public.gmane.org>

2017-01-16 13:45 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>:
> On Monday 16 January 2017 03:43 PM, Bartosz Golaszewski wrote:
>> 2017-01-13 20:25 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>:
>>>
>>> A clock multiplier property seems redundant if you are specifying a clock.
>>> It should be possible to get the rate from the clock to determine which
>>> multiplier is needed.
>>>
>>
>> I probably should have named it differently. This is not a multiplier
>> of a clock derived from PLL0 or PLL1. Instead it's a value set by
>> writing to the Port PHY Control Register (MPY bits) of the SATA
>> controller that configures the multiplier for the external low-jitter
>> clock. On the lcdk the signals (REFCLKP, REFCLKN) are provided by
>> CDCM61001 (SATA OSCILLATOR component on the schematics).
>>
>> I'll find a better name and comment the property accordingly.
>>
>> FYI: the da850 platform does not use the common clock framework, so I
>> don't specify the clock property on the sata node in the device tree.
>> Instead I add the clock lookup entry in patch [01/10]. This is
>> transparent for AHCI which can get the clock as usual by calling
>> clk_get() in ahci_platform_get_resources().
>
> I think David's point is that the SATA_REFCLK needs to be modeled as a
> actual clock input to the IP. You should be able to get the rate using
> clk_get_rate() and make the MPY bits calculation depending on the
> incoming rate.
>
> You should be able to model the clock even when not using common clock
> framework.
>
> DA850 AHCI does not use a con_id at the moment (it assumes a single
> clock), and that needs to change.
>

It's true that once davinci gets ported (is this planned?) to using
the common clock framework, we could just create a fixed-clock node in
da850-lcdk for the SATA oscillator, so the new property is redundant.

What I don't get is how should I model a clock that is not
configurable and is board-specific? Is hard-coding the relevant rate
in da850.c with a huge FIXME the right way?

Thanks,
Bartosz Golaszewski
--
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: [PATCHv3 8/8] ARM: configs: stm32: Add RTC support in STM32 defconfig
From: Alexandre Torgue @ 2017-01-16 14:37 UTC (permalink / raw)
  To: Amelie Delaunay, Alessandro Zummo, Alexandre Belloni, Rob Herring,
	Mark Rutland, Maxime Coquelin, Russell King
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gabriel Fernandez
In-Reply-To: <1483623809-29937-9-git-send-email-amelie.delaunay-qxv4g6HH51o@public.gmane.org>

Hi Amélie,

On 01/05/2017 02:43 PM, Amelie Delaunay wrote:
> This patch adds STM32 RTC support in stm32_defconfig file.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> ---
>  arch/arm/configs/stm32_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
> index be19e09..0acff9e 100644
> --- a/arch/arm/configs/stm32_defconfig
> +++ b/arch/arm/configs/stm32_defconfig
> @@ -57,6 +57,8 @@ CONFIG_LEDS_CLASS=y
>  CONFIG_LEDS_GPIO=y
>  CONFIG_LEDS_TRIGGERS=y
>  CONFIG_LEDS_TRIGGER_HEARTBEAT=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_STM32=y
>  CONFIG_DMADEVICES=y
>  CONFIG_STM32_DMA=y
>  # CONFIG_FILE_LOCKING is not set
>
Applied on stm32-defconfig-for-v4.11

thanks
Alex

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 2/3] MAINTAINERS: add zx2967 reset controller driver to ARM ZTE architecture
From: Philipp Zabel @ 2017-01-16 14:38 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: jun.nie-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1484564194-18530-2-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, 2017-01-16 at 18:56 +0800, Baoyou Xie wrote:
> Add the zx2967 reset controller driver as maintained by ARM ZTE
> architecture maintainers, as they're parts of the core IP.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2793808..08f8155 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1980,10 +1980,12 @@ L:	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org (moderated for non-subscribers)
>  S:	Maintained
>  F:	arch/arm/mach-zx/
>  F:	drivers/clk/zte/
> +F:	drivers/reset/reset-zx2967.c
>  F:	drivers/soc/zte/
>  F:	drivers/thermal/zx*
>  F:	Documentation/devicetree/bindings/arm/zte.txt
>  F:	Documentation/devicetree/bindings/clock/zx296702-clk.txt
> +F:	Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
>  F:	Documentation/devicetree/bindings/soc/zte/
>  F:	Documentation/devicetree/bindings/thermal/zx*
>  F:	include/dt-bindings/soc/zx*.h

This patch doesn't apply on top of reset/next. I can rebase it, but it
will cause a trivial merge conflict with the soc/thermal additions.

regards
Philipp

--
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: [PATCHv3 RESEND 1/8] ARM: dts: stm32: set HSE_RTC clock frequency to 1 MHz on stm32f429
From: Alexandre Torgue @ 2017-01-16 14:39 UTC (permalink / raw)
  To: Amelie Delaunay, Rob Herring, Mark Rutland, Russell King,
	Maxime Coquelin
  Cc: devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1484573388-590-1-git-send-email-amelie.delaunay@st.com>

Hi Amélie,

On 01/16/2017 02:29 PM, Amelie Delaunay wrote:
> This patch set HSE_RTC clock frequency to 1 MHz, as the clock supplied to
> the RTC must be 1 MHz.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index e9324a3..557c1f4 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -433,6 +433,8 @@
>  			reg = <0x40023800 0x400>;
>  			clocks = <&clk_hse>, <&clk_i2s_ckin>;
>  			st,syscfg = <&pwrcfg>;
> +			assigned-clocks = <&rcc 1 CLK_HSE_RTC>;
> +			assigned-clock-rates = <1000000>;
>  		};
>
>  		dma1: dma-controller@40026000 {
>

Applied on stm32-dt-for-v4.11

Thanks
Alex

^ permalink raw reply

* Re: [PATCH 2/3] dt-bindings: mtd: add a common label property to all mtd devices
From: Cédric Le Goater @ 2017-01-16 14:39 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Marek Vasut, Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <20170116145258.17707912@bbrezillon>

On 01/16/2017 02:52 PM, Boris Brezillon wrote:
> On Mon, 16 Jan 2017 14:27:04 +0100
> Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:
> 
>> This can be used to easily identify a specific chip on a system with
>> multiple chips.
>>
>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mtd/common.txt | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/common.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
>> new file mode 100644
>> index 000000000000..a781641a3b00
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/common.txt
>> @@ -0,0 +1,12 @@
>> +* Common properties of all MTD devices
>> +
>> +Optional properties:
>> +- label : name to assign to mtd. If omitted, the label is the MTD device name.
> 
> Hm, the description is not really clear. How about:
> 
> "
> - label: user-defined MTD device name. Can be used to assign user
>   friendly names to MTD devices (instead of the flash model or flash
>   controller based name) in order to ease flash device identification
>   and/or describe what they are used for.
> "

yes. I will use that in a v2.
 
Thanks,

C. 

> Otherwise,
> 
> Acked-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Thanks,
> 
> Boris
> 
>> +
>> +Example:
>> +
>> +	flash@0 {
>> +		label = "System-firmware";
>> +
>> +		/* flash type specific properties */
>> +	};
> 

--
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: [PATCHv3 RESEND 4/8] ARM: dts: stm32: Add RTC support for STM32F429 MCU
From: Alexandre Torgue @ 2017-01-16 14:39 UTC (permalink / raw)
  To: Amelie Delaunay, Rob Herring, Mark Rutland, Russell King,
	Maxime Coquelin
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484573388-590-2-git-send-email-amelie.delaunay-qxv4g6HH51o@public.gmane.org>



On 01/16/2017 02:29 PM, Amelie Delaunay wrote:
> This patch adds STM32 RTC bindings for STM32F429.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index 557c1f4..f05a9d9 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -125,6 +125,20 @@
>  			status = "disabled";
>  		};
>
> +		rtc: rtc@40002800 {
> +			compatible = "st,stm32-rtc";
> +			reg = <0x40002800 0x400>;
> +			clocks = <&rcc 1 CLK_RTC>;
> +			clock-names = "ck_rtc";
> +			assigned-clocks = <&rcc 1 CLK_RTC>;
> +			assigned-clock-parents = <&rcc 1 CLK_LSE>;
> +			interrupt-parent = <&exti>;
> +			interrupts = <17 1>;
> +			interrupt-names = "alarm";
> +			st,syscfg = <&pwrcfg>;
> +			status = "disabled";
> +		};
> +
>  		usart2: serial@40004400 {
>  			compatible = "st,stm32-usart", "st,stm32-uart";
>  			reg = <0x40004400 0x400>;
>
Applied on stm32-dt-for-v4.11

Thanks
Alex
--
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: [PATCHv3 RESEND 5/8] ARM: dts: stm32: enable RTC on stm32f429-disco
From: Alexandre Torgue @ 2017-01-16 14:39 UTC (permalink / raw)
  To: Amelie Delaunay, Rob Herring, Mark Rutland, Russell King,
	Maxime Coquelin
  Cc: devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1484573388-590-3-git-send-email-amelie.delaunay@st.com>



On 01/16/2017 02:29 PM, Amelie Delaunay wrote:
> This patch enables RTC on stm32f429-disco with LSI as clock source because
> X2 crystal for LSE is not fitted by default.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
>  arch/arm/boot/dts/stm32f429-disco.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts
> index 7d0415e..9222b9f 100644
> --- a/arch/arm/boot/dts/stm32f429-disco.dts
> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
> @@ -94,6 +94,12 @@
>  	clock-frequency = <8000000>;
>  };
>
> +&rtc {
> +	assigned-clocks = <&rcc 1 CLK_RTC>;
> +	assigned-clock-parents = <&rcc 1 CLK_LSI>;
> +	status = "okay";
> +};
> +
>  &usart1 {
>  	pinctrl-0 = <&usart1_pins_a>;
>  	pinctrl-names = "default";
>
Applied on stm32-dt-for-v4.11

Thanks
Alex

^ permalink raw reply

* Re: [PATCHv3 RESEND 6/8] ARM: dts: stm32: enable RTC on stm32f469-disco
From: Alexandre Torgue @ 2017-01-16 14:39 UTC (permalink / raw)
  To: Amelie Delaunay, Rob Herring, Mark Rutland, Russell King,
	Maxime Coquelin
  Cc: devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1484573388-590-4-git-send-email-amelie.delaunay@st.com>



On 01/16/2017 02:29 PM, Amelie Delaunay wrote:
> This patch enables RTC on stm32f469-disco with default LSE clock source.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
>  arch/arm/boot/dts/stm32f469-disco.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index fda12a4..f52b9f6 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -78,6 +78,10 @@
>  	clock-frequency = <8000000>;
>  };
>
> +&rtc {
> +	status = "okay";
> +};
> +
>  &usart3 {
>  	pinctrl-0 = <&usart3_pins_a>;
>  	pinctrl-names = "default";
>
Applied on stm32-dt-for-v4.11

Thanks
Alex

^ permalink raw reply

* Re: [PATCHv3 RESEND 7/8] ARM: dts: stm32: enable RTC on stm32429i-eval
From: Alexandre Torgue @ 2017-01-16 14:40 UTC (permalink / raw)
  To: Amelie Delaunay, Rob Herring, Mark Rutland, Russell King,
	Maxime Coquelin
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484573388-590-5-git-send-email-amelie.delaunay-qxv4g6HH51o@public.gmane.org>



On 01/16/2017 02:29 PM, Amelie Delaunay wrote:
> This patch enables RTC on stm32429i-eval with default LSE clock source.
>
> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> ---
>  arch/arm/boot/dts/stm32429i-eval.dts | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
> index 2181220..eedb27d 100644
> --- a/arch/arm/boot/dts/stm32429i-eval.dts
> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
> @@ -157,6 +157,10 @@
>  	};
>  };
>
> +&rtc {
> +	status = "okay";
> +};
> +
>  &usart1 {
>  	pinctrl-0 = <&usart1_pins_a>;
>  	pinctrl-names = "default";
>
Applied on stm32-dt-for-v4.11

Thanks
Alex
--
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] reset: zx2967: add reset controller driver for ZTE's zx2967 family
From: Philipp Zabel @ 2017-01-16 14:43 UTC (permalink / raw)
  To: Jun Nie
  Cc: Baoyou Xie, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <95ebb6fd-540b-f6d5-e039-2cd718ff35d8-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, 2017-01-16 at 22:28 +0800, Jun Nie wrote:
> On 2017年01月16日 18:56, Baoyou Xie wrote:
> > This patch adds reset controller driver for ZTE's zx2967 family.
> >
> > Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> >  drivers/reset/Kconfig        |   6 +++
> >  drivers/reset/Makefile       |   1 +
> >  drivers/reset/reset-zx2967.c | 125 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 132 insertions(+)
> >  create mode 100644 drivers/reset/reset-zx2967.c
> >
> > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> > index 172dc96..f4cdfe9 100644
> > --- a/drivers/reset/Kconfig
> > +++ b/drivers/reset/Kconfig
> > @@ -86,6 +86,12 @@ config RESET_UNIPHIER
> >  	  Say Y if you want to control reset signals provided by System Control
> >  	  block, Media I/O block, Peripheral Block.
> >
> > +config RESET_ZX2967
> > +	bool "ZTE ZX2967 Reset Driver"
> > +	depends on ARCH_ZX || COMPILE_TEST
> > +	help
> > +	  This enables the reset controller driver for ZTE's zx2967 family.
> > +
> >  config RESET_ZYNQ
> >  	bool "ZYNQ Reset Driver" if COMPILE_TEST
> >  	default ARCH_ZYNQ
> > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> > index 13b346e..2cd3f6c 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -13,4 +13,5 @@ obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> >  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> >  obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
> >  obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> > +obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
> >  obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> > diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
> > new file mode 100644
> > index 0000000..bc95261
> > --- /dev/null
> > +++ b/drivers/reset/reset-zx2967.c
> > @@ -0,0 +1,125 @@
> > +/*
> > + * ZTE's zx2967 family reset controller driver
> > + *
> > + * Copyright (C) 2017 ZTE Ltd.
> > + *
> > + * Author: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > + *
> > + * License terms: GNU General Public License (GPL) version 2
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset-controller.h>
> > +
> > +struct zx2967_reset {
> > +	void __iomem			*reg_base;
> > +	spinlock_t			lock;
> > +	struct reset_controller_dev	rcdev;
> > +};
> > +
> > +static int zx2967_reset_act(struct reset_controller_dev *rcdev,
> > +			    unsigned long id, bool assert)
> > +{
> > +	struct zx2967_reset *reset = NULL;
> > +	u32 bank = id / 32;
> > +	u32 offset = id % 32;

Not terribly important, but int for bank and offset was just fine.

> > +	u32 reg;
> > +	unsigned long flags;
> > +
> > +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> > +
> > +	spin_lock_irqsave(&reset->lock, flags);
> > +
> > +	reg = readl(reset->reg_base + (bank * 4));
> 
> readl_relaxed is recommended.
> 
> > +	if (assert)
> > +		reg &= ~BIT(offset);
> > +	else
> > +		reg |= BIT(offset);
> > +	writel(reg, reset->reg_base + (bank * 4));
> 
> writel_relaxed is recommended.

Why? Resets are usually not performance critical.

> > +
> > +	spin_unlock_irqrestore(&reset->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> > +			       unsigned long id)
> > +{
> > +	return zx2967_reset_act(rcdev, id, true);
> > +}
> > +
> > +static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> > +				 unsigned long id)
> > +{
> > +	return zx2967_reset_act(rcdev, id, false);
> > +}
> > +
> > +static struct reset_control_ops zx2967_reset_ops = {
> > +	.assert		= zx2967_reset_assert,
> > +	.deassert	= zx2967_reset_deassert,
> > +};
> > +
> > +static int zx2967_reset_probe(struct platform_device *pdev)
> > +{
> > +	struct zx2967_reset *reset;
> > +	struct resource *res;
> > +
> > +	reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
> > +	if (!reset)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(reset->reg_base))
> > +		return PTR_ERR(reset->reg_base);
> > +
> > +	spin_lock_init(&reset->lock);
> > +
> > +	reset->rcdev.owner = THIS_MODULE;
> > +	reset->rcdev.nr_resets = resource_size(res) * 8;
> > +	reset->rcdev.ops = &zx2967_reset_ops;
> > +	reset->rcdev.of_node = pdev->dev.of_node;
> > +
> > +	dev_info(&pdev->dev, "reset controller cnt:%d",
> > +		  reset->rcdev.nr_resets);

This is a bit noisy. Can we just drop it?

> > +	return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
> > +}
> > +
> > +static int zx2967_reset_remove(struct platform_device *pdev)
> > +{
> > +	return 0;
> > +}

Remove this function.

> > +static const struct of_device_id zx2967_reset_dt_ids[] = {
> > +	 { .compatible = "zte,zx296718-reset", },
> > +	 {},
> > +};
> > +MODULE_DEVICE_TABLE(of, zx2967_reset_dt_ids);
> > +
> > +static struct platform_driver zx2967_reset_driver = {
> > +	.probe	= zx2967_reset_probe,
> > +	.remove	= zx2967_reset_remove,

Remove the above line.

> > +	.driver = {
> > +		.name		= "zx2967-reset",
> > +		.of_match_table	= zx2967_reset_dt_ids,
> > +	},
> > +};
> 
> This line can replace all below code if you do not have any other 
> dependency in earlier stage.
> 
> builtin_platform_driver(zx2967_reset_driver);
> 
> > +
> > +static int __init zx2967_reset_init(void)
> > +{
> > +	return platform_driver_register(&zx2967_reset_driver);
> > +}
> > +arch_initcall(zx2967_reset_init);
> > +
> > +static void __exit zx2967_reset_exit(void)
> > +{
> > +	platform_driver_unregister(&zx2967_reset_driver);
> > +}
> > +module_exit(zx2967_reset_exit);
> > +
> > +MODULE_AUTHOR("Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
> > +MODULE_DESCRIPTION("ZTE zx2967 Reset Controller Driver");
> > +MODULE_LICENSE("GPL");
> >

regards
Philipp

--
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 v2 2/3] dt-bindings: mtd: add a common label property to all mtd devices
From: Cédric Le Goater @ 2017-01-16 14:56 UTC (permalink / raw)
  To: linux-mtd
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Marek Vasut, Rob Herring, Cédric Le Goater, Cyrille Pitchen,
	Brian Norris, David Woodhouse
In-Reply-To: <1484573225-19095-3-git-send-email-clg@kaod.org>

This can be used to easily identify a specific chip on a system with
multiple chips.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 Documentation/devicetree/bindings/mtd/common.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/common.txt

diff --git a/Documentation/devicetree/bindings/mtd/common.txt b/Documentation/devicetree/bindings/mtd/common.txt
new file mode 100644
index 000000000000..fc068b923d7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/common.txt
@@ -0,0 +1,15 @@
+* Common properties of all MTD devices
+
+Optional properties:
+- label: user-defined MTD device name. Can be used to assign user
+  friendly names to MTD devices (instead of the flash model or flash
+  controller based name) in order to ease flash device identification
+  and/or describe what they are used for.
+
+Example:
+
+	flash@0 {
+		label = "System-firmware";
+
+		/* flash type specific properties */
+	};
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related


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