Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH RESEND 2/2] gpio: axp209: add pinctrl support
From: Maxime Ripard @ 2016-11-25 15:17 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: mark.rutland, gnurou, devicetree, linus.walleij, linux-kernel,
	linux-gpio, wens, robh+dt, linux-arm-kernel
In-Reply-To: <20161123141151.25315-3-quentin.schulz@free-electrons.com>


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

Hi,

On Wed, Nov 23, 2016 at 03:11:51PM +0100, Quentin Schulz wrote:
> The GPIOs present in the AXP209 PMIC have multiple functions. They
> typically allow a pin to be used as GPIO input or output and can also be
> used as ADC or regulator for example.[1]
> 
> This adds the possibility to use all functions of the GPIOs present in
> the AXP209 PMIC thanks to pinctrl subsystem.
> 
> [1] see registers 90H, 92H and 93H at
>     http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

I've said it already face to face, but ideally you should split that
patch into logical changes.

I can see here at least three:
  - Adding the pinctrl features
  - Renaming the structure and functions
  - Removal of a few functions

Maxime

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

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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 v2] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
From: Marek Vasut @ 2016-11-25 15:12 UTC (permalink / raw)
  To: Cédric Le Goater, Joel Stanley
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Boris Brezillon, Richard Weinberger,
	Cyrille Pitchen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland
In-Reply-To: <b9d8ba33-2bb9-cb93-d833-89ddd8cfd7f9-Bxea+6Xhats@public.gmane.org>

On 11/25/2016 04:01 PM, Cédric Le Goater wrote:
> Hello Marek,

Hi!

> Sorry for the late answer. Here are a couple of answers to the naming 
> problem 
> 
> On 11/25/2016 02:57 PM, Marek Vasut wrote:
>> On 11/21/2016 05:45 AM, Joel Stanley wrote:
>>> Hello Marek,
>>
>> Hi!
>>
>>> Thank you for the review. I have answered a few of your questions;
>>> I'll leave the rest to Cedric.
>>>
>>> On Mon, Nov 21, 2016 at 8:13 AM, Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>>>>> index 4a682ee0f632..96148600fdab 100644
>>>>> --- a/drivers/mtd/spi-nor/Kconfig
>>>>> +++ b/drivers/mtd/spi-nor/Kconfig
>>>>> @@ -76,4 +76,16 @@ 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 ASPEED_FLASH_SPI
>>>>
>>>> Should be SPI_ASPEED , see the other controllers and keep the list sorted.
>>>
>>> Perhaps SPI_NOR_ASPEED so it's clear it's not a driver for a generic SPI bus?
>>
>> But it's not a driver for SPI-NOR only either, it seems it's a driver
>> for multiple distinct devices.
> 
> yes and I think it was a mistake to send the whole at once. We have 
> added the support in qemu controller by controller and it was easier 
> to understand. I need to split the patchset in the next version. 

Cool :-)

>>>>> +     tristate "Aspeed flash controllers in SPI mode"
>>>>> +     depends on HAS_IOMEM && OF
>>>>> +     depends on ARCH_ASPEED || COMPILE_TEST
>>>>> +     # IO_SPACE_LIMIT must be equivalent to (~0UL)
>>>>> +     depends on !NEED_MACH_IO_H
>>>>
>>>> Why?
>>>>
>>>>> +     help
>>>>> +       This enables support for the New Static Memory Controller
>>>>> +       (FMC) in the Aspeed SoCs (AST2400 and AST2500) when attached
>>>>> +       to SPI nor chips, and support for the SPI Memory controller
>>>>> +       (SPI) for the BIOS.
>>>>
>>>> I think there is a naming chaos between FMC, SMC (as in Static MC) and
>>>> SMC (as in SPI MC).
>>>
>>> Yes, you're spot on. This naming chaos comes form the vendor's documentation.
>>>
>>> I think we could re-work this sentence to make it clearer.
>>
>> Please do before someone's head explodes from this :)
> 
> Indeed .. :) I will give a try. Here is the status :
> 
> Aspeed SoC AST2400 has a set of SMC (Static Memory Controller) 
> controllers in which you find :
> 
> - Legacy Static Memory Controller (called SMC in the spec)  
>   . base address at 0x16000000
>   . BMC firmware
>   . old register set
>   . supports NOR flash, NAND flash and SPI flash memory. All bootable.
>   . 1 chip select pin (CE0)
> 
> - New Static Memory Controller (called FMC in the spec)
>   . base address at 0x16200000
>   . BMC firmware
>   . new register set
>   . supports NOR flash, NAND flash and SPI flash memory. 
>   . 5 chip select pins (CE0 ∼ CE4)
> 
> - SPI Flash Controller (called SPI in the spec)  
>   . base address at 0x16300000
>   . host Firmware
>   . exotic register set, between old and new ...
>   . supports SPI flash memory
>   . 1 chip select pin (CE0)

This should be (except for the base address) be in some documentation,
it helps.

> Aspeed SoC AST2500 defines has a similar set of SMC (Static Memory 
> Controller) controllers, more in the vein of the AST2400 FMC  :
> 
> - Legacy Static Memory Controller is gone, NOR and NAND support also
> 
> - Firmware SPI Memory Controller (called FMC in the spec)  
>   . base address at 0x16200000
>   . BMC firmware
>   . new register set
>   . supports SPI flash memory.
>   . 3 chip select pins (CE0 ~ CE2)
> 
> - SPI Flash Controller (called SPI1 in the spec) first
>   . base address at 0x16300000
>   . host firmware
>   . new register set
>   . supports SPI flash memory.
>   . 2 chip select pins (CE0 ~ CE1)
> 
> - SPI Flash Controller (called SPI2 in the spec) second
>   . base address at 0x16310000
>   . host firmware
>   . new register set
>   . supports SPI flash memory.
>   . 2 chip select pins (CE0 ~ CE1)
> 
> 
> So, these are the reasons behind the naming mess. Added to that the 
> driver considers the acronym SMC to stand for SPI Memory Controller, 
> which is wrong. I tried to reduce the confusion with some comments but 
> that was a failure :)

The explanation above is awesome though.

> In qemu, we have used FMC (Firmware ...) and SPI to name the controllers 
> and we just dropped the legacy SMC. I think using the same naming scheme
> is a good idea. We don't support anything else than SPI either so we can 
> drop the other types for the moment.

One thing which I still ponder about is how do you support those
controllers which support NOR and NAND flash and SPI, do you tap
into all subsystems ?

>>>>> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
>>>>> +                                 size_t len)
>>>>> +{
>>>>
>>>> What if start of buf is unaligned ?
>>>>
>>>>> +     if ((((unsigned long)src | (unsigned long)buf | len) & 3) == 0) {
>>>>
>>>> Uh, should use boolean OR, not bitwise or. Also, if you're testing
>>>> pointer for NULL, do if (!ptr) .
>>>>
>>>> if (!src || !buf || !len)
>>>>    return;
>>>
>>> That's a different test. We're testing here that the buffers are
>>> aligned to see if we can do a word-at-a-time copy.
>>>
>>> If not, it falls through to do a byte-at-a-time copy. I think this
>>> covers your first question about buf being unaligned.
>>
>> Ah, I see, thanks for clarifying. Comment in the code would be helpful
>> for why what you're doing is OK. And I think you want to cast to
>> uintptr_t instead to make this work on 64bit.
> 
> yes
> 
>>> Cedric, perhaps you could create a macro called IS_ALLIGNED to make it
>>> clear what this is doing?
>>
>> Yup, thanks!
> 
> sure. I still need to go through Marek's comments in the initial email, 
> I will split the pachset and fix the naming in next version.

Thanks!

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

^ permalink raw reply

* Re: [PATCH v2 0/7] ath9k: EEPROM swapping improvements
From: Valo, Kalle @ 2016-11-25 15:06 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: ath9k-devel,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath9k-devel-xDcbHBWguxHbcTqmT+pZeQ@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org,
	nbd-Vt+b4OUoWG0@public.gmane.org
In-Reply-To: <87insxg0yc.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>

Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> writes:

> Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> writes:
>
>> There are two types of swapping the EEPROM data in the ath9k driver.
>> Before this series one type of swapping could not be used without the
>> other.
>>
>> The first type of swapping looks at the "magic bytes" at the start of
>> the EEPROM data and performs swab16 on the EEPROM contents if needed.
>> The second type of swapping is EEPROM format specific and swaps
>> specific fields within the EEPROM itself (swab16, swab32 - depends on
>> the EEPROM format).
>>
>> With this series the second part now looks at the EEPMISC register
>> inside the EEPROM, which uses a bit to indicate if the EEPROM data
>> is Big Endian (this is also done by the FreeBSD kernel).
>> This has a nice advantage: currently there are some out-of-tree hacks
>> (in OpenWrt and LEDE) where the EEPROM has a Big Endian header on a
>> Big Endian system (= no swab16 is performed) but the EEPROM itself
>> indicates that it's data is Little Endian. Until now the out-of-tree
>> code simply did a swab16 before passing the data to ath9k, so ath9k
>> first did the swab16 - this also enabled the format specific swapping.
>> These out-of-tree hacks are still working with the new logic, but it
>> is recommended to remove them. This implementation is based on a
>> discussion with Arnd Bergmann who raised concerns about the
>> robustness and portability of the swapping logic in the original OF
>> support patch review, see [0].
>>
>> After a second round of patches (= v1 of this series) neither Arnd
>> Bergmann nor I were really happy with the complexity of the EEPROM
>> swapping logic. Based on a discussion (see [1] and [2]) we decided
>> that ath9k should use a defined format (specifying the endianness
>> of the data - I went with __le16 and __le32) when accessing the
>> EEPROM fields. A benefit of this is that we enable the EEPMISC based
>> swapping logic by default, just like the FreeBSD driver, see [3]. On
>> the devices which I have tested (see below) ath9k now works without
>> having to specify the "endian_check" field in ath9k_platform_data (or
>> a similar logic which could provide this via devicetree) as ath9k now
>> detects the endianness automatically. Only EEPROMs which are mangled
>> by some out-of-tree code still need the endian_check flag (or one can
>> simply remove that mangling from the out-of-tree code).
>>
>> Testing:
>> - tested by myself on AR9287 with Big Endian EEPROM
>> - tested by myself on AR9227 with Little Endian EEPROM
>> - tested by myself on AR9381 (using the ar9003_eeprom implementation,
>>   which did not suffer from this whole problem)
>> - how do we proceed with testing? maybe we could keep this in a
>>   feature-branch and add these patches to LEDE once we have an ACK to
>>   get more people to test this
>>
>> This series depends on my other series (v7):
>> "add devicetree support to ath9k" - see [4]
>
> I think this looks pretty good. If there's a bug somewhere it should be
> quite easy to fix so I'm not that worried and would be willing to take
> these as soon as I have applied the dependency series. IIRC your
> devicetree patches will have at least one more review round so that will
> take some time still. In the meantime it would be great if LEDE folks
> could take a look at these and comment (or test).

So are everyone happy with this? I haven't seen any comments. If I don't
here anything I'm planning to take these, most likely for 4.11.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH v2] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
From: Cédric Le Goater @ 2016-11-25 15:01 UTC (permalink / raw)
  To: Marek Vasut, Joel Stanley
  Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
	Rob Herring, linux-mtd, Cyrille Pitchen, Brian Norris,
	David Woodhouse
In-Reply-To: <9569aaca-5748-e82d-20e1-ed846fb762f6@gmail.com>

Hello Marek,

Sorry for the late answer. Here are a couple of answers to the naming 
problem 

On 11/25/2016 02:57 PM, Marek Vasut wrote:
> On 11/21/2016 05:45 AM, Joel Stanley wrote:
>> Hello Marek,
> 
> Hi!
> 
>> Thank you for the review. I have answered a few of your questions;
>> I'll leave the rest to Cedric.
>>
>> On Mon, Nov 21, 2016 at 8:13 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>>>> index 4a682ee0f632..96148600fdab 100644
>>>> --- a/drivers/mtd/spi-nor/Kconfig
>>>> +++ b/drivers/mtd/spi-nor/Kconfig
>>>> @@ -76,4 +76,16 @@ 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 ASPEED_FLASH_SPI
>>>
>>> Should be SPI_ASPEED , see the other controllers and keep the list sorted.
>>
>> Perhaps SPI_NOR_ASPEED so it's clear it's not a driver for a generic SPI bus?
> 
> But it's not a driver for SPI-NOR only either, it seems it's a driver
> for multiple distinct devices.

yes and I think it was a mistake to send the whole at once. We have 
added the support in qemu controller by controller and it was easier 
to understand. I need to split the patchset in the next version. 

>>>> +     tristate "Aspeed flash controllers in SPI mode"
>>>> +     depends on HAS_IOMEM && OF
>>>> +     depends on ARCH_ASPEED || COMPILE_TEST
>>>> +     # IO_SPACE_LIMIT must be equivalent to (~0UL)
>>>> +     depends on !NEED_MACH_IO_H
>>>
>>> Why?
>>>
>>>> +     help
>>>> +       This enables support for the New Static Memory Controller
>>>> +       (FMC) in the Aspeed SoCs (AST2400 and AST2500) when attached
>>>> +       to SPI nor chips, and support for the SPI Memory controller
>>>> +       (SPI) for the BIOS.
>>>
>>> I think there is a naming chaos between FMC, SMC (as in Static MC) and
>>> SMC (as in SPI MC).
>>
>> Yes, you're spot on. This naming chaos comes form the vendor's documentation.
>>
>> I think we could re-work this sentence to make it clearer.
> 
> Please do before someone's head explodes from this :)

Indeed .. :) I will give a try. Here is the status :

Aspeed SoC AST2400 has a set of SMC (Static Memory Controller) 
controllers in which you find :

- Legacy Static Memory Controller (called SMC in the spec)  
  . base address at 0x16000000
  . BMC firmware
  . old register set
  . supports NOR flash, NAND flash and SPI flash memory. All bootable.
  . 1 chip select pin (CE0)

- New Static Memory Controller (called FMC in the spec)
  . base address at 0x16200000
  . BMC firmware
  . new register set
  . supports NOR flash, NAND flash and SPI flash memory. 
  . 5 chip select pins (CE0 ∼ CE4)

- SPI Flash Controller (called SPI in the spec)  
  . base address at 0x16300000
  . host Firmware
  . exotic register set, between old and new ...
  . supports SPI flash memory
  . 1 chip select pin (CE0)


Aspeed SoC AST2500 defines has a similar set of SMC (Static Memory 
Controller) controllers, more in the vein of the AST2400 FMC  :

- Legacy Static Memory Controller is gone, NOR and NAND support also

- Firmware SPI Memory Controller (called FMC in the spec)  
  . base address at 0x16200000
  . BMC firmware
  . new register set
  . supports SPI flash memory.
  . 3 chip select pins (CE0 ~ CE2)

- SPI Flash Controller (called SPI1 in the spec) first
  . base address at 0x16300000
  . host firmware
  . new register set
  . supports SPI flash memory.
  . 2 chip select pins (CE0 ~ CE1)

- SPI Flash Controller (called SPI2 in the spec) second
  . base address at 0x16310000
  . host firmware
  . new register set
  . supports SPI flash memory.
  . 2 chip select pins (CE0 ~ CE1)


So, these are the reasons behind the naming mess. Added to that the 
driver considers the acronym SMC to stand for SPI Memory Controller, 
which is wrong. I tried to reduce the confusion with some comments but 
that was a failure :)

In qemu, we have used FMC (Firmware ...) and SPI to name the controllers 
and we just dropped the legacy SMC. I think using the same naming scheme
is a good idea. We don't support anything else than SPI either so we can 
drop the other types for the moment.

>>>> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
>>>> +                                 size_t len)
>>>> +{
>>>
>>> What if start of buf is unaligned ?
>>>
>>>> +     if ((((unsigned long)src | (unsigned long)buf | len) & 3) == 0) {
>>>
>>> Uh, should use boolean OR, not bitwise or. Also, if you're testing
>>> pointer for NULL, do if (!ptr) .
>>>
>>> if (!src || !buf || !len)
>>>    return;
>>
>> That's a different test. We're testing here that the buffers are
>> aligned to see if we can do a word-at-a-time copy.
>>
>> If not, it falls through to do a byte-at-a-time copy. I think this
>> covers your first question about buf being unaligned.
> 
> Ah, I see, thanks for clarifying. Comment in the code would be helpful
> for why what you're doing is OK. And I think you want to cast to
> uintptr_t instead to make this work on 64bit.

yes

>> Cedric, perhaps you could create a macro called IS_ALLIGNED to make it
>> clear what this is doing?
> 
> Yup, thanks!

sure. I still need to go through Marek's comments in the initial email, 
I will split the pachset and fix the naming in next version.

Thanks,

C. 
 
>>>
>>> while (...)
>>>
>>>> +             while (len > 3) {
>>>> +                     *(u32 *)buf = readl(src);
>>>> +                     buf += 4;
>>>> +                     src += 4;
>>>> +                     len -= 4;
>>>> +             }
>>>> +     }
>>>> +
>>>> +     while (len--) {
>>>> +             *(u8 *)buf = readb(src);
>>>> +             buf += 1;
>>>> +             src += 1;
>>>> +     }
>>>> +     return 0;
>>>> +}
>>>> +/*
>>>> + * SPI Flash Configuration Register (AST2400 SPI)
>>>> + */
>>>> +#define CONFIG_REG                   0x0
>>>> +#define    CONFIG_ENABLE_CE_INACTIVE     BIT(1)
>>>> +#define    CONFIG_WRITE                          BIT(0)
>>>
>>> #define[space]FOO[tab]BIT(bar)
>>
>> These are bits within the CONFIG_REG. It follows the same style as
>> other spi-nor drivers, eg. nxp-spifi.
>>
>> I think it's somewhat clearer, but if you have a strong preference
>> against then fair enough.
> 
> It triggers my OCD, but I think it's a matter of taste, so I don't care
> that much.
> 


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

^ permalink raw reply

* [PATCH v6 2/2] DW DMAC: add multi-block property to device tree
From: Eugeniy Paltsev @ 2016-11-25 14:59 UTC (permalink / raw)
  To: devicetree
  Cc: mark.rutland, linux-snps-arc, christian.ruppert, arnd, vinod.koul,
	vireshk, linux-kernel, Eugeniy Paltsev, robh+dt, dmaengine,
	andriy.shevchenko, shiraz.linux.kernel
In-Reply-To: <1480085947-31273-1-git-send-email-Eugeniy.Paltsev@synopsys.com>

Several versions of DW DMAC have multi block transfers hardware
support. Hardware support of multi block transfers is disabled
by default if we use DT to configure DMAC and software emulation
of multi block transfers used instead.
Add multi-block property, so it is possible to enable hardware
multi block transfers (if present) via DT.

Switch from per device is_nollp variable to multi_block array
to be able enable/disable multi block transfers separately per
channel.

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
Also:
   Update DT documentation.
   Update existing platform data and DTS.

 Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
 arch/arc/boot/dts/abilis_tb10x.dtsi                |  1 +
 arch/arm/boot/dts/spear13xx.dtsi                   |  2 ++
 drivers/dma/dw/core.c                              |  2 +-
 drivers/dma/dw/platform.c                          | 12 +++++++++++-
 drivers/dma/dw/regs.h                              |  3 ++-
 drivers/tty/serial/8250/8250_lpss.c                |  2 +-
 include/linux/platform_data/dma-dw.h               |  5 +++--
 8 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt b/Documentation/devicetree/bindings/dma/snps-dma.txt
index 0f55832..4775c66f 100644
--- a/Documentation/devicetree/bindings/dma/snps-dma.txt
+++ b/Documentation/devicetree/bindings/dma/snps-dma.txt
@@ -27,6 +27,8 @@ Optional properties:
   that services interrupts for this device
 - is_private: The device channels should be marked as private and not for by the
   general purpose DMA channel allocator. False if not passed.
+- multi-block: Multi block transfers supported by hardware. Array property with
+  one cell per channel. 0: not supported, 1 (default): supported.
 
 Example:
 
diff --git a/arch/arc/boot/dts/abilis_tb10x.dtsi b/arch/arc/boot/dts/abilis_tb10x.dtsi
index de53f5c..3121536 100644
--- a/arch/arc/boot/dts/abilis_tb10x.dtsi
+++ b/arch/arc/boot/dts/abilis_tb10x.dtsi
@@ -129,6 +129,7 @@
 			data-width = <4>;
 			clocks = <&ahb_clk>;
 			clock-names = "hclk";
+			multi-block = <1 1 1 1 1 1>;
 		};
 
 		i2c0: i2c@FF120000 {
diff --git a/arch/arm/boot/dts/spear13xx.dtsi b/arch/arm/boot/dts/spear13xx.dtsi
index 449acf0..17ea0ab 100644
--- a/arch/arm/boot/dts/spear13xx.dtsi
+++ b/arch/arm/boot/dts/spear13xx.dtsi
@@ -118,6 +118,7 @@
 			block_size = <0xfff>;
 			dma-masters = <2>;
 			data-width = <8 8>;
+			multi-block = <1 1 1 1 1 1 1 1>;
 		};
 
 		dma@eb000000 {
@@ -134,6 +135,7 @@
 			chan_priority = <1>;
 			block_size = <0xfff>;
 			data-width = <8 8>;
+			multi-block = <1 1 1 1 1 1 1 1>;
 		};
 
 		fsmc: flash@b0000000 {
diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index c2c0a61..e5adf5d 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -1569,7 +1569,7 @@ int dw_dma_probe(struct dw_dma_chip *chip)
 				(dwc_params >> DWC_PARAMS_MBLK_EN & 0x1) == 0;
 		} else {
 			dwc->block_size = pdata->block_size;
-			dwc->nollp = pdata->is_nollp;
+			dwc->nollp = !pdata->multi_block[i];
 		}
 	}
 
diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index aa7a5c1..b1655e4 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -102,7 +102,7 @@ dw_dma_parse_dt(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct dw_dma_platform_data *pdata;
-	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS];
+	u32 tmp, arr[DW_DMA_MAX_NR_MASTERS], mb[DW_DMA_MAX_NR_CHANNELS];
 	u32 nr_masters;
 	u32 nr_channels;
 
@@ -118,6 +118,8 @@ dw_dma_parse_dt(struct platform_device *pdev)
 
 	if (of_property_read_u32(np, "dma-channels", &nr_channels))
 		return NULL;
+	if (nr_channels > DW_DMA_MAX_NR_CHANNELS)
+		return NULL;
 
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
@@ -152,6 +154,14 @@ dw_dma_parse_dt(struct platform_device *pdev)
 			pdata->data_width[tmp] = BIT(arr[tmp] & 0x07);
 	}
 
+	if (!of_property_read_u32_array(np, "multi-block", mb, nr_channels)) {
+		for (tmp = 0; tmp < nr_channels; tmp++)
+			pdata->multi_block[tmp] = mb[tmp];
+	} else {
+		for (tmp = 0; tmp < nr_channels; tmp++)
+			pdata->multi_block[tmp] = 1;
+	}
+
 	return pdata;
 }
 #else
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index f65dd10..4e0128c 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -12,7 +12,8 @@
 #include <linux/interrupt.h>
 #include <linux/dmaengine.h>
 
-#define DW_DMA_MAX_NR_CHANNELS	8
+#include "internal.h"
+
 #define DW_DMA_MAX_NR_REQUESTS	16
 
 /* flow controller */
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index f607946..58cbb30 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -157,12 +157,12 @@ static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port)
 static const struct dw_dma_platform_data qrk_serial_dma_pdata = {
 	.nr_channels = 2,
 	.is_private = true,
-	.is_nollp = true,
 	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
 	.chan_priority = CHAN_PRIORITY_ASCENDING,
 	.block_size = 4095,
 	.nr_masters = 1,
 	.data_width = {4},
+	.multi_block = {0},
 };
 
 static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
diff --git a/include/linux/platform_data/dma-dw.h b/include/linux/platform_data/dma-dw.h
index 5f0e11e..e69e415 100644
--- a/include/linux/platform_data/dma-dw.h
+++ b/include/linux/platform_data/dma-dw.h
@@ -14,6 +14,7 @@
 #include <linux/device.h>
 
 #define DW_DMA_MAX_NR_MASTERS	4
+#define DW_DMA_MAX_NR_CHANNELS	8
 
 /**
  * struct dw_dma_slave - Controller-specific information about a slave
@@ -40,19 +41,18 @@ struct dw_dma_slave {
  * @is_private: The device channels should be marked as private and not for
  *	by the general purpose DMA channel allocator.
  * @is_memcpy: The device channels do support memory-to-memory transfers.
- * @is_nollp: The device channels does not support multi block transfers.
  * @chan_allocation_order: Allocate channels starting from 0 or 7
  * @chan_priority: Set channel priority increasing from 0 to 7 or 7 to 0.
  * @block_size: Maximum block size supported by the controller
  * @nr_masters: Number of AHB masters supported by the controller
  * @data_width: Maximum data width supported by hardware per AHB master
  *		(in bytes, power of 2)
+ * @multi_block: Multi block transfers supported by hardware per channel.
  */
 struct dw_dma_platform_data {
 	unsigned int	nr_channels;
 	bool		is_private;
 	bool		is_memcpy;
-	bool		is_nollp;
 #define CHAN_ALLOCATION_ASCENDING	0	/* zero to seven */
 #define CHAN_ALLOCATION_DESCENDING	1	/* seven to zero */
 	unsigned char	chan_allocation_order;
@@ -62,6 +62,7 @@ struct dw_dma_platform_data {
 	unsigned int	block_size;
 	unsigned char	nr_masters;
 	unsigned char	data_width[DW_DMA_MAX_NR_MASTERS];
+	unsigned char	multi_block[DW_DMA_MAX_NR_CHANNELS];
 };
 
 #endif /* _PLATFORM_DATA_DMA_DW_H */
-- 
2.5.5

^ permalink raw reply related

* [PATCH v6 1/2] DW DMAC: enable memory-to-memory transfers support
From: Eugeniy Paltsev @ 2016-11-25 14:59 UTC (permalink / raw)
  To: devicetree
  Cc: mark.rutland, linux-snps-arc, christian.ruppert, arnd, vinod.koul,
	vireshk, linux-kernel, Eugeniy Paltsev, robh+dt, dmaengine,
	andriy.shevchenko, shiraz.linux.kernel
In-Reply-To: <1480085947-31273-1-git-send-email-Eugeniy.Paltsev@synopsys.com>

All known devices, which use DT for configuration, support
memory-to-memory transfers. So enable it by default, if we read
configuration from DT.

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 drivers/dma/dw/platform.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 5bda0eb..aa7a5c1 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev)
 	if (of_property_read_bool(np, "is_private"))
 		pdata->is_private = true;
 
+	/*
+	 * All known devices, which use DT for configuration, support
+	 * memory-to-memory transfers. So enable it by default.
+	 */
+	pdata->is_memcpy = true;
+
 	if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
 		pdata->chan_allocation_order = (unsigned char)tmp;
 
-- 
2.5.5

^ permalink raw reply related

* [PATCH v6 0/2] DW DMAC: update device tree
From: Eugeniy Paltsev @ 2016-11-25 14:59 UTC (permalink / raw)
  To: devicetree
  Cc: mark.rutland, linux-snps-arc, christian.ruppert, arnd, vinod.koul,
	vireshk, linux-kernel, Eugeniy Paltsev, robh+dt, dmaengine,
	andriy.shevchenko, shiraz.linux.kernel

It wasn't possible to enable some features like
memory-to-memory transfers or multi block transfers via DT.
It is fixed by these patches.

Changes for v6:
 * Use "supported" as default state for "multi-block" property,
   to keep old DTBs working. Pointed by Andy Shevchenko. 

Changes for v5:
 * Update existing DTS. Pointed by Andy Shevchenko. 
 * Read multi-block per chanel instead of per master (Move 
   DW_DMA_MAX_NR_CHANNELS define from regs.h to dma-dw.h to
   implement this) Pointed by Andy Shevchenko.

Changes for v4:
 * Fix setting inverted value to "dwc->nollp". My fault - I 
   tested with wrong DTS, so DMAC was configured from autoconfig
   instead of device tree. Pointed by Andy Shevchenko.
 * Update "multi-block" diescription in documentation to be more 
   clear. Pointed by Arnd Bergmann.

Changes for v3:
 * Update existing platform data.
   We don't need to update existing DTS because default logic 
   wasn't change: we don't set "is_nollp" if we read 
   configuration from DT before. And we don't set it now if
   "multi-block" property doesn't exist in DTS.

Changes for v2:
 * I thought about is_memcpy DT property: all known devices, which 
   use DT for configuration, support memory-to-memory transfers. 
   So we don't need to read it from DT. So enable it by default, 
   if we read configuration from DT.
 * Use "multi-block" instead of "hw-llp" name to be more clear.
 * Move adding DT property and adding documentation for this
   property to one patch.

Eugeniy Paltsev (2):
  DW DMAC: enable memory-to-memory transfers support
  DW DMAC: add multi-block property to device tree

 Documentation/devicetree/bindings/dma/snps-dma.txt |  2 ++
 arch/arc/boot/dts/abilis_tb10x.dtsi                |  1 +
 arch/arm/boot/dts/spear13xx.dtsi                   |  2 ++
 drivers/dma/dw/core.c                              |  2 +-
 drivers/dma/dw/platform.c                          | 18 +++++++++++++++++-
 drivers/dma/dw/regs.h                              |  3 ++-
 drivers/tty/serial/8250/8250_lpss.c                |  2 +-
 include/linux/platform_data/dma-dw.h               |  5 +++--
 8 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.5.5

^ permalink raw reply

* [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
From: Todor Tomov @ 2016-11-25 14:56 UTC (permalink / raw)
  To: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
	s.nawrocki, linux-media, linux-kernel, robh+dt, mark.rutland,
	devicetree
  Cc: bjorn.andersson, srinivas.kandagatla, Todor Tomov

Add DT binding document for Qualcomm Camera subsystem driver.

Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
---
 .../devicetree/bindings/media/qcom,camss.txt       | 196 +++++++++++++++++++++
 1 file changed, 196 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt

diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
new file mode 100644
index 0000000..76ad89a
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
@@ -0,0 +1,196 @@
+Qualcomm Camera Subsystem
+
+* Properties
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain:
+		- "qcom,8x16-camss"
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Register ranges as listed in the reg-names property.
+- reg-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "csiphy0"
+		- "csiphy0_clk_mux"
+		- "csiphy1"
+		- "csiphy1_clk_mux"
+		- "csid0"
+		- "csid1"
+		- "ispif"
+		- "csi_clk_mux"
+		- "vfe0"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Interrupts as listed in the interrupt-names property.
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "csiphy0"
+		- "csiphy1"
+		- "csid0"
+		- "csid1"
+		- "ispif"
+		- "vfe0"
+- power-domains:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A phandle and power domain specifier pairs to the
+		    power domain which is responsible for collapsing
+		    and restoring power to the peripheral.
+- clocks:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A list of phandle and clock specifier pairs as listed
+		    in clock-names property.
+- clock-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries:
+		- "camss_top_ahb_clk"
+		- "ispif_ahb_clk"
+		- "csiphy0_timer_clk"
+		- "csiphy1_timer_clk"
+		- "csi0_ahb_clk"
+		- "csi0_clk"
+		- "csi0_phy_clk"
+		- "csi0_pix_clk"
+		- "csi0_rdi_clk"
+		- "csi1_ahb_clk"
+		- "csi1_clk"
+		- "csi1_phy_clk"
+		- "csi1_pix_clk"
+		- "csi1_rdi_clk"
+		- "camss_ahb_clk"
+		- "camss_vfe_vfe_clk"
+		- "camss_csi_vfe_clk"
+		- "iface_clk"
+		- "bus_clk"
+- vdda-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: A phandle to voltage supply for CSI2.
+- iommus:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A list of phandle and IOMMU specifier pairs.
+
+* Nodes
+
+- ports:
+	Usage: required
+	Definition: As described in video-interfaces.txt in same directory.
+	Properties:
+		- reg:
+			Usage: required
+			Value type: <u32>
+			Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
+	Endpoint node properties:
+		- clock-lanes:
+			Usage: required
+			Value type: <u32>
+			Definition: The clock lane.
+		- data-lanes:
+			Usage: required
+			Value type: <prop-encoded-array>
+			Definition: An array of data lanes.
+		- qcom,settle-cnt:
+			Usage: required
+			Value type: <u32>
+			Definition: The settle count parameter for CSI PHY.
+
+* An Example
+
+	camss: camss@1b00000 {
+		compatible = "qcom,8x16-camss";
+		reg = <0x1b0ac00 0x200>,
+			<0x1b00030 0x4>,
+			<0x1b0b000 0x200>,
+			<0x1b00038 0x4>,
+			<0x1b08000 0x100>,
+			<0x1b08400 0x100>,
+			<0x1b0a000 0x500>,
+			<0x1b00020 0x10>,
+			<0x1b10000 0x1000>;
+		reg-names = "csiphy0",
+			"csiphy0_clk_mux",
+			"csiphy1",
+			"csiphy1_clk_mux",
+			"csid0",
+			"csid1",
+			"ispif",
+			"csi_clk_mux",
+			"vfe0";
+		interrupts = <GIC_SPI 78 0>,
+			<GIC_SPI 79 0>,
+			<GIC_SPI 51 0>,
+			<GIC_SPI 52 0>,
+			<GIC_SPI 55 0>,
+			<GIC_SPI 57 0>;
+		interrupt-names = "csiphy0",
+			"csiphy1",
+			"csid0",
+			"csid1",
+			"ispif",
+			"vfe0";
+		power-domains = <&gcc VFE_GDSC>;
+		clocks = <&gcc GCC_CAMSS_TOP_AHB_CLK>,
+			<&gcc GCC_CAMSS_ISPIF_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI0PHYTIMER_CLK>,
+			<&gcc GCC_CAMSS_CSI1PHYTIMER_CLK>,
+			<&gcc GCC_CAMSS_CSI0_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI0_CLK>,
+			<&gcc GCC_CAMSS_CSI0PHY_CLK>,
+			<&gcc GCC_CAMSS_CSI0PIX_CLK>,
+			<&gcc GCC_CAMSS_CSI0RDI_CLK>,
+			<&gcc GCC_CAMSS_CSI1_AHB_CLK>,
+			<&gcc GCC_CAMSS_CSI1_CLK>,
+			<&gcc GCC_CAMSS_CSI1PHY_CLK>,
+			<&gcc GCC_CAMSS_CSI1PIX_CLK>,
+			<&gcc GCC_CAMSS_CSI1RDI_CLK>,
+			<&gcc GCC_CAMSS_AHB_CLK>,
+			<&gcc GCC_CAMSS_VFE0_CLK>,
+			<&gcc GCC_CAMSS_CSI_VFE0_CLK>,
+			<&gcc GCC_CAMSS_VFE_AHB_CLK>,
+			<&gcc GCC_CAMSS_VFE_AXI_CLK>;
+		clock-names = "camss_top_ahb_clk",
+			"ispif_ahb_clk",
+			"csiphy0_timer_clk",
+			"csiphy1_timer_clk",
+			"csi0_ahb_clk",
+			"csi0_clk",
+			"csi0_phy_clk",
+			"csi0_pix_clk",
+			"csi0_rdi_clk",
+			"csi1_ahb_clk",
+			"csi1_clk",
+			"csi1_phy_clk",
+			"csi1_pix_clk",
+			"csi1_rdi_clk",
+			"camss_ahb_clk",
+			"camss_vfe_vfe_clk",
+			"camss_csi_vfe_clk",
+			"iface_clk",
+			"bus_clk";
+		vdda-supply = <&pm8916_l2>;
+		iommus = <&apps_iommu 3>;
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			port@0 {
+				reg = <0>;
+				csiphy0_ep: endpoint {
+					clock-lanes = <1>;
+					data-lanes = <0 2>;
+					qcom,settle-cnt = <0xe>;
+					remote-endpoint = <&ov5645_ep>;
+				};
+			};
+		};
+	};
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/2] usb: ohci: s3c2410: allow probing from device tree
From: Sergio Prado @ 2016-11-25 14:47 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland, stern, kgene, krzk, javier,
	linux-usb, devicetree, linux-kernel, linux-arm-kernel,
	linux-samsung-soc
  Cc: Sergio Prado
In-Reply-To: <1480085249-25014-1-git-send-email-sergio.prado@e-labworks.com>

Allows configuring Samsung's s3c2410 USB OHCI controller using a
devicetree.

Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
 drivers/usb/host/ohci-s3c2410.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c
index 7a1919ca543a..d8e03a801f2e 100644
--- a/drivers/usb/host/ohci-s3c2410.c
+++ b/drivers/usb/host/ohci-s3c2410.c
@@ -457,6 +457,13 @@ static int ohci_hcd_s3c2410_drv_resume(struct device *dev)
 	.resume		= ohci_hcd_s3c2410_drv_resume,
 };
 
+static const struct of_device_id ohci_hcd_s3c2410_dt_ids[] = {
+	{ .compatible = "samsung,s3c2410-ohci" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, ohci_hcd_s3c2410_dt_ids);
+
 static struct platform_driver ohci_hcd_s3c2410_driver = {
 	.probe		= ohci_hcd_s3c2410_drv_probe,
 	.remove		= ohci_hcd_s3c2410_drv_remove,
@@ -464,6 +471,7 @@ static int ohci_hcd_s3c2410_drv_resume(struct device *dev)
 	.driver		= {
 		.name	= "s3c2410-ohci",
 		.pm	= &ohci_hcd_s3c2410_pm_ops,
+		.of_match_table	= ohci_hcd_s3c2410_dt_ids,
 	},
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller
From: Sergio Prado @ 2016-11-25 14:47 UTC (permalink / raw)
  To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
	kgene-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
	javier-JPH+aEBZ4P+UEJcrhfAQsw, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
  Cc: Sergio Prado
In-Reply-To: <1480085249-25014-1-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>

Adds the device tree bindings description for Samsung S3C2410 and
compatible USB OHCI controller.

Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
---
 .../devicetree/bindings/usb/s3c2410-usb.txt        | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/s3c2410-usb.txt

diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
new file mode 100644
index 000000000000..e45b38ce2986
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
@@ -0,0 +1,22 @@
+Samsung S3C2410 and compatible SoC USB controller
+
+OHCI
+
+Required properties:
+ - compatible: should be "samsung,s3c2410-ohci" for USB host controller
+ - reg: address and lenght of the controller memory mapped region
+ - interrupts: interrupt number for the USB OHCI controller
+ - clocks: Should reference the bus and host clocks
+ - clock-names: Should contain two strings
+		"usb-bus-host" for the USB bus clock
+		"usb-host" for the USB host clock
+
+Example:
+
+usb0: ohci@49000000 {
+	compatible = "samsung,s3c2410-ohci";
+	reg = <0x49000000 0x100>;
+	interrupts = <0 0 26 3>;
+	clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
+	clock-names = "usb-bus-host", "usb-host";
+};
-- 
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

* [PATCH 0/2] usb: ohci: s3c2410: add device tree support
From: Sergio Prado @ 2016-11-25 14:47 UTC (permalink / raw)
  To: gregkh, robh+dt, mark.rutland, stern, kgene, krzk, javier,
	linux-usb, devicetree, linux-kernel, linux-arm-kernel,
	linux-samsung-soc
  Cc: Sergio Prado

This series adds support for configuring Samsung's s3c2410 and
compatible USB OHCI controller via devicetree.

Tested on FriendlyARM mini2440, based on s3c2440 SoC.

Sergio Prado (2):
  dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller
  usb: ohci: s3c2410: allow probing from device tree

 .../devicetree/bindings/usb/s3c2410-usb.txt        | 22 ++++++++++++++++++++++
 drivers/usb/host/ohci-s3c2410.c                    |  8 ++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/s3c2410-usb.txt

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH v5 2/2] ARM: dts: add support for Turris Omnia
From: Andrew Lunn @ 2016-11-25 14:32 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jason Cooper, Gregory Clement, Sebastian Hesselbarth,
	Tomas Hlavacek, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Bed??icha Ko??atu
In-Reply-To: <20161125142658.21690-3-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>

On Fri, Nov 25, 2016 at 03:26:58PM +0100, Uwe Kleine-König wrote:
> This machine is an open hardware router by cz.nic driven by a
> Marvell Armada 385.
> 
> Signed-off-by: Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

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

^ permalink raw reply

* [PATCH v5 2/2] ARM: dts: add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-25 14:26 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
  Cc: Mark Rutland, devicetree, Tomas Hlavacek, Rob Herring,
	linux-arm-kernel, Bedřicha Košatu
In-Reply-To: <20161125142658.21690-1-uwe@kleine-koenig.org>

This machine is an open hardware router by cz.nic driven by a
Marvell Armada 385.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
 arch/arm/boot/dts/Makefile                    |   1 +
 arch/arm/boot/dts/armada-385-turris-omnia.dts | 334 ++++++++++++++++++++++++++
 2 files changed, 335 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-385-turris-omnia.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index befcd2619902..f1d3b9ff257e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -920,6 +920,7 @@ dtb-$(CONFIG_MACH_ARMADA_38X) += \
 	armada-385-db-ap.dtb \
 	armada-385-linksys-caiman.dtb \
 	armada-385-linksys-cobra.dtb \
+	armada-385-turris-omnia.dtb \
 	armada-388-clearfog.dtb \
 	armada-388-db.dtb \
 	armada-388-gp.dtb \
diff --git a/arch/arm/boot/dts/armada-385-turris-omnia.dts b/arch/arm/boot/dts/armada-385-turris-omnia.dts
new file mode 100644
index 000000000000..bcc10c285889
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
@@ -0,0 +1,334 @@
+/*
+ * Device Tree file for the Turris Omnia
+ *
+ * Copyright (C) 2016 Uwe Kleine-König <uwe@kleine-koenig.org>
+ * Copyright (C) 2016 Tomas Hlavacek <tmshlvkc@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is licensed under the terms of the GNU General Public
+ *     License version 2.  This program is licensed "as is" without
+ *     any warranty of any kind, whether express or implied.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-385.dtsi"
+
+/ {
+	model = "Turris Omnia";
+	compatible = "cznic,turris-omnia", "marvell,armada385", "marvell,armada380";
+
+	chosen {
+		stdout-path = &uart0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x40000000>; /* 1024 MB */
+	};
+
+	soc {
+		ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
+			  MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000
+			  MBUS_ID(0x09, 0x19) 0 0xf1100000 0x10000
+			  MBUS_ID(0x09, 0x15) 0 0xf1110000 0x10000>;
+
+		internal-regs {
+
+			/* USB part of the PCIe2/USB 2.0 port */
+			usb@58000 {
+				status = "okay";
+			};
+
+			sata@a8000 {
+				status = "okay";
+			};
+
+			sdhci@d8000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&sdhci_pins>;
+				status = "okay";
+
+				bus-width = <8>;
+				no-1-8-v;
+				non-removable;
+			};
+
+			usb3@f0000 {
+				status = "okay";
+			};
+
+			usb3@f8000 {
+				status = "okay";
+			};
+		};
+
+		pcie-controller {
+			status = "okay";
+
+			pcie@1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			pcie@2,0 {
+				/* Port 1, Lane 0 */
+				status = "okay";
+			};
+
+			pcie@3,0 {
+				/* Port 2, Lane 0 */
+				status = "okay";
+			};
+		};
+	};
+};
+
+/* Connected to 88E6176 switch, port 6 */
+&eth0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ge0_rgmii_pins>;
+	status = "okay";
+	phy-mode = "rgmii-id";
+
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
+
+/* Connected to 88E6176 switch, port 5 */
+&eth1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&ge1_rgmii_pins>;
+	status = "okay";
+	phy-mode = "rgmii-id";
+
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
+
+/* WAN port */
+&eth2 {
+	status = "okay";
+	phy-mode = "sgmii";
+	phy = <&phy1>;
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins>;
+	status = "okay";
+
+	i2cmux@70 {
+		compatible = "nxp,pca9547";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x70>;
+		status = "okay";
+
+		i2c@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			/* STM32F0 command interface at address 0x2a */
+			/* leds device (in STM32F0) at address 0x2b */
+
+			eeprom@54 {
+				compatible = "at,24c64";
+				reg = <0x54>;
+
+				/* The EEPROM contains data for bootloader.
+				 * Contents:
+				 * 	struct omnia_eeprom {
+				 * 		u32 magic; (=0x0341a034 in LE)
+				 *		u32 ramsize; (in GiB)
+				 * 		char regdomain[4];
+				 * 		u32 crc32;
+				 * 	};
+				 */
+			};
+		};
+
+		i2c@1 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <1>;
+
+			/* routed to PCIe0/mSATA connector (CN7A) */
+		};
+
+		i2c@2 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <2>;
+
+			/* routed to PCIe1/USB2 connector (CN61A) */
+		};
+
+		i2c@3 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <3>;
+
+			/* routed to PCIe2 connector (CN62A) */
+		};
+
+		i2c@4 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <4>;
+
+			/* routed to SFP+ */
+		};
+
+		i2c@5 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <5>;
+
+			/* ATSHA204A at address 0x64 */
+		};
+
+		i2c@6 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <6>;
+
+			/* exposed on pin header */
+		};
+
+		i2c@7 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <7>;
+
+			pcawan: gpio@71 {
+				/*
+				 * GPIO expander for SFP+ signals and
+				 * and phy irq
+				 */
+				compatible = "nxp,pca9538";
+				reg = <0x71>;
+
+				pinctrl-names = "default";
+				pinctrl-0 = <&pcawan_pins>;
+
+				interrupt-parent = <&gpio1>;
+				interrupts = <14 IRQ_TYPE_LEVEL_LOW>;
+
+				gpio-controller;
+				#gpio-cells = <2>;
+			};
+		};
+	};
+};
+
+&mdio {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mdio_pins>;
+	status = "okay";
+
+	phy1: phy@1 {
+		status = "okay";
+		compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22";
+		reg = <1>;
+
+		/* irq is connected to &pcawan pin 7 */
+	};
+
+	/* Switch MV88E7176 at address 0x10 */
+};
+
+&pinctrl {
+	pcawan_pins: pcawan-pins {
+		marvell,pins = "mpp46";
+		marvell,function = "gpio";
+	};
+
+	spi0cs0_pins: spi0cs0-pins {
+		marvell,pins = "mpp25";
+		marvell,function = "spi0";
+	};
+
+	spi0cs1_pins: spi0cs1-pins {
+		marvell,pins = "mpp26";
+		marvell,function = "spi0";
+	};
+};
+
+&spi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi0_pins &spi0cs0_pins>;
+	status = "okay";
+
+	spi-nor@0 {
+		compatible = "spansion,s25fl164k", "jedec,spi-nor";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0>;
+		spi-max-frequency = <40000000>;
+
+		partition@0 {
+			reg = <0x0 0x00100000>;
+			label = "U-Boot";
+		};
+
+		partition@1 {
+			reg = <0x00100000 0x00700000>;
+			label = "Rescue system";
+		};
+	};
+
+	/* MISO, MOSI, SCLK and CS1 are routed to pin header CN11 */
+};
+
+&uart0 {
+	/* Pin header CN10 */
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins>;
+	status = "okay";
+};
+
+&uart1 {
+	/* Pin header CN11 */
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "okay";
+};
-- 
2.10.2


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

^ permalink raw reply related

* [PATCH v5 1/2] devicetree: Add vendor prefix for CZ.NIC
From: Uwe Kleine-König @ 2016-11-25 14:26 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
  Cc: Mark Rutland, devicetree, Tomas Hlavacek, Rob Herring,
	linux-arm-kernel, Bedřicha Košatu
In-Reply-To: <20161125142658.21690-1-uwe@kleine-koenig.org>

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea78659..ae9fce9fed03 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -67,6 +67,7 @@ creative	Creative Technology Ltd
 crystalfontz	Crystalfontz America, Inc.
 cubietech	Cubietech, Ltd.
 cypress	Cypress Semiconductor Corporation
+cznic	CZ.NIC, z.s.p.o.
 dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
 davicom	DAVICOM Semiconductor, Inc.
 delta	Delta Electronics, Inc.
-- 
2.10.2


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

^ permalink raw reply related

* [PATCH v5 0/2] ARM: dts: add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-25 14:26 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth
  Cc: Mark Rutland, devicetree, Tomas Hlavacek, Rob Herring,
	linux-arm-kernel, Bedřicha Košatu

Hello,

This is basicly my v3 plus some of the changes that Tomas changed for
his RFC patch (implicitly "v4"). So compared to v3 the following
changed:

 - Add more documentation (mostly from Tomas)
 - Add more mbus ranges (by Tomas)
 - Move link to schematic below copyright header (suggested by Andrew)
 - reenable rtc as this is not that broken on production board
 - fix spi chip select pinmuxing

Uwe Kleine-König (2):
  devicetree: Add vendor prefix for CZ.NIC
  ARM: dts: add support for Turris Omnia

 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/armada-385-turris-omnia.dts      | 334 +++++++++++++++++++++
 3 files changed, 336 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-385-turris-omnia.dts

-- 
2.10.2


_______________________________________________
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 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2016-11-25 14:04 UTC (permalink / raw)
  To: Ulf Hansson, Ziji Hu
  Cc: Jimmy Xu, Andrew Lunn, Romain Perier, Hanna Hawa, Nadav Haklai,
	Zhen Huang, Victor Gu, Doug Jones, Jisheng Zhang, Yehuda Yitschak,
	Wei(SOCP) Liu, Kostya Porotchkin, Sebastian Hesselbarth,
	devicetree@vger.kernel.org, Jason Cooper, Rob Herring, Ryan Gao,
	Gregory CLEMENT, Marcin Wojtas,
	linux-arm-kernel@lists.infradead.org, Thomas Petazzoni, linux-mmc
In-Reply-To: <CAPDyKFr9uEjVQmTNP0KK8Zj9mxCW3i564E=47vTK0RLvXCjw3Q@mail.gmail.com>

On 24/11/16 15:34, Ulf Hansson wrote:
> On 24 November 2016 at 13:41, Ziji Hu <huziji@marvell.com> wrote:
>> On 2016/11/24 18:43, Ulf Hansson wrote:
>>> On 31 October 2016 at 12:09, Gregory CLEMENT
>>> <gregory.clement@free-electrons.com> wrote:
>>>> From: Ziji Hu <huziji@marvell.com>
>>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>>> +                                            struct mmc_ios *ios)
>>>> +{
>>>> +       struct sdhci_host *host = mmc_priv(mmc);
>>>> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> +       struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>>> +
>>>> +       /*
>>>> +        * Before SD/SDIO set signal voltage, SD bus clock should be
>>>> +        * disabled. However, sdhci_set_clock will also disable the Internal
>>>> +        * clock in mmc_set_signal_voltage().
>>>
>>> If that's the case then that is wrong in the generic sdhci code.
>>> What's the reason why it can't be fixed there instead of having this
>>> workaround?
>>>
>>     In my very own opinion, SD Spec doesn't specify whether SDCLK should be
>>     enabled or not during power setting.
>>     Enabling SDCLK might be a special condition only required by our SDHC.
>>     I try to avoid breaking other vendors' SDHC functionality
>>     if their SDHCs require SDCLK disabled.
>>     Thus I prefer to keep it inside our SDHC driver.
> 
> I let Adrian comment on this.
> 
> For sure we should avoid breaking other sdhci variant, but on the
> other hand *if* the generic code is wrong we should fix it!

Yes, this looks like something that could perhaps be fixed in sdhci.  I will
look into it.

^ permalink raw reply

* Re: [PATCH v2] mtd/spi-nor: Add SPI memory controllers for Aspeed SoCs
From: Marek Vasut @ 2016-11-25 13:57 UTC (permalink / raw)
  To: Joel Stanley, Cédric Le Goater
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
	Brian Norris, Boris Brezillon, Richard Weinberger,
	Cyrille Pitchen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland
In-Reply-To: <CACPK8Xc7Dy=4oaZ_+j3hCEkMY+sqZ_-QkCQ9okmUoOjTpuZvvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 11/21/2016 05:45 AM, Joel Stanley wrote:
> Hello Marek,

Hi!

> Thank you for the review. I have answered a few of your questions;
> I'll leave the rest to Cedric.
> 
> On Mon, Nov 21, 2016 at 8:13 AM, Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>>> index 4a682ee0f632..96148600fdab 100644
>>> --- a/drivers/mtd/spi-nor/Kconfig
>>> +++ b/drivers/mtd/spi-nor/Kconfig
>>> @@ -76,4 +76,16 @@ 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 ASPEED_FLASH_SPI
>>
>> Should be SPI_ASPEED , see the other controllers and keep the list sorted.
> 
> Perhaps SPI_NOR_ASPEED so it's clear it's not a driver for a generic SPI bus?

But it's not a driver for SPI-NOR only either, it seems it's a driver
for multiple distinct devices.

>>> +     tristate "Aspeed flash controllers in SPI mode"
>>> +     depends on HAS_IOMEM && OF
>>> +     depends on ARCH_ASPEED || COMPILE_TEST
>>> +     # IO_SPACE_LIMIT must be equivalent to (~0UL)
>>> +     depends on !NEED_MACH_IO_H
>>
>> Why?
>>
>>> +     help
>>> +       This enables support for the New Static Memory Controller
>>> +       (FMC) in the Aspeed SoCs (AST2400 and AST2500) when attached
>>> +       to SPI nor chips, and support for the SPI Memory controller
>>> +       (SPI) for the BIOS.
>>
>> I think there is a naming chaos between FMC, SMC (as in Static MC) and
>> SMC (as in SPI MC).
> 
> Yes, you're spot on. This naming chaos comes form the vendor's documentation.
> 
> I think we could re-work this sentence to make it clearer.

Please do before someone's head explodes from this :)

>>> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
>>> +                                 size_t len)
>>> +{
>>
>> What if start of buf is unaligned ?
>>
>>> +     if ((((unsigned long)src | (unsigned long)buf | len) & 3) == 0) {
>>
>> Uh, should use boolean OR, not bitwise or. Also, if you're testing
>> pointer for NULL, do if (!ptr) .
>>
>> if (!src || !buf || !len)
>>    return;
> 
> That's a different test. We're testing here that the buffers are
> aligned to see if we can do a word-at-a-time copy.
> 
> If not, it falls through to do a byte-at-a-time copy. I think this
> covers your first question about buf being unaligned.

Ah, I see, thanks for clarifying. Comment in the code would be helpful
for why what you're doing is OK. And I think you want to cast to
uintptr_t instead to make this work on 64bit.

> Cedric, perhaps you could create a macro called IS_ALLIGNED to make it
> clear what this is doing?

Yup, thanks!

>>
>> while (...)
>>
>>> +             while (len > 3) {
>>> +                     *(u32 *)buf = readl(src);
>>> +                     buf += 4;
>>> +                     src += 4;
>>> +                     len -= 4;
>>> +             }
>>> +     }
>>> +
>>> +     while (len--) {
>>> +             *(u8 *)buf = readb(src);
>>> +             buf += 1;
>>> +             src += 1;
>>> +     }
>>> +     return 0;
>>> +}
>>> +/*
>>> + * SPI Flash Configuration Register (AST2400 SPI)
>>> + */
>>> +#define CONFIG_REG                   0x0
>>> +#define    CONFIG_ENABLE_CE_INACTIVE     BIT(1)
>>> +#define    CONFIG_WRITE                          BIT(0)
>>
>> #define[space]FOO[tab]BIT(bar)
> 
> These are bits within the CONFIG_REG. It follows the same style as
> other spi-nor drivers, eg. nxp-spifi.
> 
> I think it's somewhat clearer, but if you have a strong preference
> against then fair enough.

It triggers my OCD, but I think it's a matter of taste, so I don't care
that much.

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

^ permalink raw reply

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

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

[...]

> +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[sfc->num_chip].nor.mtd));
                                                   ^^^^^^^^^^^^^
This will always unregister the same flash, no ? This cannot work.

> +}

[...]

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

^ permalink raw reply

* Re: [PATCH 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Marek Vasut @ 2016-11-25 13:54 UTC (permalink / raw)
  To: Shawn Lin, Rob Herring, David Woodhouse, Brian Norris
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <76670120-dacd-ef0e-cf36-cbf549b19853-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On 11/21/2016 03:51 AM, Shawn Lin wrote:
> Hi Marek,

Hi!

[...]

>>> sfc->num_chip stands for how many flashes registered successfully.
>>
>> Does it work if you have a hole in there ? Like if you have a flash on
>> chipselect 0 and chipselect 2 ?
> 
> Yes it does, as it won't leave a room for chipselect 1 whose node isn't
> present, which means there isn't a hole in there at all. :)

Ah , because you are not indexing those NORs with their CS number , right ?


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

^ permalink raw reply

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

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

[...]

> +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(enum read_mode flash_read)
> +{
> +	enum rockchip_sfc_iftype if_type;
> +
> +	switch (flash_read) {
> +	case SPI_NOR_DUAL:
> +		if_type = IF_TYPE_DUAL;
> +		break;
> +	case SPI_NOR_QUAD:
> +		if_type = IF_TYPE_QUAD;
> +		break;
> +	case SPI_NOR_NORMAL:
> +	case SPI_NOR_FAST:
> +		if_type = IF_TYPE_STD;
> +		break;
> +	default:
> +		pr_err("unsupported SPI read mode\n");

I'd switch this to dev_err() , so it's obvious from which device this
error came. It's OK to pass in the sfc pointer.

> +		return -EINVAL;
> +	}
> +
> +	return if_type;
> +}

[...]

> +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, i;
> +
> +	/* Align bytes to dwords */
> +	dwords = (len + 3) >> 2;
> +
> +	for (i = 0; i < dwords; i++)
> +		writel_relaxed(*(buf + 4 * i), sfc->regbase + SFC_DATA);

Can $buf be unaligned to 4-bytes ? :-)

> +	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 (op_type == SFC_CMD_DIR_WR)
> +		reg = (nor->program_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT;
> +	else
> +		reg = (nor->read_opcode & SFC_CMD_IDX_MASK) <<
> +		       SFC_CMD_IDX_SHIFT;

You can define some SFC_CMD_IDX(foo) to avoid this two-line reg assignment:

#define SFC_CMD_IDX(opc) \
 ((opc) & SFC_CMD_IDX_MASK) << SFC_CMD_IDX_SHIFT)

reg = SFC_CMD_IDX(nor->read_opcode);

> +	reg |= op_type << SFC_CMD_DIR_SHIFT;
> +	reg |= (nor->addr_width == 4) ?
> +		SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS;
> +
> +	if_type = get_if_type(nor->flash_read);
> +	writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) |
> +		       (if_type << SFC_CTRL_ADDR_BITS_SHIFT) |
> +		       (if_type << SFC_CTRL_CMD_BITS_SHIFT) |
> +		       (sfc->negative_edge ? SFC_CTRL_PHASE_SEL_NEGETIVE : 0),
> +		       sfc->regbase + SFC_CTRL);
> +
> +	reg |= (priv->cs & SFC_CMD_CS_MASK) << SFC_CMD_CS_SHIFT;
> +	reg |= (len & SFC_CMD_TRAN_BYTES_MASK) << SFC_CMD_TRAN_BYTES_SHIFT;
> +
> +	if (op_type == SFC_CMD_DIR_RD)
> +		reg |= 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_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;

Don't you want to stop the DMA too ?

> +	}
> +
> +	/* 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)
> +		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, tx_wl, count, i;
> +	unsigned long timeout;
> +	int ret = 0;
> +	u32 *tbuf = (u32 *)buf;
> +
> +	/*
> +	 * 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 = (len + 3) >> 2;

Kernel has macros for rounding up, like DIV_ROUND_UP().

> +	while (dwords) {
> +		tx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_TX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_TX_WATER_LVL_MASK;
> +
> +		if (tx_wl > 0) {
> +			count = min_t(u32, dwords, tx_wl);
> +			for (i = 0; i < count; i++) {
> +				writel_relaxed(*tbuf++,
> +					       sfc->regbase + SFC_DATA);
> +				dwords--;
> +			}
> +
> +			if (dwords == 0)
> +				break;
> +			timeout = 0;
> +		} else {
> +			mdelay(1);

That is a long delay, shouldn't you wait using udelay() here ?

> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (ret)
> +		return ret;
> +	else
> +		return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
> +					size_t len)
> +{
> +	u32 dwords, rx_wl, count, i, tmp;
> +	unsigned long timeout;
> +	int ret = 0;
> +	u32 *tbuf = (u32 *)buf;
> +	u_char *tbuf2;
> +
> +	/*
> +	 * Align bytes to dwords, and get the remained bytes.
> +	 * We should always round down to DWORD as the ahb for
> +	 * Rockchip Socs won't support non-aligned-to-DWORD transfer.
> +	 * So please don't use any APIs that will finally use non-aligned
> +	 * read, for instance, memcpy_fromio or ioread32_rep etc.
> +	 */
> +	dwords = len >> 2;
> +	len = len & 0x3;

Won't this overwrite some bits past the $buf if you write more than $len
bytes into this memory location ?

> +	while (dwords) {
> +		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_RX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_RX_WATER_LVL_MASK;
> +
> +		if (rx_wl > 0) {
> +			count = min_t(u32, dwords, rx_wl);
> +			for (i = 0; i < count; i++) {
> +				*tbuf++ = readl_relaxed(sfc->regbase +
> +							SFC_DATA);
> +				dwords--;
> +			}
> +
> +			if (dwords == 0)
> +				break;
> +			timeout = 0;
> +		} else {
> +			mdelay(1);
> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	/* Read the remained bytes */
> +	timeout = 0;
> +	tbuf2 = (u_char *)tbuf;
> +	while (len) {
> +		rx_wl = (readl_relaxed(sfc->regbase + SFC_FSR) >>
> +			 SFC_FSR_RX_WATER_LVL_SHIFT) &
> +			 SFC_FSR_RX_WATER_LVL_MASK;
> +		if (rx_wl > 0) {
> +			tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> +			for (i = 0; i < len; i++)
> +				tbuf2[i] = (u8)((tmp >> (i * 8)) & 0xff);
> +			goto done;
> +		} else {
> +			mdelay(1);
> +			if (timeout++ > SFC_MAX_IDLE_RETRY) {
> +				ret = -ETIMEDOUT;
> +				break;
> +			}
> +		}
> +	}

Seems a lot like the write path, can you unify the code ?

> +done:
> +	if (ret)
> +		return ret;
> +	else
> +		return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to,
> +				     size_t len, u_char *buf, u8 op_type)
> +{
> +	struct rockchip_sfc_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 ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from, size_t len,
> +				 u_char *read_buf)
> +{
> +	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;
> +
> +	if (!sfc->use_dma)
> +		goto no_dma;

You should extract this DMA code into rockchip_sfc_dma_read/write()
instead and have this method-agnostic function only do the decision
between calling the PIO one and DMA one. That'd improve the structure
of the code a lot.

> +	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> +		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> +		dma_addr = dma_map_single(NULL, (void *)read_buf,
> +					  trans, DMA_FROM_DEVICE);
> +		if (dma_mapping_error(sfc->dev, dma_addr))
> +			dma_addr = 0;
> +
> +		/* Fail to map dma, use pre-allocated area instead */
> +		ret = rockchip_sfc_dma_transfer(nor, from + offset,
> +						dma_addr ? dma_addr :
> +						sfc->dma_buffer,
> +						trans, SFC_CMD_DIR_RD);
> +
> +		if (dma_addr) {
> +			/* Invalidate the read data from dma_addr */
> +			dma_sync_single_for_cpu(sfc->dev, dma_addr,
> +						trans, DMA_FROM_DEVICE);
> +			dma_unmap_single(NULL, dma_addr,
> +					 trans, DMA_FROM_DEVICE);
> +		}
> +
> +		if (ret) {
> +			dev_warn(nor->dev, "DMA read timeout\n");
> +			return ret;
> +		}
> +		if (!dma_addr)
> +			memcpy(read_buf + offset, sfc->buffer, trans);
> +	}
> +
> +	return len;
> +
> +no_dma:
> +	ret = rockchip_sfc_pio_transfer(nor, from, len,
> +					read_buf, SFC_CMD_DIR_RD);
> +	if (ret) {
> +		dev_warn(nor->dev, "PIO read timeout\n");
> +		return ret;
> +	}
> +	return len;
> +}
> +
> +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
> +				  size_t len, const u_char *write_buf)
> +{
> +	struct rockchip_sfc_chip_priv *priv = nor->priv;
> +	struct rockchip_sfc *sfc = priv->sfc;
> +	size_t offset;
> +	int ret;
> +	dma_addr_t dma_addr = 0;
> +
> +	if (!sfc->use_dma)
> +		goto no_dma;
> +
> +	for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> +		size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> +		dma_addr = dma_map_single(NULL, (void *)write_buf,
> +					  trans, DMA_TO_DEVICE);
> +		if (dma_mapping_error(sfc->dev, dma_addr)) {
> +			dma_addr = 0;
> +			memcpy(sfc->buffer, write_buf + offset, trans);
> +		} else {
> +			/* Flush the write data to dma memory */
> +			dma_sync_single_for_device(sfc->dev, dma_addr,
> +						   trans, DMA_TO_DEVICE);
> +		}
> +
> +		/* Fail to map dma, use pre-allocated area instead */
> +		ret = rockchip_sfc_dma_transfer(nor, to + offset,
> +						dma_addr ? dma_addr :
> +						sfc->dma_buffer,
> +						trans, SFC_CMD_DIR_WR);
> +		if (dma_addr)
> +			dma_unmap_single(NULL, dma_addr,
> +					 trans, DMA_TO_DEVICE);
> +		if (ret) {
> +			dev_warn(nor->dev, "DMA write timeout\n");
> +			return ret;
> +		}
> +	}

Again, the read and write looks really similar. I wonder if you cannot
parametrize it and unify the code.

> +	return len;
> +no_dma:
> +	ret = rockchip_sfc_pio_transfer(nor, to, len,
> +					(u_char *)write_buf, SFC_CMD_DIR_WR);
> +	if (ret) {
> +		dev_warn(nor->dev, "PIO write timeout\n");
> +		return ret;
> +	}
> +	return len;
> +}
> +
> +/**
> + * Get spi flash device information and register it as a mtd device.
> + */
> +static int rockchip_sfc_register(struct device_node *np,
> +				 struct rockchip_sfc *sfc)
> +{
> +	struct device *dev = sfc->dev;
> +	struct mtd_info *mtd;
> +	int ret;
> +
> +	sfc->flash[sfc->num_chip].nor.dev = dev;
> +	spi_nor_set_flash_node(&(sfc->flash[sfc->num_chip].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;
> +	sfc->flash[sfc->num_chip].nor.priv = &(sfc->flash[sfc->num_chip]);

You can add nor = sfc->flash[sfc->num_chip].nor; here to avoid
constantly replicating the whole sfc->flash[sfc->num_chip].nor .

> +	sfc->flash[sfc->num_chip].nor.prepare = rockchip_sfc_prep;
> +	sfc->flash[sfc->num_chip].nor.unprepare = rockchip_sfc_unprep;
> +	sfc->flash[sfc->num_chip].nor.read_reg = rockchip_sfc_read_reg;
> +	sfc->flash[sfc->num_chip].nor.write_reg = rockchip_sfc_write_reg;
> +	sfc->flash[sfc->num_chip].nor.read = rockchip_sfc_read;
> +	sfc->flash[sfc->num_chip].nor.write = rockchip_sfc_write;
> +	sfc->flash[sfc->num_chip].nor.erase = NULL;
> +	ret = spi_nor_scan(&(sfc->flash[sfc->num_chip].nor),
> +			    NULL, SPI_NOR_QUAD);
> +	if (ret)
> +		return ret;
> +
> +	mtd = &(sfc->flash[sfc->num_chip].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[sfc->num_chip].nor.mtd));

Same here. Also, what happens if you have a hole in the SPI NOR
numbering, ie you have only SPI NOR 0 and 2 registered ? This will fail,
so see the cadence qspi how to handle such case.

> +}
> +
> +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 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");

I think this should rather be a boolean property -- but isn't this
something like CPOL or CPHA anyway ?

> +	/* 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;
> +#if 1
> +	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);
> +#endif

#if 1, remove, #endif :-)

> +	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 0;
> +}
> +#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");

Email in MODULE_AUTHOR would be great addition.

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

^ permalink raw reply

* Re: [PATCH 3/3] pinctrl: sx150x: add support for sx1501, sx1504, sx1505 and sx1507
From: Linus Walleij @ 2016-11-25 13:45 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Mark Rutland, Andrey Smirnov, Neil Armstrong,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1480020320-28354-4-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On Thu, Nov 24, 2016 at 9:45 PM, Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org> wrote:

> Untested, register offsets carefully copied from datasheets.
>
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Why not. Nice to support all of them.

Patch applied, fixing the typ you mentioned in the follow
up in the process.

Yours,
Linus Walleij
--
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 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2016-11-25 13:43 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Jimmy Xu, Andrew Lunn, Romain Perier, Hanna Hawa,
	linux-kernel@vger.kernel.org, Nadav Haklai, Victor Gu, Doug Jones,
	Jisheng Zhang, Yehuda Yitschak, Wei(SOCP) Liu, Kostya Porotchkin,
	Sebastian Hesselbarth, devicetree@vger.kernel.org, Jason Cooper,
	Rob Herring, Ryan Gao, Gregory CLEMENT, Marcin Wojtas,
	linux-arm-kernel@lists.infradead.org, Thomas Petazzoni
In-Reply-To: <CAPDyKFoqyF5QkJy+PmGa-b8JhaUmVXeoqBfv+pdupFiJBgyiLg@mail.gmail.com>

Hi Ulf,

On 2016/11/25 21:06, Ulf Hansson wrote:
> [...]
> 
>>>>>>> +
>>>>>>> +       /*
>>>>>>> +        * Xenon Specific property:
>>>>>>> +        * emmc: explicitly indicate whether this slot is for eMMC
>>>>>>> +        * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
>>>>>>> +        * tun-count: the interval between re-tuning
>>>>>>> +        * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
>>>>>>> +        */
>>>>>>> +       if (of_property_read_bool(np, "marvell,xenon-emmc"))
>>>>>>> +               priv->emmc_slot = true;
>>>>>>
>>>>>> So, you need this because of the eMMC voltage switch behaviour, right?
>>>>>>
>>>>>> Then I would rather like to describe this a generic DT bindings for
>>>>>> the eMMC voltage level support. There have acutally been some earlier
>>>>>> discussions for this, but we haven't yet made some changes.
>>>>>>
>>>>>> I think what is missing is a mmc-ddr-3_3v DT binding, which when set,
>>>>>> allows the host driver to accept I/O voltage switches to 3.3V. If not
>>>>>> supported the  ->start_signal_voltage_switch() ops may return -EINVAL.
>>>>>> This would inform the mmc core to move on to the next supported
>>>>>> voltage level. There might be some minor additional changes to the mmc
>>>>>> card initialization sequence, but those should be simple.
>>>>>>
>>>>>> I can help out to look into this, unless you want to do it yourself of course!?
>>>>>>
>>>>>    Yes. One of the reasons is to provide eMMC specific voltage setting.
>>>>>    But in my very own opinion, it should be irrelevant to voltage level.
>>>>>    The eMMC voltage setting on our SDHC is different from SD/SDIO voltage switch.
>>>>>    It will become more complex with different SOC implementation details.
>>>>
>>>> Got it. Although I think we can cope with that fine just by using the
>>>> different SD/eMMC speed modes settings defined in DT (or from the
>>>> SDHCI caps register)
>>>>
>>>     In my very opinion, I'm not sure if there is any corner case that driver cannot
>>>     determine the eMMC card type from DT and SDHC caps.
>>>
>>>>>    Unfortunately, MMC driver cannot determine the card type yet when eMMC voltage
>>>>>    setting should be executed.
>>>>>    Thus an flag is required here to tell driver to execute eMMC voltage setting.
>>>>>
>>>>>    Besides, additional eMMC specific settings might be implemented in future, besides
>>>>>    voltage setting. Most of them should be completed before MMC driver recognizes the
>>>>>    card type. Thus I have to keep this flag to indicate current SDHC is for eMMC.
>>>>
>>>> I doubt you will need a generic "eMMC" flag, but let's see when we go forward.
>>>>
>>>> Currently it's clear you don't need such a flag, so I will submit a
>>>> change adding a DT binding for "mmc-ddr-3_3v" then we can take it from
>>>> there, to see if it suits your needs.
>>>>
>>
>>     Another reason for a special "xenon-emmc" property is that our host IP usually can
>>     support both eMMC and SD. Whether a host is used as eMMC or SD depends on the
>>     final implementation of the actual product.
>>     Thus our host driver needs to know whether current SDHC is fixed as eMMC or SD.
>>     So far, It can only get the information from DT.
> 
> As a matter of fact for mounted non-removable cards, such as eMMC, we
> already have the option to describe some of their characteristics in
> DT. Perhaps that's what you need?
> 
> Please have a look at:
> Documentation/devicetree/bindings/mmc/mmc-card.txt
> 
   Great!
   I will try this mmc-card sub-node.

   Thank you very much.

Best regards,
Hu Ziji

>>
>>     After out host driver get the card type information from DT, it can prepare eMMC
>>     specific voltage, set eMMC specific mmc->caps/caps2 flags and do other
>>     vendor specific init, before card init procedure.
>>     Otherwise, our host driver has to wait until card type is determined in mmc_rescan().
>>
>>     A generic "eMMC" flag is unnecessary. I just require a private property,
>>     which is only used in our host driver and DT.
>>
>>     Thank you.
>>
>> Best regards,
>> Hu Ziji
>>
>>>
>>>     Actually, our eMMC is usually fixed as 1.8V.
>>>
>>>     The pair "no-sd" + "no-sdio" can provide the similar information.
>>>     But I'm not sure if it is proper to use those two property in such a way.
>>>
>>>     Thank you.
>>>
>>> Best regards
>>> Hu Ziji
>>>
>>>> [...]
>>>>
>>>> Kind regards
>>>> Uffe
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 
> Kind regards
> Uffe
> 

^ permalink raw reply

* Re: [PATCH 2/3] pinctrl: sx150x: sort chips by part number
From: Linus Walleij @ 2016-11-25 13:41 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland,
	Andrey Smirnov, Neil Armstrong, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <1480020320-28354-3-git-send-email-peda@axentia.se>

On Thu, Nov 24, 2016 at 9:45 PM, Peter Rosin <peda@axentia.se> wrote:

> Signed-off-by: Peter Rosin <peda@axentia.se>

Pretty, satisfies my OCD.

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* RE: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: David Laight @ 2016-11-25 13:41 UTC (permalink / raw)
  To: 'Martin Blumenstingl', linux-amlogic@lists.infradead.org,
	devicetree@vger.kernel.org, netdev@vger.kernel.org,
	davem@davemloft.net, khilman@baylibre.com, mark.rutland@arm.com,
	robh+dt@kernel.org
  Cc: f.fainelli@gmail.com, alexandre.torgue@st.com,
	catalin.marinas@arm.com, will.deacon@arm.com, carlo@caione.org,
	peppe.cavallaro@st.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl@googlemail.com>

From: Martin Blumenstingl
> Sent: 25 November 2016 13:02
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
...

Interesting thought.
Can you test phy loopback with different delays?
It might be then possible to default to 'auto'.

	David

^ permalink raw reply

* Re: [PATCH 1/3] pinctrl: sx150x: use correct registers for reg_sense (sx1502 and sx1508)
From: Linus Walleij @ 2016-11-25 13:40 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel@vger.kernel.org, Rob Herring, Mark Rutland,
	Andrey Smirnov, Neil Armstrong, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <1480020320-28354-2-git-send-email-peda@axentia.se>

On Thu, Nov 24, 2016 at 9:45 PM, Peter Rosin <peda@axentia.se> wrote:

> All other registers on these chips are 8-bit, but reg_sense is 16-bits
> and therefore needs to be moved down one notch.
> This was apparently overlooked in the conversion to regmap, which only
> updated the register locations for the 16-bit chips.
>
> Fixes: 6489677f86c3 ("pinctrl-sx150x: Replace sx150x_*_cfg by means of regmap API")
> Signed-off-by: Peter Rosin <peda@axentia.se>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply


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