Re: [RESEND PATCH 0/3 v6] Add Mediatek MT8173 cpufreq driver

[Pi-Cheng Chen]

On Wed, Aug 26, 2015 at 10:16 AM, Viresh Kumar  wrote:
> On 26-08-15, 09:25, Pi-Cheng Chen wrote:
>> The [3/3] is based on Mediatek SoC maintainer tree[1] and the patch which
>> introduce a new clock type[2] consumed by MT8173 cpufreq driver. So it will
>> cause some conflicts if it goes through your tree. I am not sure how this
>> should be handled, but should it be merged through Mediatek SoC maintainer
>> tree?
>
> Just get that applied to MTK tree, it doesn't have any dependency on
> rest of the patches for build/boot. The only thing is that cpufreq
> wouldn't work and it will work as soon as Rafael's and MTK's trees are
> merged by Linus.

Thanks for your explanation.

@Rafael, Would you please apply [1,2] to your tree?

@Matthias, Would you please apply [3/3] of this series?

Thanks.

Best Regards,
Pi-Cheng

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

Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.

[Marek Vasut]

On Wednesday, August 26, 2015 at 12:09:50 AM, vikas wrote:
> Hi,
> 
> On 08/21/2015 02:20 AM, Marek Vasut wrote:
> > From: Graham Moore 
> > 
> > Add support for the Cadence QSPI controller. This controller is
> > present in the Altera SoCFPGA SoCs and this driver has been tested
> > on the Cyclone V SoC.
> > 
> > Signed-off-by: Graham Moore 
> > Signed-off-by: Marek Vasut 
> > Cc: Alan Tull 
> > Cc: Brian Norris 
> > Cc: David Woodhouse 
> > Cc: Dinh Nguyen 
> > Cc: Graham Moore 
> > Cc: Vikas MANOCHA 
> > Cc: Yves Vandervennet 
> > Cc: devicetree@vger.kernel.org
> > ---
> > 
> >  drivers/mtd/spi-nor/Kconfig   |   11 +
> >  drivers/mtd/spi-nor/Makefile  |1 +
> >  drivers/mtd/spi-nor/cadence-quadspi.c | 1260
> >  + 3 files changed, 1272 insertions(+)
> >  create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
> > 
> > V2: use NULL instead of modalias in spi_nor_scan call
> > V3: Use existing property is-decoded-cs instead of creating duplicate.
> > V4: Support Micron quad mode by snooping command stream for EVCR command
> > 
> > and subsequently configuring Cadence controller for quad mode.
> > 
> > V5: Clean up sparse and smatch complaints.  Remove snooping of Micron
> > 
> > quad mode.  Add comment on XIP mode bit and dummy clock cycles.  Set
> > up SRAM partition at 1:1 during init.
> > 
> > V6: Remove dts patch that was included by mistake.  Incorporate Vikas's
> > 
> > comments regarding fifo width, SRAM partition setting, and trigger
> > address.  Trigger address was added as an unsigned int, as it is not
> > an IO resource per se, and does not need to be mapped. Also add
> > Marek Vasut's workaround for picking up OF properties on subnodes.
> > 
> > V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
> > 
> >   macros and replace them with functions. Get rid of unused
> >   variables.
> > 
> > - Implement support for nor->set_protocol() to handle Quad-command,
> > 
> >   this patch now depends on the following patch:
> >   mtd: spi-nor: notify (Q)SPI controller about protocol change
> > 
> > - Replace that cqspi_fifo_read() disaster with plain old readsl()
> > 
> >   and cqspi_fifo_write() tentacle horror with pretty writesl().
> > 
> > - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> > - Get rid of cqspi_find_chipselect() mess, instead just place the
> > 
> >   struct cqspi_st and chipselect number into struct cqspi_flash_pdata
> >   and set nor->priv to the struct cqspi_flash_pdata of that
> >   particular chip.
> > 
> > - Replace the odd math in calculate_ticks_for_ns() with
> > DIV_ROUND_UP(). - Make variables const where applicable.
> > 
> > V8: - Implement a function to wait for bit being set/unset for a given
> > 
> >   period of time and use it to replace the ad-hoc bits of code.
> > 
> > - Configure the write underflow watermark to be 1/8 if FIFO size.
> > - Extract out the SPI NOR flash probing code into separate function
> > 
> >   to clearly mark what will soon be considered a boilerplate code.
> > 
> > - Repair the handling of mode bits, which caused instability in V7.
> > - Clean up the interrupt handling
> > - Fix Kconfig help text and make the patch depend on OF and
> > COMPILE_TEST.
> > 
> > diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> > index 89bf4c1..ed253a2 100644
> > --- a/drivers/mtd/spi-nor/Kconfig
> > +++ b/drivers/mtd/spi-nor/Kconfig
> > @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
> > 
> >   Flash. Enable this option if you have a device with a SPIFI
> >   controller and want to access the Flash as a mtd device.
> > 
> > +config SPI_CADENCE_QUADSPI
> > +   tristate "Cadence Quad SPI controller"
> > +   depends on OF && COMPILE_TEST
> > +   help
> > + Enable support for the Cadence Quad SPI Flash controller.
> > +
> > + Cadence QSPI is a specialized controller for connecting an SPI
> > + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> > + device with a Cadence QSPI controller and want to access the
> > + Flash as an MTD device.
> > +
> 
> the patch failed to apply, please rebase it to master.

it's based on -next, I'm sure you can fix trivial conflicts yourself if you
need to apply it elsewhere. Do you have any other review comments ?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] qcom: ipq40xx: Add basic board/dts support for IPQ40XX SoC

[Varadarajan Narayanan]

On Tue, Aug 25, 2015 at 02:14:05PM -0700, Stephen Boyd wrote:
> On 08/25, Varadarajan Narayanan wrote:
> > On Mon, Aug 24, 2015 at 03:49:28PM -0700, Stephen Boyd wrote:
> > > On 08/24, Varadarajan Narayanan wrote:
> >
> > > > +   compatible = "qcom,ipq40xx-r3pc", "qcom,ipq40xx";
> > > > +
> > > > +   memory {
> > > > +   device_type = "memory";
> > > > +   reg = <0x8000 0x2000>; /* 512MB */
> > > > +   };
> > >
> > > Doesn't the bootloader fill the memory node for us? Why do we
> > > need this?
> > >
> > > > +
> > > > +   chosen {
> > > > +   bootargs = "root=/dev/ram rw init=/init 
> > > > console=ttyMSM0,115200n8 initrd=0x8200,0x000E2246";
> > > > +   };
> > >
> > > Please don't add bootargs. Use stdout-path for the console part
> > > and everything else should be done by the bootloader or is the
> > > defaults.
> >
> > Since this is for the emulation platform, we don't have the bootloader.
>
> Why are we upstreaming an emulation platform?

Ok, will skip this.

-Varada

> I assume you have some sort of scripts or something to load the
> kernel image into the emulator, so those scripts could go and
> modify the dtb in place to populate the initrd and commandline.
> If you need pointers on that feel free to contact me directly.
>
> >
> > > > +   compatible = "arm,armv7-timer";
> > > > +   interrupts = <1 2 0xf08>,
> > > > +<1 3 0xf08>,
> > > > +<1 4 0xf08>,
> > > > +<1 1 0xf08>;
> > > > +   clock-frequency = <2083>;
> > >
> > > Drop this clock-frequency part if you can. The hardware should
> > > properly report the frequency.
> >
> > Cannot drop this. This is the ref-clock frequency. For the
> > chip it is 48MHz, for emulation is ~20MHz. Similar to
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/qcom-msm8974.dtsi#n89
>
> And the hardware is not emulated properly to report the actual
> frequency that the timer is running at? That's sad.
>
> At the least, this should be in the board specific dtsi file
> instead of the SoC specific file, because it's a board quirk.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/07] ARM: shmobile: APMU DT support via SMP Enable method V2

[Simon Horman]

[resending as vegr rejected the previous attempt]

On 25 August 2015 at 13:09, Magnus Damm  wrote:

> Hi Simon,
>
> On Tue, Aug 25, 2015 at 9:49 AM, Simon Horman  wrote:
> > Hi Magnus,
> >
> > On Sun, Aug 23, 2015 at 04:24:27PM +0900, Magnus Damm wrote:
> >> ARM: shmobile: APMU DT support via SMP Enable method V2
> >>
> >> [PATCH v2 01/07] devicetree: bindings: Renesas APMU and SMP Enable
> method
> >> [PATCH v2 02/07] ARM: shmobile: Add APMU DT support via Enable method
> >> [PATCH v2 03/07] ARM: shmobile: Add APMU nodes to r8a7790 DTSI
> >> [PATCH v2 04/07] ARM: shmobile: Add APMU nodes to r8a7791 DTSI
> >> [PATCH v2 05/07] ARM: shmobile: Add function to prioritize DT SMP
> >> [PATCH v2 06/07] ARM: shmobile: Prioritize r8a7790 DT APMU support
> >> [PATCH v2 07/07] ARM: shmobile: Prioritize r8a7791 DT APMU support
> >>
> >> These patches add DT support for the APMU hardware commonly found in
> >> Renesas R-Car Gen2 SoCs. Without these patches the APMU gets configured
> >> through data expressed in C, and with this series applied it is possible
> >> to describe the APMU configuration in DT and let the enable method point
> >> out that the APMU should be used.
> >>
> >> Patch 1 and 2 are Documenting and adding DT support to the APMU driver
> >> together with enabling use of the enable-method way to describe that
> >> the APMU hardware is needed for SMP operation.
> >>
> >> Patch 3 and 4 are related to r8a7790/r8a7791 support that get a DTSI
> update
> >> to describe the APMU hardware. To avoid breaking support for older DTBs
> out
> >> in the wild these patches keep the older existing C code APMU
> configuration
> >> as-is. Patch 5-7 make sure that during run-time, if the APMU is
> installed
> >> via the DT enable-method then it will not be overriden by older non-DT
> >> configuration.
> >>
> >> I suggest making APMU DT configuration mandatory for SMP operation on
> >> newer SoCs and that we keep the old APMU support code in place for a
> >> good number of kernel releases or until we can identify a couple of
> major
> >> reasons good enough to force a DTB update on the end users.
> >>
> >> In the future r8a7793 and r8a7794 support may be added by using code
> >> similar to patch 3 and 4 but without any C-based SMP code and fallback.
> >
> > Sounds reasonable.
> >
> > Looking over the review of this series I see only one minor comment
> > regarding spelling in documentation. I have it in mind to queue up
> > this series with that problem fixed unless objections are raised
> > in the near future. Please feel free to convince me otherwise or
> > repost the series with the spelling error fixed.
>
> Thanks for your feedback! My plan is to update this series once more
> early next month, so no need to pick anything up right now.
>

Thanks, I will immediately do nothing.


> I feel that we should discuss with the ARM SoC guys about the reason
> why SMP Enable DT binding is needed. From my side it seems enough to
> determine the SMP configuration from the SoC part number (and maybe
> APMU DT bits) that we already have encoded in DT.
>
> Perhaps discussing that at ELCE would be good?
>
> Best,
>
> / magnus
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Haojian Zhuang]

On Tue, 2015-08-25 at 16:37 +0100, Leif Lindholm wrote:
> On Tue, Aug 25, 2015 at 04:51:22PM +0200, Ard Biesheuvel wrote:
> > >>Arm kernel should either fetch memory information from
> > >>efi or DT.
> > >
> > > Absolutely.
> > >
> > >>Currently arm kernel fetch both efi memory information and
> > >>reserved buffer from DTB at the same time.
> > >
> > > No, it does not.
> > 
> > It should not, but it does. Due to an oversight, the stub removes
> > /memreserve/ entries but ignores the reserved-memory node completely.
> 
> Urgh.
> 
> > This was reported here in fact
> > 
> > http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742
> > 
> > but there has not been a followup to this series.
> 
> Are all of those patches still relevant, or did some of them go in
> already?
> 
> Haojian: can you give that patch a spin and see if it does what you
> need, combined with adding the reserved areas to the UEFI memory map?
> 
> /
> Leif

It's so nice. This patch is what I need.

Thanks
Haojian

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

Re: [PATCH V1 0/3] fix clock issue for fsl,spdif

[Shengjiu Wang]

On Tue, Aug 25, 2015 at 11:45:27AM -0700, Michael Turquette wrote:
> Quoting Shengjiu Wang (2015-08-11 02:26:51)
> > fix clock issue for fsl,spdif
> > 
> > Shengjiu Wang (3):
> >   ARM: imx6q: Add SPDIF_GCLK clock in clock tree
> >   ARM: imx6sl: Add SPDIF_GCLK clock in clock tree
> >   ARM: imx6sx: Add SPDIF_GCLK clock in clock tree
> 
> Hello Shengjiu Wang,
> 
> Please move the dts changes into separate patches. I can take the
> changes to the clock driver and the header through the clk tree, but the
> dt patches need to go through arm-soc.
> 
> Regards,
> Mike

Thanks Mike. I will send another patches for dts changes.

> 
> > 
> >  arch/arm/boot/dts/imx6qdl.dtsi|  2 +-
> >  arch/arm/boot/dts/imx6sl.dtsi | 16 
> >  arch/arm/boot/dts/imx6sx.dtsi |  2 +-
> >  drivers/clk/imx/clk-imx6q.c   |  4 +++-
> >  drivers/clk/imx/clk-imx6sl.c  |  4 +++-
> >  drivers/clk/imx/clk-imx6sx.c  |  1 +
> >  include/dt-bindings/clock/imx6qdl-clock.h |  3 ++-
> >  include/dt-bindings/clock/imx6sl-clock.h  |  3 ++-
> >  include/dt-bindings/clock/imx6sx-clock.h  |  3 ++-
> >  9 files changed, 31 insertions(+), 7 deletions(-)
> > 
> > -- 
> > 1.9.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RESEND PATCH 0/3 v6] Add Mediatek MT8173 cpufreq driver

[Viresh Kumar]

On 26-08-15, 09:25, Pi-Cheng Chen wrote:
> The [3/3] is based on Mediatek SoC maintainer tree[1] and the patch which
> introduce a new clock type[2] consumed by MT8173 cpufreq driver. So it will
> cause some conflicts if it goes through your tree. I am not sure how this
> should be handled, but should it be merged through Mediatek SoC maintainer
> tree?

Just get that applied to MTK tree, it doesn't have any dependency on
rest of the patches for build/boot. The only thing is that cpufreq
wouldn't work and it will work as soon as Rafael's and MTK's trees are
merged by Linus.

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

Re: [PATCH v2 7/7] ARM: bcm2835: Add VC4 to the device tree.

[Stephen Warren]

On 08/18/2015 03:54 PM, Eric Anholt wrote:
> VC4 is the GPU (display and 3D) present on the 2835.

This patch and patch 1 seem OK to me, although I'll withhold any ack
until the DT binding design discussion with Rob has been resolved. I
haven't looked at the OF graph bindings he mentioned so have no clue
about his suggestions.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 6/7] ARM: bcm2835: Add the DDC I2C controller to the device tree.

[Stephen Warren]

On 08/18/2015 03:54 PM, Eric Anholt wrote:
> We need to use it for getting video modes over HDMI.

This patch,
Acked-by: Stephen Warren 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/5] spi: bcm2835: new driver implementing auxiliar spi1/spi2 on the bcm2835 soc

[Stephen Warren]

On 08/24/2015 02:40 AM, ker...@martin.sperl.org wrote:
> From: Martin Sperl 

Patch description?

>  arch/arm/configs/bcm2835_defconfig |1 +
>  drivers/spi/Kconfig|   12 +
>  drivers/spi/Makefile   |1 +
>  drivers/spi/spi-bcm2835aux.c   |  506 
> 

A change to the defconfig would be applied by the RPi maintainers, and a
change to drivers/spi by the SPI maintainers. Those need to be in
different patches.

> diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c

> +static int bcm2835aux_spi_probe(struct platform_device *pdev)

> + clk_prepare_enable(bs->clk);

Error checking?

> + /* enable HW block */
> + bcm2835aux_enable(&pdev->dev, ENABLE_PROPERTY);

The return value needs to be error-checked, so that deferred probe can
work, and so other kinds of errors can be detected. Wasn't this correct
in a previous patch version?

Note that I didn't review any code besides probe(), remove() and the
driver boiler-plate that refers to those functions.

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

Re: [PATCH v4 1/5] soc: bcm2835: auxiliar devices enable infrastructure

[Stephen Warren]

On 08/24/2015 02:40 AM, ker...@martin.sperl.org wrote:
> From: Martin Sperl 
> 
> The bcm2835 SOC contains 3 auxiliar devices (spi1, spi2 and uart1)
> that all are enabled via a shared register.
> 
> To serialize access to this shared register this soc-driver
> is created that implements:
>   bcm2835aux_enable(struct device *dev, const char *property);
>   bcm2835aux_disable(struct device *dev, const char *property);
> 
> Which will read the property from the device tree of the device
> and enable/disable that specific device as per device tree.
> 
> First use of this api will be spi-bcm2835aux.

> diff --git a/drivers/soc/bcm/bcm2835-aux.c b/drivers/soc/bcm/bcm2835-aux.c

> +static void *bcm2835aux_find_base(struct device *dev, const char *property)
> +{
> + struct device *found = NULL;
> + struct device_node *np;
> +
> + /* get the phandle of the device */
> + np = of_parse_phandle(dev->of_node, property, 0);
> + if (!np) {
> + dev_err(dev, "missing property %s\n", property);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + /* now find the device it points to */
> + found = driver_find_device(&bcm2835aux_driver.driver, NULL,
> +np, bcm2835aux_dev_match);
> + if (!found) {
> + dev_err(dev, "device for phandle of %s not found\n",
> + property);
> + return ERR_PTR(-ENODEV);

That should return ERR_PTR(-EPROBE_DEFER) so that client drivers know
when to defer their own probe, and not print an error. This is an
expected condition during probing. I could have sworn this was correct
in a previous patch revision.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] dt/bindings: bcm2835: Add binding documentation for auxiliar spi devices

[Stephen Warren]

On 08/24/2015 02:40 AM, ker...@martin.sperl.org wrote:
> From: Martin Sperl 

Patch description?

> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt 
> b/Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt

> +Required properties:
> +- compatible: Should be "brcm,bcm2835-aux-spi".
> +- reg: Should contain register location and length for the spi block
> +   as well as for the common aux block control

Is that meant to imply that reg should contain a single value that
covers both the common aux registers and the SPI device, or two separate
values, one for the aux common registers and another for the SPI device?
Neither of those options sound correct. I would expect only a single
entry which covered solely the SPI registers. The common aux registers
are owned by the other brcm,bcm2835-aux binding.

> +Example:
> +
> +spi1@7e215080 {
> + compatible = "brcm,bcm2835-aux-spi";
> + reg = <0x7e215080 0x40>;

That seems to match what I'd expect, but doesn't correspond to the
description above.

> +/* the necessary syscon config referenced above*/
> +aux_enable: aux_enable@0x7e215004 {

It's not a "syscon"...

> +Note that it also requires the GPIOs to be set up with the
> +correct ALT-functions.
> +
> +For spi1 the following pins need to be set as:
> +* ALT4: 19, 20, 21 (MISO, MOSI, SCK)
> +
> +For spi2 the following pins need to be set as:
> +* ALT4: 40, 41, 42 (MISO, MOSI, SCK)
> +
> +CS-GPIOS need to get set as output - typically:
> +* spi1: 18, 17, 16 (CS0, CS1, CS2)
> +* spi2: 43, 44, 45 (CS0, CS1, CS2)

That's generally true of any HW block, and has nothing to do with the
binding for the device. I would suggest removing that chunk of text.

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

Re: [PATCH v4 3/5] dt/bindings: bcm2835: add binding documentation for bcm2835-aux

[Stephen Warren]

On 08/24/2015 02:40 AM, ker...@martin.sperl.org wrote:
> From: Martin Sperl 

Patch description?

> diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt 
> b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt

> +Required properties:
> +- compatible: Should be "brcm,bcm2835-aux".
> +- reg: Should contain register location and length for the
> +   enable register

As I mentioned before, why not all the aux registers (that aren't part
of a sub-device like SPI).

> +Example:
> +
> +aux_enable: aux_enable@0x7e215004 {
> + compatible = "bcrm,bcm2835-aux";
> + reg = <0x7e215004 0x04>;

I'd expect that to be <0x7e215000 0x8>;
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] ARM: uniphier: add outer cache support

[Masahiro Yamada]

Hi Arnd,


2015-08-25 4:59 GMT+09:00 Arnd Bergmann :
> On Monday 24 August 2015 11:18:10 Masahiro Yamada wrote:
>> diff --git 
>> a/Documentation/devicetree/bindings/arm/uniphier/cache-uniphier.txt 
>> b/Documentation/devicetree/bindings/arm/uniphier/cache-uniphier.txt
>> new file mode 100644
>> index 000..6428289
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/uniphier/cache-uniphier.txt
>> @@ -0,0 +1,30 @@
>> +UniPhier outer cache controller
>> +
>> +UniPhier SoCs are integrated with a level 2 cache controller that resides
>> +outside of the ARM cores, some of them also have a level 3 cache controller.
>> +
>> +Required properties:
>> +- compatible: should be one of the followings:
>> +   "socionext,uniphier-l2-cache"   (L2 cache)
>> +   "socionext,uniphier-l3-cache"   (L3 cache)
>> +- reg: offsets and lengths of the register sets for the device.  It should
>> +  contain 3 regions: control registers, revision registers, operation
>> +  registers, in this order.
>> +
>> +The L2 cache must exist to use the L3 cache; adding only an L3 cache device
>> +node to the device tree causes the initialization failure of the whole outer
>> +cache system.
>>
>
> How much does this outer cache have in common with the l2x0/pl310 cache
> controller model?

Nothing.

This outer cache is not a variant of l2x0/pl310.
It was designed only for our SoCs from scratch.


> Would it make sense to at least share the
> common entry point at l2x0_of_init() so you don't need to call it from
> the platform file?

I do not think so.


l2x0_of_init() checks L2X0_AUX_CTRL register,
but the cache-uniphier does not have such register,
so the boot code crashes.



BTW, what makes l2x0_of_init() so special?

Only L2x0/L310 and variants can be initialized
directly from init_IRQ().

Moreover, outer-cache init seems to be unrelated to
IRQ init.


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Haojian Zhuang]

On Wed, 2015-08-26 at 00:00 +0800, Leo Yan wrote:
> On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote:
> > On Tue, 2015-08-25 at 11:42 +0100, Mark Rutland wrote:
> > > > > Are you then going to hack GRUB, release a special HiKey version of
> > > > > GRUB, not support any other versions, and still can your firmware
> > > > > UEFI?
> > > > 
> > > > I don't need to hack GRUB at all.
> > > 
> > > Then it is working for you by pure chance alone.
> > > 
> > > Please listen to the advice you are being given here; we're trying to
> > > ensure that your platform functions (and continues to function) as best
> > > it can.
> > 
> > Since we discussed a lot on this, let's make a conclusion on it.
> > 
> > 1. UEFI could append the reserved buffer in it's memory mapping.
> > 2. These reserved buffer must be declared in DT, since we also need to
> >support non-UEFI (uboot) at the same time.
> > 3. Mailbox node should reference reserved buffer by phandle in DT. Then
> >map the buffer as non-cacheable in driver.
> > 4. These reserved buffer must use "no-map" property since it should be
> >non-cacheable in driver.
> 
> For more specific discussion for DTS, i list two options at here;
> 
> - Option 1: just simply reserve memory regions through memory node,
>   and mailbox node will directly use the buffer through reg ranges;
> 
> - Option 2: use reserved-memory and mailbox node will refer phandle
>   of reserved-memory;
> 
> These two options both can work well with UEFI and Uboot, but option 1
> is more simple and straightforward; so i personally prefer it. But
> look forwarding your guys' suggestion.
> 
> Option 1:
> 
>   memory@0 {
>   device_type = "memory";
>   reg = <0x 0x 0x 0x05e0>,
> <0x 0x05f0 0x 0x00eff000>,
> <0x 0x06e0 0x 0x0060f000>,
> <0x 0x0741 0x 0x38bf>;
>   };
> 
> [...]
> 
>   mailbox: mailbox@f751 {
>   #mbox-cells = <1>;
>   compatible = "hisilicon,hi6220-mbox";
>   reg = <0x0 0xf751 0x0 0x1000>, /* IPC_S */
> <0x0 0x06dff800 0x0 0x0800>; /* Mailbox buffer */
>   interrupts = ;
>   };
> 
> Option 2:
> 
>   memory@0 {
>   device_type = "memory";
>   reg = <0x0 0x0 0x0 0x4000>;
>   };
> 
>   reserved-memory {
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> 
>   mcu_reserved: mcu_reserved@06dff000 {
>   no-map;
>   reg = <0x0 0x06dff000 0x0 0x1000>,  /* MCU mailbox 
> buffer */
> <0x0 0x05e0 0x0 0x0010>,  /* MCU firmware 
> buffer */
> <0x0 0x0740f000 0x0 0x1000>;  /* MCU firmware 
> section */
>   };
>   };
> 
> [...]
> 
>   mailbox: mailbox@f751 {
>   #mbox-cells = <1>;
>   compatible = "hisilicon,hi6220-mbox";
>   reg = <0x0 0xf751 0x0 0x1000>; /* IPC_S */
>   memory-region = <&mcu_reserved>;   /* Mailbox buffer */
>   interrupts = ;
>   };

I prefer the second one. From my view, memory node should only describe
the hardware information of memory.

Regards
Haojian

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

Re: [RESEND PATCH 0/3 v6] Add Mediatek MT8173 cpufreq driver

[Pi-Cheng Chen]

Hi Rafael,

On Wed, Aug 26, 2015 at 7:01 AM, Rafael J. Wysocki  wrote:
> On Tuesday, August 25, 2015 10:10:44 AM Pi-Cheng Chen wrote:
>> On Mon, Aug 17, 2015 at 5:24 PM, Pi-Cheng Chen  
>> wrote:
>> > MT8173 is a ARMv8 based SoC with 2 clusters. All CPUs in a single cluster
>> > share the same power and clock domain. This series tries to add cpufreq 
>> > support
>> > for MT8173 SoC. The v6 of this series is resent with Acks added.
>>
>> Hi Rafael,
>>
>> Not sure if I has missed the merge window.
>> Do I have chance to have this series merged for 4.3?
>
> Yes, it should make it.
>
>> Would you please take [1,2] of this series?

Thanks.

>
> I'm not sure what you mean.  Are you withdrawing the [3/3]?

The [3/3] is based on Mediatek SoC maintainer tree[1] and the patch which
introduce a new clock type[2] consumed by MT8173 cpufreq driver. So it will
cause some conflicts if it goes through your tree. I am not sure how this
should be handled, but should it be merged through Mediatek SoC maintainer
tree?

@Matthias?

Thanks

Best Regards,
Pi-Cheng

[1] https://github.com/mbgg/linux-mediatek.git v4.2-next/arm64
[2] http://article.gmane.org/gmane.linux.kernel/2021379

>
> Thanks,
> Rafael
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of/irq: export of_get_irq_byname()

[Stephen Rothwell]

Hi Dmitry,

On Tue, 25 Aug 2015 17:04:02 -0700 Dmitry Torokhov  
wrote:
>
> Similarly to of_get_irq(), let's export of_irq_get_byname(), so if a bus core
> can be compiled as a module (such as I2C) it can have access to the symbol.
> 
> Reported-by: Stephen Rothwell 
> Reported-by: kbuild test robot 
> Signed-off-by: Dmitry Torokhov 

I applied that to linux-next today.  Pelase get it applied to the i2c
tree (with appropriate acks).

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] of/irq: export of_get_irq_byname()

[Dmitry Torokhov]

Similarly to of_get_irq(), let's export of_irq_get_byname(), so if a bus core
can be compiled as a module (such as I2C) it can have access to the symbol.

Reported-by: Stephen Rothwell 
Reported-by: kbuild test robot 
Signed-off-by: Dmitry Torokhov 
---
 drivers/of/irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 7ef837e..1a7911d 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -446,6 +446,7 @@ int of_irq_get_byname(struct device_node *dev, const char 
*name)
 
return of_irq_get(dev, index);
 }
+EXPORT_SYMBOL_GPL(of_irq_get_byname);
 
 /**
  * of_irq_count - Count the number of IRQs a node uses
-- 
2.5.0.457.gab17608


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

Re: [RESEND PATCH v16 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs

[Doug Anderson]

Kevin,

On Tue, Aug 25, 2015 at 3:45 PM, Kevin Hilman  wrote:
>> To put things in a
>> concrete way, for pd_vio we'd go through the entire device tree
>> ourselves and find all properties that look like "power-domains =
>> <&power RK3288_PD_VIO>;".  We'd then find the parent of those
>> properties and look for a property named "clocks".  We'd then iterate
>> over all those clocks and turn those on.  Did I get that right?
>
> ... but you make it sound like more work than it is.  The genpd already
> keeps a list of devices that refer to the power domain.  In fact, the
> genpd 'attach' method can be platform-specific, so could be used to keep
> track of a list (or a subset) of clocks which are needed for reset.

OK.  I'll need to dig through and figure out how this works.  So this
list will include devices whose drivers are compiled out and also
devices that are marked as 'status = "disabled";' in the device tree?
It would need to do this in case someone had a board that didn't use
one of these peripherals (since we still need to clock it as we make
the domain go up and down).

I took a quick gander and didn't see code that would do this.  Can you
give me a quick pointer?

We also need to really make sure that there are no probe order issues...


>> 2. If we absolutely need to turn all clocks and we get clocks from
>> device tree nodes on then it means we need device tree nodes for every
>> device in the domain.
>
> Isn't that the end goal?

In the ideal world, yes.  ...but it's possibly years before all
drivers are added, or possibly they never will be.  To name a concrete
example, unless I'm mistaken, the "hardware crypto" device present in
exynos5250 still isn't anywhere in upstream despite several years
having passed.  There are just so many devices in these SoCs that
aren't used.


>> These would be needed even if there are no
>> accepted bindings for this device yet.  So we'd need to do one of: A)
>> Block power domain patches on feature complete bindings for all
>> drivers; B) Make up non-approved compatible strings for all devices
>> and throw them into the DTS; C) Add nodes in the DTS without a
>> compatible string just to satisfy the power domain requirements.  None
>> of these seem terribly appealing.
>
> well, I think we can be slightly more accomodating than that and go for
> somewhere in between:
>
> D) In the power-domain DTS, clearly describe why each clock is needed by
> the power-domain.  In particular list out clocks that are not device
> clocks (and why they need to be asserted for reset) and separate those
> from device clocks which are only listed in the power-domain because
> there is not *yet* a driver/binding for that device node.
>
> Doing it that way also makes it clear that when a new driver/binding is
> added, the clocks should be removed from the power-domain node and put
> into the device node.
>
> Also, this addresses my primary concern that the DTS acurately describes
> the hardware.  IOW, in hardware, most of these clocks are are properties
> of devices, not power-domains, and the DT should reflect that.
>
> IMO, if it's not describing the hardware, and is a placeholder until a
> driver/binding is in place, it should be properly documented.

Agreed that more documentation about why each clock is needed would go
a long way to helping.

I'd say that the clocks are properties of the device, but I'd again
assert that not all clocks that are owned by the device are relevant
to turning power domains on and off.  If you really wanted to be
correct, you'd add a property like this to devices:

power-on-clocks = <..>;

...this would be a list of clocks needed to power on the device
properly.  If such a property was added that would erase my objection
to that.  Note that you could say that if "power-on-clocks" wasn't
present then you could fall back to "clocks".  I do worry a little bit
about over-engineering, so I guess this could just be a future
improvement...


>> 3. It is entirely possible that there are clocks that will be listed
>> in the individual devices that aren't needed for powering on the power
>> domain.  I'd tend to believe that PCLK_EDP_CTRL (the pixel clock)
>> doesn't really need to be turned on when adjusting the "VIO" power
>> domain.  Right now Caesar has it listed, but it probably isn't needed
>> (Caesar: can you confirm?).
>
> Yes, I suspect there are several of those, which is also why I'd like to
> see the clocks in the power-domain node described in detail, and exacly
> why they're needed to be enabled.

I guess my point was that this was a reason for doing something like
"power-on-clocks" because it would let you avoid turning on
PCLK_EDP_CTRL (which is listed in the "clocks" of the EDP device but
maybe not needed while powering on/off the domain).  ...or just have
the list as part of the power domain.  :-P


> Also, in the current proposed DTS, clocks that are listed in both device
> nodes and the power domain are also suspicous, especially when the
> device

Re: [RESEND PATCH v16 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs

[Dmitry Torokhov]

On Tue, Aug 25, 2015 at 3:45 PM, Kevin Hilman  wrote:
> Doug Anderson  writes:
>
>> To put things in a
>> concrete way, for pd_vio we'd go through the entire device tree
>> ourselves and find all properties that look like "power-domains =
>> <&power RK3288_PD_VIO>;".  We'd then find the parent of those
>> properties and look for a property named "clocks".  We'd then iterate
>> over all those clocks and turn those on.  Did I get that right?
>
> ... but you make it sound like more work than it is.  The genpd already
> keeps a list of devices that refer to the power domain.  In fact, the
> genpd 'attach' method can be platform-specific, so could be used to keep
> track of a list (or a subset) of clocks which are needed for reset.

That is not really workable: the attach and detach happen in
probe/remove path; if you do not have driver for the device you will
miss the clocks for it. And the quirk of this SoC is that we need to
turn all clocks during domain transition, regardless of whether there
is a driver for all devices.

Thanks.

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

[PATCH v3 1/1][Resend] misc: sram: add dev_pm_ops to support module power gate

[Shenwei Wang]

When system goes into low power states like SUSPEND_MEM and
HIBERNATION, the hardware IP block may be powered off to reduce
the power consumption. This power down will lost all the
data inside the ram. This patch added the dev_pm_ops and
implemented two callbacks: suspend_noirq and resume_noirq, which
will save the data in the on-chip-ram right before power down
and restore it after system resumes.

A new property string named "can-power-gate" is added to
the devicetree bindings too.

Based-on-a-patch-by: Anson Huang 
Signed-off-by: Shenwei Wang 

---
Change log:

PATCH v3
Removed the unnecessary clk_enable/clk_disable.

PATCH v2
Use vmalloc to allocate the SRAM backup memory.
Code clean up.

 Documentation/devicetree/bindings/misc/sram.txt |  2 ++
 drivers/misc/sram.c | 42 +
 2 files changed, 44 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/sram.txt 
b/Documentation/devicetree/bindings/misc/sram.txt
index 36cbe5a..1170086 100644
--- a/Documentation/devicetree/bindings/misc/sram.txt
+++ b/Documentation/devicetree/bindings/misc/sram.txt
@@ -33,6 +33,8 @@ Optional properties in the area nodes:

 - compatible : standard definition, should contain a vendor specific string
in the form ,[-]
+- can-power-gate: a property to tell the driver that the sram can support
+   power gate

 Example:

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 15c33cc..db9f1fa 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -30,7 +30,9 @@

 struct sram_dev {
struct device *dev;
+   void *power_off_save;
void __iomem *virt_base;
+   u32 size;

struct gen_pool *pool;
struct clk *clk;
@@ -156,6 +158,33 @@ static int sram_reserve_regions(struct sram_dev *sram, 
struct resource *res)
return ret;
 }

+static int sram_suspend_noirq(struct device *dev)
+{
+   struct platform_device *pdev = to_platform_device(dev);
+   struct sram_dev *sram = platform_get_drvdata(pdev);
+
+   if (!sram->power_off_save)
+   return 0;
+
+   /* Save necessary regs */
+   memcpy(sram->power_off_save, sram->virt_base, sram->size);
+
+   return 0;
+}
+
+static int sram_resume_noirq(struct device *dev)
+{
+   struct platform_device *pdev = to_platform_device(dev);
+   struct sram_dev *sram = platform_get_drvdata(pdev);
+
+   if (!sram->power_off_save)
+   return 0;
+
+   memcpy(sram->virt_base, sram->power_off_save, sram->size);
+
+   return 0;
+}
+
 static int sram_probe(struct platform_device *pdev)
 {
struct sram_dev *sram;
@@ -176,6 +205,7 @@ static int sram_probe(struct platform_device *pdev)
}

size = resource_size(res);
+   sram->size = size;

if (!devm_request_mem_region(sram->dev, res->start, size, pdev->name)) {
dev_err(sram->dev, "could not request region for resource\n");
@@ -203,6 +233,9 @@ static int sram_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, sram);

+   if (of_property_read_bool(pdev->dev.of_node, "can-power-gate"))
+   sram->power_off_save = vmalloc(size);
+
dev_dbg(sram->dev, "SRAM pool: %zu KiB @ 0x%p\n",
gen_pool_size(sram->pool) / 1024, sram->virt_base);

@@ -219,6 +252,9 @@ static int sram_remove(struct platform_device *pdev)
if (sram->clk)
clk_disable_unprepare(sram->clk);

+   if (sram->power_off_save)
+   vfree(sram->power_off_save);
+
return 0;
 }

@@ -229,10 +265,16 @@ static const struct of_device_id sram_dt_ids[] = {
 };
 #endif

+static const struct dev_pm_ops sram_pm_ops = {
+   .suspend_noirq = sram_suspend_noirq,
+   .resume_noirq = sram_resume_noirq,
+};
+
 static struct platform_driver sram_driver = {
.driver = {
.name = "sram",
.of_match_table = of_match_ptr(sram_dt_ids),
+   .pm = &sram_pm_ops,
},
.probe = sram_probe,
.remove = sram_remove,
--
2.5.0.rc2


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

Re: [PATCH 2/5] ARM: NSP: add minimal Northstar Plus device tree

[Olof Johansson]

Hi,

I'm not sure what the strategy behind your cc:ing on this patch set
is. I only got a couple of them in my inbox, and this one wasn't one
of them. :)

On Thu, Aug 20, 2015 at 10:46 AM, Jon Mason  wrote:
> Add a very minimalistic set of Northstar Plus Device Tree files which
> describes the SoC and the BCM958625 implementation.  The perpherials
> described are:
>
> ARM Cortex A9 CPU
> 2 8250 UARTs
> ARM GIC
> PL310 L2 Cache
> ARM A9 Global timer
>
> Signed-off-by: Jon Mason 
> Signed-off-by: Kapil Hali 
> Reviewed-by: Ray Jui 
> Reviewed-by: Scott Branden 

Seeing reviewed-by already attached to a v1 of a patchset has limited
value for someone on the outside.

Reviewed-by is one of those tags that has a value that's mostly
dependent on who it comes from. By not actually seeing the review and
the feedback provided (and revisions made), less data is provided to
tell if it's a valuable review or not.

Also, if you're posting the code you should probably have your name
below Kapil's, since you're the one signing off the origin of the
code. See Documentation/SubmittingPatches.txt for details on what
S-o-b actually means.


> --- /dev/null
> +++ b/arch/arm/boot/dts/bcm-nsp.dtsi
> @@ -0,0 +1,105 @@
> +/*
> + *  BSD LICENSE
> + *
> + *  Copyright(c) 2015 Broadcom Corporation.  All rights reserved.
> + *
> + *  Redistribution and use in source and binary forms, with or without
> + *  modification, are permitted provided that the following conditions
> + *  are met:
> + *
> + ** Redistributions of source code must retain the above copyright
> + *  notice, this list of conditions and the following disclaimer.
> + ** Redistributions in binary form must reproduce the above copyright
> + *  notice, this list of conditions and the following disclaimer in
> + *  the documentation and/or other materials provided with the
> + *  distribution.
> + ** Neither the name of Broadcom Corporation nor the names of its
> + *  contributors may be used to endorse or promote products derived
> + *  from this software without specific prior written permission.
> + *
> + *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */

I'm not sure we've seen BSD-only submissions before. I'll let DT
maintainers (or Ian) speak up in case this would cause problems.


> +
> +#include 
> +#include 
> +
> +#include "skeleton.dtsi"
> +
> +/ {
> +   compatible = "brcm,nsp";
> +   model = "Broadcom Northstar Plus SoC";
> +   interrupt-parent = <&gic>;
> +
> +   cpus {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   cpu@0 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a9";
> +   next-level-cache = <&L2>;
> +   reg = <0x0>;
> +   };
> +   };
> +
> +   clocks {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   ranges;
> +
> +   periph_clk: periph_clk {
> +   compatible = "fixed-clock";
> +   #clock-cells = <0>;
> +   clock-frequency = <5>;
> +   };
> +   };
> +
> +   uart0: serial@18000300 {
> +   compatible = "ns16550a";
> +   reg = <0x18000300 0x100>;
> +   interrupts = ;
> +   clock-frequency = <62499840>;
> +   status = "disabled";
> +   };
> +
> +   uart1: serial@18000400 {
> +   compatible = "ns16550a";
> +   reg = <0x18000400 0x100>;
> +   interrupts = ;
> +   clock-frequency = <62499840>;
> +   status = "disabled";
> +   };
> +
> +   gic: interrupt-controller@19021000 {
> +   compatible = "arm,cortex-a9-gic";
> +   #interrupt-cells = <3>;
> +   #address-cells = <0>;
> +   interrupt-controller;
> +   reg = <0x19021000 0x1000>,
> + <0x19020100 0x100>;
> +   };
> +
> +   L2: l2-cache {
> +   compatible = "arm,pl310-cache";
> +   reg = <0x19022000 0x1000>;
> +   cache-unified;

Re: [PATCH 1/7] drm/vc4: Add devicetree bindings for VC4.

[Rob Herring]

On Tue, Aug 25, 2015 at 3:42 PM, Rob Clark  wrote:
> On Mon, Aug 24, 2015 at 9:47 AM, Rob Herring  wrote:
>> On Mon, Aug 17, 2015 at 1:30 PM, Eric Anholt  wrote:
>>> Stephen Warren  writes:
>>>
 On 08/12/2015 06:56 PM, Eric Anholt wrote:
> Signed-off-by: Eric Anholt 

 This one definitely needs a patch description, since someone might not
 know what a VC4 is, and "git log" won't show the text from the binding
 doc itself. I'd suggest adding the initial paragraph of the binding doc
 as the patch description, or more.

> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt 
> b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>>
> +- hvss: List of references to HVS video scalers
> +- encoders: List of references to output encoders (HDMI, SDTV)

 Would it make sense to make all those nodes child node of the vc4
 object. That way, there's no need to have these lists of objects; they
 can be automatically built up as the DT is enumerated. I know that e.g.
 the NVIDIA Tegra host1x binding works this way, and I think it may have
 been inspired by other similar cases.
>>>
>>> I've looked at tegra, and the component system used by msm appears to be
>>> nicer than it.  To follow tegra's model, it looks like I need to build
>>> this extra bus thing corresponding to host1x that is effectively the
>>> drivers/base/component.c code, so that I can get at vc4's structure from
>>> the component drivers.
>>>
 Of course, this is only appropriate if the HW modules really are
 logically children of the VC4 HW module. Perhaps they aren't. If they
 aren't though, I wonder what this "vc4" module actually represents in HW?
>>>
>>> It's the subsystem, same as we use a subsystem node for msm, sti,
>>> rockchip, imx, and exynos.  This appears to be the common model of how
>>> the collection of graphics-related components is represented in the DT.
>>
>> I think most of these bindings are wrong. They are grouped together
>> because that is what DRM wants not because that reflects the h/w. So
>> convince me this is one block, not that it is what other people do.
>
> I think, when it comes to more complex driver subsystems (like drm in
> particular) we have a bit of mismatch between how things look from the
> "pure hw ignoring sw" perspective, and the "how sw and in particular
> userspace expects things" perspective.  Maybe it is less a problem in
> other subsystems, where bindings map to things that are only visible
> in the kernel, or well defined devices like uart or sata controller.
> But when given the choice, I'm going to err on the side of not
> confusing userspace and the large software stack that sits above
> drm/kms, over dt purity.

I wasn't implying that this should get exposed to userspace as
components. V4L2 has gone that route with media controller and
sub-devs. Perhaps that is needed for DRM, perhaps not. For the moment,
I definitely agree the kernel should hide most/all of those details,
but I don't think that means DT has to hide the details or know what
components are handled by a single driver. My point was that on the DT
side we have a mixture of OF graph usage, parent-child nodes or custom
phandles (this case) to describe the relationships between h/w
components. That's not necessarily wrong, but we should have some
rules around how certain relationships are described. Then in the
drivers we have a mixture of deferred probe, component API, and custom
inter-module APIs to control init order. We then have a mixture of all
those which leads to very few if any drivers having the same overall
structure that could be shared. Should we mandate using the component
API for h/w that is discrete blocks? Should we throw out the component
API for something else? Can we tie the graph parsing and component API
together with common code?


> Maybe it would be nice to have a sort of dt overlay that adds the bits
> needed to tie together hw blocks that should be assembled into a
> single logical device for linux and userspace (but maybe not some
> other hypothetical operating system).  But so far that doesn't exist.

OF graph is supposed to do this. OF graph is a double edged sword. It
is very flexible, but then each platform can do something different.
We need to have some level of requirements around how the OF graph is
used. As an example, any system with an HDMI connector should have an
"hdmi-connector" compatible node or encoder/bridge chips/blocks must
have certain ports defined.


> All we have is a hammer (devicetree), everything looks like a nail.
> End result is we end up adding some things in the bindings which
> aren't purely about the hw.  Until someone invents a screwdriver, I'm
> not sure what else to do.  In the end, other hypothetical OS is free
> to ignore those extra fields in the bindings if it doesn't need them.
> So meh?

We really want to err on the side of fewer bindings, not more as once
used 

Re: [RFC PATCH 1/7] DMA: tegra-apb: Correct runtime-pm usage

[Rafael J. Wysocki]

On 8/25/2015 11:37 AM, Jon Hunter wrote:

On 25/08/15 01:04, Rafael J. Wysocki wrote:

On Monday, August 24, 2015 07:51:43 PM Vinod Koul wrote:

On Mon, Aug 24, 2015 at 02:22:49PM +0100, Jon Hunter wrote:

On 24/08/15 10:22, Vinod Koul wrote:

On Mon, Aug 24, 2015 at 09:47:13AM +0100, Jon Hunter wrote:

On 23/08/15 15:17, Vinod Koul wrote:

On Tue, Aug 18, 2015 at 02:49:09PM +0100, Jon Hunter wrote:


@@ -1543,7 +1531,7 @@ static int tegra_dma_pm_suspend(struct device *dev)
int ret;
  
  	/* Enable clock before accessing register */

-   ret = tegra_dma_runtime_resume(dev);
+   ret = pm_runtime_get_sync(dev);

why is this required ?

Because the clock could be disabled when this function is called. This
function saves the DMA context so that if the context is lost during
suspend, it can be restored.

Have you verified this? Coz my understanding is that when PM does suspend it
will esnure you are runtime resume if runtime suspended and then will do
suspend.
So you do not need to do above

I see what you are saying. I did some testing with ftrace today to trace
rpm and suspend/resume calls. If the dma controller is runtime suspended
and I do not call pm_runtime_get_sync() above then I do not see any
runtime resume of the dma controller prior to suspend. Now I was hoping
that this would cause a complete kernel crash but it did not and so the
DMA clock did not appear to be needed here (at least on the one board I
tested). However, I would not go as far as to remove this and prefer to
keep as above.

Okay am adding Rafael here for his recommendations.

Well, and what is the question I'm supposed to answer, exactly?

I was in Seattle last week, so haven't been following this closely.


I have tested in past and if my driver was runtime suspended we were resumed
prior to being suspended. So I am not sure why you did not see that
behaviour, and if that is right we don't need to force resume here

We're adding code for skipping runtime-resume-before-system-suspend, because
it is not desirable in general.

The rule of thumb is that if you know you need to change the device's settings
(eg. because of wakeup being enabled or not) for system suspend and that
requires the device to be resumed, resume it.  It can stay suspended
otherwise.

Thanks Rafael.

Vinod, thinking about this some more, I am wondering if it is just
better to get rid of the suspend/resume callbacks and simply handling
the state in the runtime suspend/resume callbacks. I think that would be
safe too, because once the clock has been disabled, then who knows what
the context state will be.


One caveat here: system suspend may be invoked at any time, so you need 
to ensure that the device is properly suspended when that happens.


I believe you at least need a ->suspend callback for that.

Cheers,
Rafael

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

Re: [RESEND PATCH v16 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs

[Kevin Hilman]

Doug Anderson  writes:

> Kevin,
>
> On Tue, Aug 25, 2015 at 2:07 PM, Kevin Hilman  wrote:
>> Caesar Wang  writes:
>>
>>> We can add more domains node in the future.
>>> This patch add the needed clocks into power-controller.
>>> As the discuess about all the device clocks being listed in
>>> the power-domains itself.
>>>
>>> There are several reasons as follows:
>>>
>>> Firstly, the clocks need be turned off to save power when
>>> the system enter the suspend state. So we need to enumerate
>>> the clocks in the dts. In order to power domain can turn on and off.
>>
>> Yes, but this is the job of device drivers which are runtime PM adapted
>> to gate their own clocks.  I agree these clocks need to be enumerated in
>> the DTS, but they should be in the device nodes.
>
> I _think_ what Caesar means is that the alternative to this patch is
> to leave the clocks on all the time as they were during the early days
> of Rockchip (AKA last year).  If the clocks are on all the time then
> the power domain patches can work fine.  However, once you start
> letting clocks turn off then you need to make sure that the power
> domain code turns the back on temporarily.

Yup, I understand that part, and many SoCs have this same "feature".

>>> Secondly, the reset-circuit should reset be synchronous on RK3288,
>>> then sync revoked. So we need to enable clocks of all devices.
>>> In other words, we have to enable the clocks before you operate them
>>> if all the device clocks are included in someone domians.
>>
>> Yes, this is pretty common for reset.
>>
>>> Someone wish was to get the clocks by reading the clocks from the
>>> device nodes, We can do that but we can solve the above issues.
>>
>> I don't follow this sentence.  Are you saying doing that will not solve
>> the above issues?  Why not?  Please explain.
>>
>> If there are non-device clocks that also need to be enabled before
>> asserting reset, then those are candidates for the power-domain node,
>> but not device clocks.
>
> It's been a long time and I don't know that I've reviewed every
> revision of this series, but I think there was a proposal that we
> shouldn't list clocks here.  Instead we should search through and find
> all devices that refer to this power domain, reach in and find their
> clocks, and turn them on.  Did I get that right?  

Yes...

> To put things in a
> concrete way, for pd_vio we'd go through the entire device tree
> ourselves and find all properties that look like "power-domains =
> <&power RK3288_PD_VIO>;".  We'd then find the parent of those
> properties and look for a property named "clocks".  We'd then iterate
> over all those clocks and turn those on.  Did I get that right?

... but you make it sound like more work than it is.  The genpd already
keeps a list of devices that refer to the power domain.  In fact, the
genpd 'attach' method can be platform-specific, so could be used to keep
track of a list (or a subset) of clocks which are needed for reset.

> The above doesn't seem like a terribly great idea to me for a number
> of reasons, including:
>
> 1. If I remember correctly, it's important to turn on clocks for
> devices even if they're not something you're using / have a driver
> for.  If you don't then the device won't get reset properly and this
> can affect things like suspend/resume because the hardware in the SoC
> will query all devices at suspend time to make sure they're ready.  

Correct.  This condition also exists in the clock framework when unused
clocks are disabled, or if you have drivers but PM_RUNTIME is disabled
(which can happen from userspace on a per-device basis) so it needs to
be understood and managed already.

> If
> a device is wedged because its clock wasn't on at the right them then
> it will cause problems.

Right.  I'm not arguing that the power domain doesn't have to deal with
device clocks. It has to for sync reset.  The objection I have have is
where these clocks are described.

> 2. If we absolutely need to turn all clocks and we get clocks from
> device tree nodes on then it means we need device tree nodes for every
> device in the domain.  

Isn't that the end goal?

> These would be needed even if there are no
> accepted bindings for this device yet.  So we'd need to do one of: A)
> Block power domain patches on feature complete bindings for all
> drivers; B) Make up non-approved compatible strings for all devices
> and throw them into the DTS; C) Add nodes in the DTS without a
> compatible string just to satisfy the power domain requirements.  None
> of these seem terribly appealing.

well, I think we can be slightly more accomodating than that and go for
somewhere in between:

D) In the power-domain DTS, clearly describe why each clock is needed by
the power-domain.  In particular list out clocks that are not device
clocks (and why they need to be asserted for reset) and separate those
from device clocks which are only listed in the power-domain because
there is not *yet* a driver/binding

Re: [RESEND PATCH 0/3 v6] Add Mediatek MT8173 cpufreq driver

[Rafael J. Wysocki]

On Tuesday, August 25, 2015 10:10:44 AM Pi-Cheng Chen wrote:
> On Mon, Aug 17, 2015 at 5:24 PM, Pi-Cheng Chen  
> wrote:
> > MT8173 is a ARMv8 based SoC with 2 clusters. All CPUs in a single cluster
> > share the same power and clock domain. This series tries to add cpufreq 
> > support
> > for MT8173 SoC. The v6 of this series is resent with Acks added.
> 
> Hi Rafael,
> 
> Not sure if I has missed the merge window.
> Do I have chance to have this series merged for 4.3?

Yes, it should make it.

> Would you please take [1,2] of this series?

I'm not sure what you mean.  Are you withdrawing the [3/3]?

Thanks,
Rafael

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

Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.

[vikas]

Hi,

On 08/21/2015 02:20 AM, Marek Vasut wrote:
> From: Graham Moore 
> 
> Add support for the Cadence QSPI controller. This controller is
> present in the Altera SoCFPGA SoCs and this driver has been tested
> on the Cyclone V SoC.
> 
> Signed-off-by: Graham Moore 
> Signed-off-by: Marek Vasut 
> Cc: Alan Tull 
> Cc: Brian Norris 
> Cc: David Woodhouse 
> Cc: Dinh Nguyen 
> Cc: Graham Moore 
> Cc: Vikas MANOCHA 
> Cc: Yves Vandervennet 
> Cc: devicetree@vger.kernel.org
> ---
>  drivers/mtd/spi-nor/Kconfig   |   11 +
>  drivers/mtd/spi-nor/Makefile  |1 +
>  drivers/mtd/spi-nor/cadence-quadspi.c | 1260 
> +
>  3 files changed, 1272 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c
> 
> V2: use NULL instead of modalias in spi_nor_scan call
> V3: Use existing property is-decoded-cs instead of creating duplicate.
> V4: Support Micron quad mode by snooping command stream for EVCR command
> and subsequently configuring Cadence controller for quad mode.
> V5: Clean up sparse and smatch complaints.  Remove snooping of Micron
> quad mode.  Add comment on XIP mode bit and dummy clock cycles.  Set
> up SRAM partition at 1:1 during init.
> V6: Remove dts patch that was included by mistake.  Incorporate Vikas's
> comments regarding fifo width, SRAM partition setting, and trigger
> address.  Trigger address was added as an unsigned int, as it is not
> an IO resource per se, and does not need to be mapped. Also add
> Marek Vasut's workaround for picking up OF properties on subnodes.
> V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*()
>   macros and replace them with functions. Get rid of unused variables.
> - Implement support for nor->set_protocol() to handle Quad-command,
>   this patch now depends on the following patch:
>   mtd: spi-nor: notify (Q)SPI controller about protocol change
> - Replace that cqspi_fifo_read() disaster with plain old readsl()
>   and cqspi_fifo_write() tentacle horror with pretty writesl().
> - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken.
> - Get rid of cqspi_find_chipselect() mess, instead just place the
>   struct cqspi_st and chipselect number into struct cqspi_flash_pdata
>   and set nor->priv to the struct cqspi_flash_pdata of that particular
>   chip.
> - Replace the odd math in calculate_ticks_for_ns() with DIV_ROUND_UP().
> - Make variables const where applicable.
> V8: - Implement a function to wait for bit being set/unset for a given
>   period of time and use it to replace the ad-hoc bits of code.
> - Configure the write underflow watermark to be 1/8 if FIFO size.
> - Extract out the SPI NOR flash probing code into separate function
>   to clearly mark what will soon be considered a boilerplate code.
> - Repair the handling of mode bits, which caused instability in V7.
> - Clean up the interrupt handling
> - Fix Kconfig help text and make the patch depend on OF and COMPILE_TEST.
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 89bf4c1..ed253a2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI
>   Flash. Enable this option if you have a device with a SPIFI
>   controller and want to access the Flash as a mtd device.
> 
> +config SPI_CADENCE_QUADSPI
> +   tristate "Cadence Quad SPI controller"
> +   depends on OF && COMPILE_TEST
> +   help
> + Enable support for the Cadence Quad SPI Flash controller.
> +
> + Cadence QSPI is a specialized controller for connecting an SPI
> + Flash over 1/2/4-bit wide bus. Enable this option if you have a
> + device with a Cadence QSPI controller and want to access the
> + Flash as an MTD device.
> +

the patch failed to apply, please rebase it to master.

Cheers,
Vikas

>  endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e5e..446c6b9 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_MTD_SPI_NOR)  += spi-nor.o
> +obj-$(CONFIG_SPI_CADENCE_QUADSPI)  += cadence-quadspi.o
>  obj-$(CONFIG_SPI_FSL_QUADSPI)  += fsl-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)+= nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c 
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> new file mode 100644
> index 000..8e024b8
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -0,0 +1,1260 @@
> +/*
> + * Driver for Cadence QSPI Controller
> + *
> + * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This progra

[PATCH] ARM: dts: DRA7: fix a typo in ethernet

[Vishal Mahaveer]

Register address in name of the node is wrong

Signed-off-by: Vishal Mahaveer 
Acked-by: Mugunthan V N 
---
 arch/arm/boot/dts/dra7.dtsi |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1e29ccf..9a20d46 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1398,7 +1398,7 @@
ti,irqs-safe-map = <0>;
};
 
-   mac: ethernet@4a10 {
+   mac: ethernet@48484000 {
compatible = "ti,cpsw";
ti,hwmods = "gmac";
clocks = <&dpll_gmac_ck>, <&gmac_gmii_ref_clk_div>;
-- 
1.7.4.1

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

Re: [RESEND PATCH v16 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs

[Doug Anderson]

Kevin,

On Tue, Aug 25, 2015 at 2:07 PM, Kevin Hilman  wrote:
> Caesar Wang  writes:
>
>> We can add more domains node in the future.
>> This patch add the needed clocks into power-controller.
>> As the discuess about all the device clocks being listed in
>> the power-domains itself.
>>
>> There are several reasons as follows:
>>
>> Firstly, the clocks need be turned off to save power when
>> the system enter the suspend state. So we need to enumerate
>> the clocks in the dts. In order to power domain can turn on and off.
>
> Yes, but this is the job of device drivers which are runtime PM adapted
> to gate their own clocks.  I agree these clocks need to be enumerated in
> the DTS, but they should be in the device nodes.

I _think_ what Caesar means is that the alternative to this patch is
to leave the clocks on all the time as they were during the early days
of Rockchip (AKA last year).  If the clocks are on all the time then
the power domain patches can work fine.  However, once you start
letting clocks turn off then you need to make sure that the power
domain code turns the back on temporarily.


>> Secondly, the reset-circuit should reset be synchronous on RK3288,
>> then sync revoked. So we need to enable clocks of all devices.
>> In other words, we have to enable the clocks before you operate them
>> if all the device clocks are included in someone domians.
>
> Yes, this is pretty common for reset.
>
>> Someone wish was to get the clocks by reading the clocks from the
>> device nodes, We can do that but we can solve the above issues.
>
> I don't follow this sentence.  Are you saying doing that will not solve
> the above issues?  Why not?  Please explain.
>
> If there are non-device clocks that also need to be enabled before
> asserting reset, then those are candidates for the power-domain node,
> but not device clocks.

It's been a long time and I don't know that I've reviewed every
revision of this series, but I think there was a proposal that we
shouldn't list clocks here.  Instead we should search through and find
all devices that refer to this power domain, reach in and find their
clocks, and turn them on.  Did I get that right?  To put things in a
concrete way, for pd_vio we'd go through the entire device tree
ourselves and find all properties that look like "power-domains =
<&power RK3288_PD_VIO>;".  We'd then find the parent of those
properties and look for a property named "clocks".  We'd then iterate
over all those clocks and turn those on.  Did I get that right?

The above doesn't seem like a terribly great idea to me for a number
of reasons, including:

1. If I remember correctly, it's important to turn on clocks for
devices even if they're not something you're using / have a driver
for.  If you don't then the device won't get reset properly and this
can affect things like suspend/resume because the hardware in the SoC
will query all devices at suspend time to make sure they're ready.  If
a device is wedged because its clock wasn't on at the right them then
it will cause problems.

2. If we absolutely need to turn all clocks and we get clocks from
device tree nodes on then it means we need device tree nodes for every
device in the domain.  These would be needed even if there are no
accepted bindings for this device yet.  So we'd need to do one of: A)
Block power domain patches on feature complete bindings for all
drivers; B) Make up non-approved compatible strings for all devices
and throw them into the DTS; C) Add nodes in the DTS without a
compatible string just to satisfy the power domain requirements.  None
of these seem terribly appealing.

3. It is entirely possible that there are clocks that will be listed
in the individual devices that aren't needed for powering on the power
domain.  I'd tend to believe that PCLK_EDP_CTRL (the pixel clock)
doesn't really need to be turned on when adjusting the "VIO" power
domain.  Right now Caesar has it listed, but it probably isn't needed
(Caesar: can you confirm?).

4. It seems just slightly brittle to be reaching into other device
nodes and making assumptions about their properties.  Yeah, it's
probably safe to assume that "clocks" has a list of clocks and
"power-domains" will point to something whose first entry is a
phandle, but it still seems just a tad bit like violating an
abstraction barrier.


Anyway, perhaps I'm misunderstanding, or perhaps my concerns are
simply not for important things.  If so feel free to yell at me.  ;)


>> Anyway, the best ideas we can fix it in the future SoCs.
>
> I don't think this is an SoC design issue as this is needed when you
> have synchronous reset.  My concern is primarily around how to describe
> this in the DT.

I suppose the SoC could override things and make sure clocks are on in
this case?  ...or if a clock is off it could defer powering it up
somehow until the clock came on?  ...dunno how this actually looks in
hardware.  In any case letting devices get wedged doesn't seem
ideal...

-Doug
--
To uns

Re: [PATCH v2 5/6] ARM: dts: qs600: add alias to serial0

[Andy Gross]

On Tue, Aug 18, 2015 at 02:10:27PM +0100, Srinivas Kandagatla wrote:
> This patch add alias node with serial0.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts | 4 
>  1 file changed, 4 insertions(+)

Looks fine.  Applied.

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

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

Re: [PATCH v2 5/6] ARM: dts: qs600: add alias to serial0

[Stephen Boyd]

On 08/18/2015 06:10 AM, Srinivas Kandagatla wrote:

This patch add alias node with serial0.

Signed-off-by: Srinivas Kandagatla 
---


I have a whole set of patches to add all the serial aliases and 
stdout-path properties [1]. Please drop this patch from your series.


[1] 
http://lkml.kernel.org/r/1434490313-13905-1-git-send-email-sb...@codeaurora.org


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

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

Re: [PATCH v2 3/6] ARM: dts: apq8064-ifc6410: add notify led support.

[Stephen Boyd]

On 08/18/2015 06:10 AM, Srinivas Kandagatla wrote:

diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts 
b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index b1f9ddb..08daafe 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -221,6 +221,18 @@
status = "okay";
};
  
+		leds {

+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <¬ify_led>;
+
+   led@1 {
+   label = "apq8064:green:user1";
+   gpios = <&pm8921_gpio 18 GPIO_ACTIVE_HIGH>;
+   default-state = "on";
+   };
+   };


Wrong place. Should be in root, not soc node.

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

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

Re: [PATCH v2 1/6] ARM: dts: ifc6410: Add pwrseq support for WLAN

[Stephen Boyd]

On 08/18/2015 06:06 AM, Srinivas Kandagatla wrote:

@@ -10,6 +11,20 @@
serial1 = &gsbi6_serial;
};
  
+	pwrseq {

+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;


Why do we need any of these three properties?


+   compatible = "simple-bus";
+
+   sdcc4_pwrseq: sdcc4_pwrseq {
+   pinctrl-names = "default";
+   pinctrl-0 = <&wlan_default_gpios>;
+   compatible = "mmc-pwrseq-simple";
+   reset-gpios = <&pm8921_gpio 43 GPIO_ACTIVE_LOW>;


Especially because this node doesn't have a reg property.

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

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

Re: [PATCH v2 4/6] ARM: dts: qs600: remove unnecessary i2c pinctrl nodes in board file.

[Andy Gross]

On Tue, Aug 18, 2015 at 02:10:19PM +0100, Srinivas Kandagatla wrote:
> This patch removes unnecessary i2c pinctrl nodes in board file, these
> are already defined in the soc specific file qcom-apq8064.dtsi.
> 
> Signed-off-by: Srinivas Kandagatla 
> ---
>  arch/arm/boot/dts/qcom-apq8064-cm-qs600.dts | 9 -
>  1 file changed, 9 deletions(-)

Applied, thanks!

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

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

Re: [PATCH v3] qcom: ipq40xx: Add basic board/dts support for IPQ40XX SoC

[Stephen Boyd]

On 08/25, Varadarajan Narayanan wrote:
> On Mon, Aug 24, 2015 at 03:49:28PM -0700, Stephen Boyd wrote:
> > On 08/24, Varadarajan Narayanan wrote:
> 
> > > + compatible = "qcom,ipq40xx-r3pc", "qcom,ipq40xx";
> > > +
> > > + memory {
> > > + device_type = "memory";
> > > + reg = <0x8000 0x2000>; /* 512MB */
> > > + };
> >
> > Doesn't the bootloader fill the memory node for us? Why do we
> > need this?
> >
> > > +
> > > + chosen {
> > > + bootargs = "root=/dev/ram rw init=/init 
> > > console=ttyMSM0,115200n8 initrd=0x8200,0x000E2246";
> > > + };
> >
> > Please don't add bootargs. Use stdout-path for the console part
> > and everything else should be done by the bootloader or is the
> > defaults.
> 
> Since this is for the emulation platform, we don't have the bootloader.

Why are we upstreaming an emulation platform?

I assume you have some sort of scripts or something to load the
kernel image into the emulator, so those scripts could go and
modify the dtb in place to populate the initrd and commandline.
If you need pointers on that feel free to contact me directly.

> 
> > > + compatible = "arm,armv7-timer";
> > > + interrupts = <1 2 0xf08>,
> > > +  <1 3 0xf08>,
> > > +  <1 4 0xf08>,
> > > +  <1 1 0xf08>;
> > > + clock-frequency = <2083>;
> >
> > Drop this clock-frequency part if you can. The hardware should
> > properly report the frequency.
> 
> Cannot drop this. This is the ref-clock frequency. For the
> chip it is 48MHz, for emulation is ~20MHz. Similar to
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/qcom-msm8974.dtsi#n89

And the hardware is not emulated properly to report the actual
frequency that the timer is running at? That's sad.

At the least, this should be in the board specific dtsi file
instead of the SoC specific file, because it's a board quirk.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] arm64: dts: qcom: Add RNG device tree node

[Andy Gross]

On Tue, Aug 25, 2015 at 06:37:42PM +0300, Stanimir Varbanov wrote:
> Adds rng device tree node for msm8916 SoCs.
> 
> Signed-off-by: Stanimir Varbanov 
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi |7 +++
>  1 file changed, 7 insertions(+)

Looks good to me.  Applied.

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

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

Re: [RESEND PATCH v16 4/4] ARM: dts: add the support power-domain node on RK3288 SoCs

[Kevin Hilman]

Caesar Wang  writes:

> We can add more domains node in the future.
> This patch add the needed clocks into power-controller.
> As the discuess about all the device clocks being listed in
> the power-domains itself.
>
> There are several reasons as follows:
>
> Firstly, the clocks need be turned off to save power when
> the system enter the suspend state. So we need to enumerate
> the clocks in the dts. In order to power domain can turn on and off.

Yes, but this is the job of device drivers which are runtime PM adapted
to gate their own clocks.  I agree these clocks need to be enumerated in
the DTS, but they should be in the device nodes.

> Secondly, the reset-circuit should reset be synchronous on RK3288,
> then sync revoked. So we need to enable clocks of all devices.
> In other words, we have to enable the clocks before you operate them
> if all the device clocks are included in someone domians.

Yes, this is pretty common for reset.  

> Someone wish was to get the clocks by reading the clocks from the
> device nodes, We can do that but we can solve the above issues.

I don't follow this sentence.  Are you saying doing that will not solve
the above issues?  Why not?  Please explain.

If there are non-device clocks that also need to be enabled before
asserting reset, then those are candidates for the power-domain node,
but not device clocks.

> Anyway, the best ideas we can fix it in the future SoCs.

I don't think this is an SoC design issue as this is needed when you
have synchronous reset.  My concern is primarily around how to describe
this in the DT.

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

Re: [RESEND PATCH v16 3/4] soc: rockchip: power-domain: Add power domain driver

[Kevin Hilman]

Caesar Wang  writes:

> This driver is found on RK3288 SoCs.
>
> In order to meet high performance and low power requirements, a power
> management unit is designed or saving power when RK3288 in low power
> mode.
> The RK3288 PMU is dedicated for managing the power of the whole chip.
>
> PMU can work in the Low Power Mode by setting bit[0] of PMU_PWRMODE_CON
> register. After setting the register, PMU would enter the Low Power mode.
> In the low power mode, pmu will auto power on/off the specified power
> domain, send idle req to specified power domain, shut down/up pll and
> so on. All of above are configurable by setting corresponding registers.
>
> Signed-off-by: jinkun.hong 
> Signed-off-by: Caesar Wang 

[...]

> diff --git a/drivers/soc/rockchip/Kconfig b/drivers/soc/rockchip/Kconfig
> new file mode 100644
> index 000..87a09c3
> --- /dev/null
> +++ b/drivers/soc/rockchip/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# Rockchip Soc drivers
> +#
> +config PM_GENERIC_DOMAINS

Why are you (re)defining this config option?  This name is already used in 
kernel/pm/Kconfig.

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

Re: [PATCH v10 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support

[Boris Brezillon]

Brian, Stefan,

On Tue, 25 Aug 2015 12:54:11 -0700
Brian Norris  wrote:

> On Mon, Aug 03, 2015 at 11:28:43AM +0200, Stefan Agner wrote:
> > On 2015-08-03 11:27, Stefan Agner wrote:
> > 
> > > +static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t 
> > > *dat,
> > > +  uint8_t *oob, int oob_loaded)
> > > +{
> > > + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> > > + u8 ecc_status;
> > > + u8 ecc_count;
> > > + int flip;
> > > +
> > > + ecc_status = __raw_readb(nfc->regs + ECC_SRAM_ADDR * 8 + ECC_OFFSET);
> 
> Why __raw_readb()? That's not normally encourage, and it has issues with
> endianness. It looks like maybe this is actulaly a 32-bit register, and
> you're having trouble when trying to do bytewise access? I see this
> earlier:
> 
> /*
>  * ECC status is stored at NFC_CFG[ECCADD] +4 for little-endian
>  * and +7 for big-endian SoCs.
>  */
> #ifdef __LITTLE_ENDIAN
> #define ECC_OFFSET  4
> #else
> #define ECC_OFFSET  7
> #endif
> 
> So maybe you really just want:
> 
> #define ECC_OFFSET4
> ...
>   ecc_status = vf610_nfc_read(ECC_SRAM_ADDR * 8 + ECC_OFFSET) & 0xff;
> 
> ?
> 
> > > + ecc_count = ecc_status & ECC_ERR_COUNT;
> > > +
> > > + if (!(ecc_status & ECC_STATUS_MASK))
> > > + return ecc_count;
> > > +
> > > + if (!oob_loaded)
> > > + vf610_nfc_read_buf(mtd, oob, mtd->oobsize);
> > > +
> > > + /*
> > > +  * On an erased page, bit count (including OOB) should be zero or
> > > +  * at least less then half of the ECC strength.
> > > +  */
> > > + flip = count_written_bits(dat, nfc->chip.ecc.size, ecc_count);
> 
> Another side note: why are you using ecc_count as a max threshold? AIUI,
> an ECC algorithm doesn't really report useful error count information if
> it's above the correction limit. So wouldn't we be looking to count up
> to our SW threshold? i.e., ecc.strength / 2, or similar? Similar
> comments below.

The exact threshold value is still something I'm not sure about, though
I'm sure it should be correlated to ecc.strength value (whether it's
directly set to ecc.strength or less than ecc.strength is something
we'll have to figure out).

> 
> > > + flip += count_written_bits(oob, mtd->oobsize - nfc->chip.ecc.bytes,
> > > +ecc_count);
> > 
> > With ECC the controller seems to clear the ECC bytes in SRAM buffer.
> > This is a dump of 64 Bit OOB with the 32-error ECC mode which requires
> > 60 bytes of OOB for ECC:
> > 
> > [   22.190273] ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 
> Hmm, that's not really good. The point is that we need to make sure that
> everything that could have been programmed (including the ECC area) was
> not actually programmed. But your ECC controller is not, contrary to
> MTD's expectations, dumping raw uncorrected data here.

Yep, for this test we really need the ECC bytes generated for the chunk
you're currently testing.
How to retrieve those bytes really depends on your NAND controller, but
such controllers usually provides a way to disable the ECC engine. The
only thing you'll have to do in this case is disable the ECC engine and
read the OOB data (using RNDOUT and read_buf for example).

> 
> > [   22.209698] vf610_nfc_correct_data, flips 1
> > 
> > Not sure if this is acceptable, but I now only count the bits in the
> > non-ECC area of the OOB.
> 
> That's not the intention of my suggestion. You're still missing out on a
> class of patterns that might look close to all 0xff but are not
> actually.

Exactly.

> 
> If the HW ECC really doesn't give you valid data+OOB at this point, then
> you might have to re-read with ECC disabled. Of course, that's got a
> performance cost...

As suggested above, if that's possible, reading the OOB area (or a
portion of the OOB area) with the ECC engine disabled should be enough.

> 
> Or perhaps Boris has a better suggestion? He's been surveying other NAND
> drivers that need to do similar things, and he's working on providing
> some support code for common design patterns.

Yep, the patch series is here in case you want to have a look [1].

Best Regards,

Boris

[1]https://patchwork.ozlabs.org/patch/509970/


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] drm/vc4: Add devicetree bindings for VC4.

[Rob Clark]

On Mon, Aug 24, 2015 at 9:47 AM, Rob Herring  wrote:
> On Mon, Aug 17, 2015 at 1:30 PM, Eric Anholt  wrote:
>> Stephen Warren  writes:
>>
>>> On 08/12/2015 06:56 PM, Eric Anholt wrote:
 Signed-off-by: Eric Anholt 
>>>
>>> This one definitely needs a patch description, since someone might not
>>> know what a VC4 is, and "git log" won't show the text from the binding
>>> doc itself. I'd suggest adding the initial paragraph of the binding doc
>>> as the patch description, or more.
>>>
 diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt 
 b/Documentation/devicetree/bindings/gpu/brcm,bcm-vc4.txt
>>
 +- hvss: List of references to HVS video scalers
 +- encoders: List of references to output encoders (HDMI, SDTV)
>>>
>>> Would it make sense to make all those nodes child node of the vc4
>>> object. That way, there's no need to have these lists of objects; they
>>> can be automatically built up as the DT is enumerated. I know that e.g.
>>> the NVIDIA Tegra host1x binding works this way, and I think it may have
>>> been inspired by other similar cases.
>>
>> I've looked at tegra, and the component system used by msm appears to be
>> nicer than it.  To follow tegra's model, it looks like I need to build
>> this extra bus thing corresponding to host1x that is effectively the
>> drivers/base/component.c code, so that I can get at vc4's structure from
>> the component drivers.
>>
>>> Of course, this is only appropriate if the HW modules really are
>>> logically children of the VC4 HW module. Perhaps they aren't. If they
>>> aren't though, I wonder what this "vc4" module actually represents in HW?
>>
>> It's the subsystem, same as we use a subsystem node for msm, sti,
>> rockchip, imx, and exynos.  This appears to be the common model of how
>> the collection of graphics-related components is represented in the DT.
>
> I think most of these bindings are wrong. They are grouped together
> because that is what DRM wants not because that reflects the h/w. So
> convince me this is one block, not that it is what other people do.

I think, when it comes to more complex driver subsystems (like drm in
particular) we have a bit of mismatch between how things look from the
"pure hw ignoring sw" perspective, and the "how sw and in particular
userspace expects things" perspective.  Maybe it is less a problem in
other subsystems, where bindings map to things that are only visible
in the kernel, or well defined devices like uart or sata controller.
But when given the choice, I'm going to err on the side of not
confusing userspace and the large software stack that sits above
drm/kms, over dt purity.

Maybe it would be nice to have a sort of dt overlay that adds the bits
needed to tie together hw blocks that should be assembled into a
single logical device for linux and userspace (but maybe not some
other hypothetical operating system).  But so far that doesn't exist.
All we have is a hammer (devicetree), everything looks like a nail.
End result is we end up adding some things in the bindings which
aren't purely about the hw.  Until someone invents a screwdriver, I'm
not sure what else to do.  In the end, other hypothetical OS is free
to ignore those extra fields in the bindings if it doesn't need them.
So meh?

BR,
-R


> Rob
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Brian Norris]

One more thing...

On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote:
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,645 @@
...
> +struct vf610_nfc {
> + struct mtd_info mtd;
> + struct nand_chip chip;
> + struct device *dev;
> + void __iomem *regs;
> + struct completion cmd_done;
> + uint buf_offset;
> + int page_sz;

AFAICT (even with the 2nd patch), you never really use this field. You
just set it/increment it, but don't use it for anything. Kill it?

> + /* Status and ID are in alternate locations. */
> + int alt_buf;
> +#define ALT_BUF_ID   1
> +#define ALT_BUF_STAT 2
> +#define ALT_BUF_ONFI 3
> + struct clk *clk;
> +};

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

Re: [PATCH v10 3/5] mtd: nand: vf610_nfc: add device tree bindings

[Brian Norris]

Sorry, I realized a potential issue here.

On Mon, Aug 03, 2015 at 11:27:28AM +0200, Stefan Agner wrote:
> Signed-off-by: Bill Pringlemeir 
> Acked-by: Shawn Guo 
> Reviewed-by: Brian Norris 
> Signed-off-by: Stefan Agner 
> ---
>  .../devicetree/bindings/mtd/vf610-nfc.txt  | 45 
> ++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/vf610-nfc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/vf610-nfc.txt 
> b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
> new file mode 100644
> index 000..cae5f25
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/vf610-nfc.txt
> @@ -0,0 +1,45 @@
> +Freescale's NAND flash controller (NFC)
> +
> +This variant of the Freescale NAND flash controller (NFC) can be found on
> +Vybrid (vf610), MPC5125, MCF54418 and Kinetis K70.
> +
> +Required properties:
> +- compatible: Should be set to "fsl,vf610-nfc"
> +- reg: address range of the NFC
> +- interrupts: interrupt of the NFC
> +- nand-bus-width: see nand.txt
> +- nand-ecc-mode: see nand.txt
> +- nand-on-flash-bbt: see nand.txt

Stumbling across the "multi-CS" questions on the driver reminds me: it
typically makes sense to define new NAND bindings using separate NAND
*controller* and *flash* device nodes. The above 3 properties, at least,
would apply on a per-flash basis, not per-controller typically. See
sunxi-nand, for instance:

http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt

brcmnand had a similar pattern:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt

(Perhaps it's time we standardized this a little more formally...)

> +- assigned-clocks: main clock from the SoC, for Vybrid <&clks VF610_CLK_NFC>;
> +- assigned-clock-rates: The NAND bus timing is derived from this clock
> +rate and should not exceed maximum timing for any NAND memory chip
> +in a board stuffing. Typical NAND memory timings derived from this
> +clock are found in the SoC hardware reference manual. Furthermore,
> +there might be restrictions on maximum rates when using hardware ECC.
> +
> +- #address-cells, #size-cells : Must be present if the device has sub-nodes
> +  representing partitions.
> +
> +Required properties for hardware ECC:
> +- nand-ecc-strength: supported strengths are 24 and 32 bit (see nand.txt)
> +- nand-ecc-step-size: step size equals page size, currently only 2k pages are
> +supported
> +
> +Example:
> +
> + nfc: nand@400e {
> + compatible = "fsl,vf610-nfc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x400e 0x4000>;
> + interrupts = ;
> + clocks = <&clks VF610_CLK_NFC>;
> + clock-names = "nfc";
> + assigned-clocks = <&clks VF610_CLK_NFC>;
> + assigned-clock-rates = <3300>;
> + nand-bus-width = <8>;
> + nand-ecc-mode = "hw";
> + nand-ecc-strength = <32>;
> + nand-ecc-step-size = <2048>;
> + nand-on-flash-bbt;
> + };

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

Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

[Brian Norris]

A few more comments.

On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote:
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 000..5c8dfe8
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,645 @@

...

> +/*
> + * This function supports Vybrid only (MPC5125 would have full RB and four 
> CS)
> + */
> +static void vf610_nfc_select_chip(struct mtd_info *mtd, int chip)
> +{
> +#ifdef CONFIG_SOC_VF610

Why the #ifdef? I don't see anything compile-time specific to SOC_VF610.

If this is trying to handle the comment above ("This function supports
Vybrid only (MPC5125 would have full RB and four CS)") then that's the
wrong way of doing it, as you need to support multiplatform kernels.
You'll need to have a way to differentiate the different platform
support at runtime, not compile time.

> + struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> + u32 tmp = vf610_nfc_read(nfc, NFC_ROW_ADDR);
> +
> + tmp &= ~(ROW_ADDR_CHIP_SEL_RB_MASK | ROW_ADDR_CHIP_SEL_MASK);
> + tmp |= 1 << ROW_ADDR_CHIP_SEL_RB_SHIFT;
> +
> + if (chip == 0)
> + tmp |= 1 << ROW_ADDR_CHIP_SEL_SHIFT;
> + else if (chip == 1)
> + tmp |= 2 << ROW_ADDR_CHIP_SEL_SHIFT;

else ... ?

Maybe you can write this as a formulaic pattern (e.g.:

tmp |= (chip + 1) << ROW_ADDR_CHIP_SEL_SHIFT;

) and just do the "max # of chips" checks on a per-platform basis in the
probe(). Then I'm guessing this same function can apply to both
platforms. (I'm not looking at HW datasheets for this, BTW, just
guessing based on the context here.)

But wait...I see that you call nand_scan_ident() with a max of 1 chip.
So you won't ever see the chip > 0 case, right?

So does this driver support multiple flash attached or not? Looks like
you're assuming you'll only be using chip-select 0. (This is fine for
now, but at least your code should acknowledge this. Perhaps a comment
at the top under "limitations.")

> +
> + vf610_nfc_write(nfc, NFC_ROW_ADDR, tmp);
> +#endif
> +}

...

> +static int vf610_nfc_probe(struct platform_device *pdev)
> +{

...

> + /* first scan to find the device and get the page size */
> + if (nand_scan_ident(mtd, 1, NULL)) {
> + err = -ENXIO;
> + goto error;
> + }

...

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

Re: [PATCH v10 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support

[Brian Norris]

On Mon, Aug 03, 2015 at 11:28:43AM +0200, Stefan Agner wrote:
> On 2015-08-03 11:27, Stefan Agner wrote:
> 
> > +static inline int vf610_nfc_correct_data(struct mtd_info *mtd, uint8_t 
> > *dat,
> > +uint8_t *oob, int oob_loaded)
> > +{
> > +   struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> > +   u8 ecc_status;
> > +   u8 ecc_count;
> > +   int flip;
> > +
> > +   ecc_status = __raw_readb(nfc->regs + ECC_SRAM_ADDR * 8 + ECC_OFFSET);

Why __raw_readb()? That's not normally encourage, and it has issues with
endianness. It looks like maybe this is actulaly a 32-bit register, and
you're having trouble when trying to do bytewise access? I see this
earlier:

/*
 * ECC status is stored at NFC_CFG[ECCADD] +4 for little-endian
 * and +7 for big-endian SoCs.
 */
#ifdef __LITTLE_ENDIAN
#define ECC_OFFSET  4
#else
#define ECC_OFFSET  7
#endif

So maybe you really just want:

#define ECC_OFFSET  4
...
ecc_status = vf610_nfc_read(ECC_SRAM_ADDR * 8 + ECC_OFFSET) & 0xff;

?

> > +   ecc_count = ecc_status & ECC_ERR_COUNT;
> > +
> > +   if (!(ecc_status & ECC_STATUS_MASK))
> > +   return ecc_count;
> > +
> > +   if (!oob_loaded)
> > +   vf610_nfc_read_buf(mtd, oob, mtd->oobsize);
> > +
> > +   /*
> > +* On an erased page, bit count (including OOB) should be zero or
> > +* at least less then half of the ECC strength.
> > +*/
> > +   flip = count_written_bits(dat, nfc->chip.ecc.size, ecc_count);

Another side note: why are you using ecc_count as a max threshold? AIUI,
an ECC algorithm doesn't really report useful error count information if
it's above the correction limit. So wouldn't we be looking to count up
to our SW threshold? i.e., ecc.strength / 2, or similar? Similar
comments below.

> > +   flip += count_written_bits(oob, mtd->oobsize - nfc->chip.ecc.bytes,
> > +  ecc_count);
> 
> With ECC the controller seems to clear the ECC bytes in SRAM buffer.
> This is a dump of 64 Bit OOB with the 32-error ECC mode which requires
> 60 bytes of OOB for ECC:
> 
> [   22.190273] ff ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Hmm, that's not really good. The point is that we need to make sure that
everything that could have been programmed (including the ECC area) was
not actually programmed. But your ECC controller is not, contrary to
MTD's expectations, dumping raw uncorrected data here.

> [   22.209698] vf610_nfc_correct_data, flips 1
> 
> Not sure if this is acceptable, but I now only count the bits in the
> non-ECC area of the OOB.

That's not the intention of my suggestion. You're still missing out on a
class of patterns that might look close to all 0xff but are not
actually.

If the HW ECC really doesn't give you valid data+OOB at this point, then
you might have to re-read with ECC disabled. Of course, that's got a
performance cost...

Or perhaps Boris has a better suggestion? He's been surveying other NAND
drivers that need to do similar things, and he's working on providing
some support code for common design patterns.

> Btw, if the ECC check fails, the controller seems kind of count the
> amount of bitflips. It works for most devices reliable, but we had
> devices for which that number was not accurate, see:
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/357439

I'm a little confused there. Why would you be expecting to get a count
of bitflips, when the ECC engine can't correct all errors? How is it
supposed to know what the "right" data is if the bit errors are beyond
the correction strength?

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

Re: [PATCH 1/2] watchdog: bcm7038: add device tree binding documentation

[Justin Chen]

On Tue, Aug 25, 2015 at 12:04:48PM -0700, Guenter Roeck wrote:
> Justin,
> 
> On Tue, Aug 25, 2015 at 10:55:40AM -0700, Justin Chen wrote:
> > 
> > Hello Guenter,
> > 
> > > Is 'clock-frequency' really needed (and useful), or would it make more 
> > > sense
> > > to expect the user to configure a fixed clock if nothing else is 
> > > available ?
> > > How do other drivers handle this ?
> > 
> > The reason for 'clock-frequency' was for a case where our device tree did 
> > not
> > have clocks. Creating a new fixed clock for a single device seems 
> > unnecessary
> > compared to a 'clock-frequency' property. Their is a use for 
> > 'clock-frequency',
> > but it is not really necessary. However, this is my first linux patch, so I 
> > do 
> > not fully trust my judgement on this...
> > 
> 
> All that is needed for a fixed clock is a devicetree entry for it. Not sure
> I understand your line of argument; you add a lot of complexity and code
> just to avoid those few lines in the dts file (especially with 500+
> "fixed-clock" nodes in other devicetree files).
> 
> Thanks,
> Guenter

Ok your argument makes sense. I will remove the clock-frequency property and 
modify the driver accordingly. Thank you for your feedback!

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

[PATCH] of: device: fix NULL pointer dereference on driver removal

[Felipe Balbi]

If we don't insert resources into the resource tree,
calls to of_platform_depopulate() will end up in NULL
pointer dereferences because the resource parent will
be set to NULL even though we still have more resources
to go through.

Without this patch, the result is as follows:

[   28.238158] Unable to handle kernel NULL pointer dereference at virtual 
address 0008
[   28.246646] pgd = ed3d8000
[   28.249480] [0008] *pgd=
[   28.253247] Internal error: Oops: 5 [#1] SMP ARM
[   28.258072] Modules linked in: input_leds hid_generic usbkbd usbhid
xhci_plat_hcd xhci_hcd usbcore joydev dwc3 udc_core usb_common m25p80 evdev 
spi_nor omapfb cfbfillrect cfbimg blt cpufreq_dt cfbcopyarea thermal_sys 
snd_soc_simple_card leds_gpio matrix_keypad pwm_bl hwmon led_class 
matrix_keymap panel_dpi snd_soc_tlv320aic3x snd_soc_davinci_mcasp snd_soc_edma 
snd_soc_omap snd_soc_core omapdss snd_compress snd_pcm_dmaengine snd_pcm 
dwc3_omap(-) extcon snd_timer pwm_tiecap snd lis3lv02d_i2c lis3lv02d soundcore 
input_polldev rtc_omap spi_ti_qspi tps65218_pwrbutton omap_wdt phy_omap_usb2 
autofs4
[   28.313186] CPU: 0 PID: 289 Comm: modprobe Not tainted 
4.2.0-rc7-next-20150824-2-g5681a109a938-dirty #1093
[   28.323643] Hardware name: Generic AM43 (Flattened Device Tree)
[   28.329836] task: ed39d180 ti: ed076000 task.ti: ed076000
[   28.335496] PC is at __release_resource+0x30/0x13c
[   28.340501] LR is at __release_resource+0x24/0x13c
[   28.345501] pc : []lr : []psr: 600d0013
[   28.345501] sp : ed077e98  ip : 0007  fp : befb3e14
[   28.357487] r10:   r9 : ed076000  r8 : c000f724
[   28.362941] r7 :   r6 : ee6f9800  r5 : ed268d40  r4 : c094679c
[   28.369755] r3 :   r2 : 00f4  r1 : c0648b34  r0 : 0045
[   28.376560] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM Segment user
[   28.384008] Control: 10c5387d  Table: ad3d8059  DAC: 0015
[   28.390005] Process modprobe (pid: 289, stack limit = 0xed076218)
[   28.396375] Stack: (0xed077e98 to 0xed078000)
[   28.400924] 7e80: ed268d40 
[   28.409455] 7ea0: befb3e14 c0640838 0001 c094679c ed268d40 ee6f9800 
0081 c0043b1c
[   28.417996] 7ec0: 001c 0001 ee6f9800 c03e2674 ee6f9800  
c04d3f20 c03e26ac
[   28.426537] 7ee0: ee6f9810 c04d3fac  c03dcf10 ee1592c0 ed268cf0 
ee170010 ee170010
[   28.435065] 7f00: ee170044 c04d3f08 ed268ed0 bf093208 ee170010 bf093c94 
ee170044 c03e2754
[   28.443607] 7f20: ee170010 bf093c94 ee170044 c03e095c ee170010 bf093c94 
ee170044 c03e116c
[   28.452150] 7f40: bf093c94 7f6232fc 0800 c03e04e4 bf093d00 c00c8a80 
 33637764
[   28.460703] 7f60: 616d6f5f b6f70070 ed39d180  ed076000  
befb3e14 c005be74
[   28.469241] 7f80:  ed076000 7f6232c0 7f6232c0 0001 f67c 
7f6232c0 7f6232c0
[   28.477783] 7fa0: 0001 c000f540 7f6232c0 7f6232c0 7f6232fc 0800 
66d19c00 
[   28.486341] 7fc0: 7f6232c0 7f6232c0 0001 0081  0001 
7f6232c0 befb3e14
[   28.494903] 7fe0: b6f1c2e1 befb2a3c 7f60638f b6f1c2e6 800d0030 7f6232fc 
 
[   28.503476] [] (__release_resource) from [] 
(release_resource+0x30/0x54)
[   28.512299] [] (release_resource) from [] 
(platform_device_del+0x70/0x9c)
[   28.521218] [] (platform_device_del) from [] 
(platform_device_unregister+0xc/0x20)
[   28.530962] [] (platform_device_unregister) from [] 
(of_platform_device_destroy+0x8c/0x98)
[   28.541425] [] (of_platform_device_destroy) from [] 
(device_for_each_child+0x50/0x7c)
[   28.551430] [] (device_for_each_child) from [] 
(of_platform_depopulate+0x2c/0x44)
[   28.561101] [] (of_platform_depopulate) from [] 
(dwc3_omap_remove+0x3c/0x5c [dwc3_omap])
[   28.571390] [] (dwc3_omap_remove [dwc3_omap]) from [] 
(platform_drv_remove+0x18/0x30)
[   28.581387] [] (platform_drv_remove) from [] 
(__device_release_driver+0x88/0x114)
[   28.591023] [] (__device_release_driver) from [] 
(driver_detach+0xb4/0xb8)
[   28.600014] [] (driver_detach) from [] 
(bus_remove_driver+0x4c/0xa0)
[   28.608485] [] (bus_remove_driver) from [] 
(SyS_delete_module+0x11c/0x1e4)
[   28.617518] [] (SyS_delete_module) from [] 
(ret_fast_syscall+0x0/0x54)
[   28.626172] Code: eb0354bf e5957010 e3a020f4 e59f10f0 (e5973008)
[   28.632722] ---[ end trace d2a21fc5d73a2dfd ]---

Cc: 
Signed-off-by: Felipe Balbi 
---

very easy to trigger with 'modprobe -r dwc3-omap' on any of TI's platform
sporting dwc3

 drivers/of/device.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 8b91ea241b10..c03600c08207 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -53,6 +53,8 @@ EXPORT_SYMBOL(of_dev_put);
 
 int of_device_add(struct platform_device *ofdev)
 {
+   int i;
+
BUG_ON(ofdev->dev.of_node == NULL);
 
/* name and id have to be set so that the platform bus doesn't get
@@ -66,6 +68,27 @@ int of_device_add(struct platform_device *ofdev)
if (!of

Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c

[Thomas Gleixner]

On Wed, 26 Aug 2015, Fu Wei wrote:
> >>  /* Initialize per-processor generic timer */
> >> -static int __init arch_timer_acpi_init(struct acpi_table_header *table)
> >> +void __init arch_timer_acpi_init(void)
> >>  {
> >
> > And how is that supposed to work when we have next generation CPUs
> > which implement a different timer? You break multisystem kernels that
> > way.
> 
> yes, you are right, If there is a  next generation CPUs  which
> implement a different timer, (maybe) this driver can not work.
> we may need a new timer driver.
> 
> But,
> (1) for now,  aarch64  core always has the arch timer(this timer is
> part of aarch64 architecture).
> and the existing code make  ARM64 kernel "select ARM_ARCH_TIMER "
> (2) GTDT is designed for generic timer, so in this call "
> arch_timer_acpi_init"  we  parse the gtdt info.
> (3) once we have a ARM64 CPUs which implement a different timer, we
> may need to select a right timer in the config stage.
> and this timer may not be described in GTDT.  So we can implement
> another arch_timer_acpi_init by that time in new timer driver..
> if the new time still uses GTDT(or new version GTDT), we may need to
> update gtdt.c for new timer by that time.

That's simply wrong. You want to build kernels which run on both cpus
and the selection of the timer happens at runtime depending on the
ACPI info. We do the same thing with device tree.
 
> but before we really have this new timer, I think this code is OK to use.

I don't think so, but I leave this decision to the ARM64 maintainers.

Thanks,

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

Re: [PATCH 1/2] watchdog: bcm7038: add device tree binding documentation

[Guenter Roeck]

Justin,

On Tue, Aug 25, 2015 at 10:55:40AM -0700, Justin Chen wrote:
> 
> Hello Guenter,
> 
> > Is 'clock-frequency' really needed (and useful), or would it make more sense
> > to expect the user to configure a fixed clock if nothing else is available ?
> > How do other drivers handle this ?
> 
> The reason for 'clock-frequency' was for a case where our device tree did not
> have clocks. Creating a new fixed clock for a single device seems unnecessary
> compared to a 'clock-frequency' property. Their is a use for 
> 'clock-frequency',
> but it is not really necessary. However, this is my first linux patch, so I 
> do 
> not fully trust my judgement on this...
> 

All that is needed for a fixed clock is a devicetree entry for it. Not sure
I understand your line of argument; you add a lot of complexity and code
just to avoid those few lines in the dts file (especially with 500+
"fixed-clock" nodes in other devicetree files).

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

Re: [PATCH V1 0/3] fix clock issue for fsl,spdif

[Michael Turquette]

Quoting Shengjiu Wang (2015-08-11 02:26:51)
> fix clock issue for fsl,spdif
> 
> Shengjiu Wang (3):
>   ARM: imx6q: Add SPDIF_GCLK clock in clock tree
>   ARM: imx6sl: Add SPDIF_GCLK clock in clock tree
>   ARM: imx6sx: Add SPDIF_GCLK clock in clock tree

Hello Shengjiu Wang,

Please move the dts changes into separate patches. I can take the
changes to the clock driver and the header through the clk tree, but the
dt patches need to go through arm-soc.

Regards,
Mike

> 
>  arch/arm/boot/dts/imx6qdl.dtsi|  2 +-
>  arch/arm/boot/dts/imx6sl.dtsi | 16 
>  arch/arm/boot/dts/imx6sx.dtsi |  2 +-
>  drivers/clk/imx/clk-imx6q.c   |  4 +++-
>  drivers/clk/imx/clk-imx6sl.c  |  4 +++-
>  drivers/clk/imx/clk-imx6sx.c  |  1 +
>  include/dt-bindings/clock/imx6qdl-clock.h |  3 ++-
>  include/dt-bindings/clock/imx6sl-clock.h  |  3 ++-
>  include/dt-bindings/clock/imx6sx-clock.h  |  3 ++-
>  9 files changed, 31 insertions(+), 7 deletions(-)
> 
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] watchdog: bcm7038: add device tree binding documentation

[Justin Chen]

On Sun, Aug 23, 2015 at 08:32:21PM -0700, Guenter Roeck wrote:
> Hi Justin,
> 
> On 08/20/2015 10:41 AM, Justin Chen wrote:
> >Add device tree binding docmentation for the watchdog hardware block
> >on bcm7038 and newer SoCs.
> >
> >Signed-off-by: Justin Chen 
> >---
> >  .../devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt | 19 
> > +++
> >  1 file changed, 19 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
> >
> >diff --git a/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt 
> >b/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
> >new file mode 100644
> >index 000..adb8260
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/watchdog/brcm,bcm7038-wdt.txt
> >@@ -0,0 +1,19 @@
> >+BCM7038 Watchdog timer
> >+
> >+Required properties:
> >+
> >+- compatible : should be "brcm,bcm7038-wdt"
> >+- reg : Specifies base physical address and size of the registers.
> >+
> >+Optional properties:
> >+
> >+- clocks: the clock running the watchdog
> >+- clock-frequency: the rate of the clock
> 
> Is 'clock-frequency' really needed (and useful), or would it make more sense
> to expect the user to configure a fixed clock if nothing else is available ?
> How do other drivers handle this ?
> 
> Thanks,
> Guenter
> 
> >+
> >+Example:
> >+
> >+watchdog {
> >+compatible = "brcm,bcm7038-wdt";
> >+clocks = <&upg_fixed>;
> >+reg = <0xf040a7e8 0x16>;
> >+};
> >
>

Hello Guenter,

> Is 'clock-frequency' really needed (and useful), or would it make more sense
> to expect the user to configure a fixed clock if nothing else is available ?
> How do other drivers handle this ?

The reason for 'clock-frequency' was for a case where our device tree did not
have clocks. Creating a new fixed clock for a single device seems unnecessary
compared to a 'clock-frequency' property. Their is a use for 'clock-frequency',
but it is not really necessary. However, this is my first linux patch, so I do 
not fully trust my judgement on this...

Looking at other drivers, none of them have both clock and clock-frequency.

Thank you for your time on reviewing the patch Guenter. Much appreciated!

Thanks,
Justin Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/4] spi: mediatek: replace int with u32, delete TAB and define MTK_SPI_PAUSE_INT_STATUS marco

[Mark Brown]

On Mon, Aug 24, 2015 at 11:45:16AM +0800, Leilk Liu wrote:

> this patch replaces int with u32, deletes TAB, and defines
> MTK_SPI_PAUSE_INT_STATUS marco.

You've listed three changes in one patch and they're not obviously
interrelated.  As has been said before one change per patch please :(


signature.asc
Description: Digital signature

Re: [PATCH v7 8/8] clocksource: simplify ACPI code in arm_arch_timer.c

[Fu Wei]

Hi Thomas,


On 25 August 2015 at 01:50, Thomas Gleixner  wrote:
> On Tue, 25 Aug 2015, fu@linaro.org wrote:
>
> You Cc the world and some more on your patch, but you fail to add the
> maintainers of the clocksource code to the Cc list. Sigh.

my fault
So sorry for that, I will add the maintainers of clocksource into cc list.

>
>> From: Fu Wei 
>>
>> The patch update arm_arch_timer driver to use the function
>> provided by the new GTDT driver of ACPI.
>> By this way, arm_arch_timer.c can be simplified, and separate
>> all the ACPI GTDT knowledge from this timer driver.
>
> That's not a proper changelog and this patch want's to be split in two:
>
> 1) Implement the new ACPI function
>
> 2) Make use of it

Yes, you are right , will improve this

>
>> index 0aa135d..99505bb 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -817,68 +817,30 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, 
>> "arm,armv7-timer-mem",
>>  arch_timer_mem_init);
>>
>>  #ifdef CONFIG_ACPI
>> -static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
>> -{
>> - int trigger, polarity;
>> -
>> - if (!interrupt)
>> - return 0;
>> -
>> - trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
>> - : ACPI_LEVEL_SENSITIVE;
>> -
>> - polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
>> - : ACPI_ACTIVE_HIGH;
>> -
>> - return acpi_register_gsi(NULL, interrupt, trigger, polarity);
>> -}
>> -
>>  /* Initialize per-processor generic timer */
>> -static int __init arch_timer_acpi_init(struct acpi_table_header *table)
>> +void __init arch_timer_acpi_init(void)
>>  {
>
> And how is that supposed to work when we have next generation CPUs
> which implement a different timer? You break multisystem kernels that
> way.

yes, you are right, If there is a  next generation CPUs  which
implement a different timer, (maybe) this driver can not work.
we may need a new timer driver.

But,
(1) for now,  aarch64  core always has the arch timer(this timer is
part of aarch64 architecture).
and the existing code make  ARM64 kernel "select ARM_ARCH_TIMER "
(2) GTDT is designed for generic timer, so in this call "
arch_timer_acpi_init"  we  parse the gtdt info.

(3) once we have a ARM64 CPUs which implement a different timer, we
may need to select a right timer in the config stage.
and this timer may not be described in GTDT.  So we can implement
another arch_timer_acpi_init by that time in new timer driver..
if the new time still uses GTDT(or new version GTDT), we may need to
update gtdt.c for new timer by that time.

but before we really have this new timer, I think this code is OK to use.

Do I understand your comment correctly? :-)

>
> Thanks,
>
> tglx



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] dmaengine: st_fdma: Add STMicroelectronics FDMA DT binding documentation

[Peter Griffin]

Hi Vinod,

Thanks for reviewing.

On Wed, 19 Aug 2015, Vinod Koul wrote:

> On Wed, Jul 08, 2015 at 05:11:22PM +0100, Peter Griffin wrote:
> > This patch adds the DT binding documentation for the FDMA constroller
> > found on STi based chipsets from STMicroelectronics.
> > 
> > Signed-off-by: Ludovic Barre 
> > Signed-off-by: Peter Griffin 
> > ---
> >  Documentation/devicetree/bindings/dma/st_fdma.txt | 76 
> > +++
> >  1 file changed, 76 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/dma/st_fdma.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/st_fdma.txt 
> > b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > new file mode 100644
> > index 000..1ec7470
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/st_fdma.txt
> > @@ -0,0 +1,76 @@
> > +* STMicroelectronics Flexible Direct Memory Access Device Tree bindings
> > +
> > +The FDMA is a general-purpose direct memory access controller capable of
> > +supporting 16 independent DMA channels. It accepts up to 32 DMA requests.
> > +The FDMA is based on a Slim processor which require a firmware.
> > +
> > +* FDMA Controller
> > +
> > +Required properties:
> > +- compatible   : Should be "st,fdma_mpe31"
> > +- reg  : Should contain DMA registers location and length
> > +- interrupts   : Should contain one interrupt shared by all channel
> 
> s/channel/channels

Will fix in v2.

> 
> > +- dma-channels : Number of channels supported by the controller
> > +- #dma-cells   : Must be <3>.
> 
> any reason three?

Yes, it is documented further down in the file under DMA Client.

To make it clearer in V2 I could change to

"- #dma-cells   : Must be <3>. See DMA client section below"?

regards,

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

Re: [PATCH v2 05/11] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.

[Peter Griffin]

Hi Mauro,

On Mon, 17 Aug 2015, Mauro Carvalho Chehab wrote:

> Em Thu, 30 Jul 2015 18:08:55 +0100
> Peter Griffin  escreveu:
> 
> > This patch adds in the required DT node for the c8sectpfe
> > Linux DVB demux driver which allows the tsin channels
> > to be used on an upstream kernel.
> > 
> > Signed-off-by: Peter Griffin 
> > ---
> >  arch/arm/boot/dts/stihxxx-b2120.dtsi | 38 
> > 
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
> > b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> > index 62994ae..1bc018e 100644
> > --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> > +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> > @@ -6,6 +6,10 @@
> >   * it under the terms of the GNU General Public License version 2 as
> >   * published by the Free Software Foundation.
> >   */
> > +
> > +#include 
> > +#include 
> > +
> >  / {
> > soc {
> > sbc_serial0: serial@953 {
> > @@ -85,5 +89,39 @@
> > status = "okay";
> > };
> >  
> > +   c8sectpfe@08a2 {
> > +   compatible = "st,stih407-c8sectpfe";
> > +   status = "okay";
> > +   reg = <0x08a2 0x1>, <0x08a0 0x4000>;
> > +   reg-names = "c8sectpfe", "c8sectpfe-ram";
> > +
> > +   interrupts = <0 34 0>, <0 35 0>;
> > +   interrupt-names = "c8sectpfe-error-irq",
> > + "c8sectpfe-idle-irq";
> > +
> > +   pinctrl-names   = "tsin0-serial", "tsin0-parallel",
> > + "tsin3-serial", "tsin4-serial",
> > + "tsin5-serial";
> > +
> > +   pinctrl-0   = <&pinctrl_tsin0_serial>;
> > +   pinctrl-1   = <&pinctrl_tsin0_parallel>;
> > +   pinctrl-2   = <&pinctrl_tsin3_serial>;
> > +   pinctrl-3   = <&pinctrl_tsin4_serial_alt3>;
> > +   pinctrl-4   = <&pinctrl_tsin5_serial_alt1>;
> > +
> > +   clocks = <&clk_s_c0_flexgen CLK_PROC_STFE>;
> > +   clock-names = "c8sectpfe";
> > +
> > +   /* tsin0 is TSA on NIMA */
> > +   tsin0: port@0 {
> > +
> > +   tsin-num = <0>;
> > +   serial-not-parallel;
> > +   i2c-bus = <&ssc2>;
> 
> There's no ssc2 defined at the device tree.

Whoops, I missed one patch off from the series in v2.
> 
> I'll revert this patch and mark the driver as broken until this gets
> fixed.

I just sent you another series here https://lkml.org/lkml/2015/8/25/499
which includes this patch, and also the patch which adds ssc2/3, along
with a fix to the Kconfig file spotted by Valentin Rothberg.

regards,

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

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Rob Herring]

On Sat, Aug 15, 2015 at 8:46 AM, Simon Glass  wrote:
> Hi Rob,
>
> On 14 August 2015 at 14:29, Rob Herring  wrote:
>> On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass  wrote:
>>> -linux-tegra
>>>
>>> Hi,
>>>
>>> On 12 August 2015 at 07:21, Simon Glass  wrote:
 Hi Lucas,

 On 11 August 2015 at 11:05, Lucas Stach  wrote:
> Hi Simon,
>
> why did you send this to the Tegra ML?
>
> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>> This updates the device tree from the kernel version to something 
>> suitable
>> for U-Boot:
>>
>> - Add stdout-path alias for console
>> - Mark the /soc node to be available pre-relocation so that the early
>> serial console works (we need the 'ranges' property to be available)
>>
>> I find it quite strange that you must explicitly enable the parent
>> node, but not the uart node.
>
> U-Boot tries hard to find and bind the UART using the stdout-path
> link. I would like to add it in the UART node also but we were able to
> work around it so far.
>
>>
>>
>> Signed-off-by: Simon Glass 
>> ---
>>
>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
>> b/arch/arm/boot/dts/bcm2835.dtsi
>> index 301c73f..bd6bff6 100644
>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>> @@ -8,6 +8,7 @@
>>
>>   chosen {
>>   bootargs = "earlyprintk console=ttyAMA0";
>> + stdout-path = &uart;
>>   };
>>
>>   soc {
>> @@ -16,6 +17,7 @@
>>   #size-cells = <1>;
>>   ranges = <0x7e00 0x2000 0x0200>;
>>   dma-ranges = <0x4000 0x 0x2000>;
>> + u-boot,dm-pre-reloc;
>
> Why do you need this and why should upstream carry your favourite
> bootloaders configuration? This is in no way hardware description.

 I'm not sure how much you know about U-Boot, so let me know if you
 need more info.

 U-Boot normally starts up by setting up its serial UART and displaying
 a banner message. At this stage typically only a few devices are
 initialised (e.g. maybe just the UART). It then relocates itself to
 the top of memory and starts up all the devices. It throws away any
 previous devices that it set up before relocation and starts again.

 U-Boot uses a thing called driver model (dm) which handles driver
 binding and probing. Driver model has the device tree and would
 normally scan through it and create devices for everything it finds.
>>
>> How do you debug the DM itself? It seems like you still would need
>> something earlier for debug like earlycon in the kernel. u-boot DM is
>> probably simple enough you can get away with using it early, but you
>> often can't as the complexity increases. Ultimately you need something
>> simple that just hits all the registers needed to get characters out.
>> What happens when you add pinmux, clocks, PMIC, power domains, etc. to
>> the DM and they all become dependencies for the UART?
>
> This doesn't seem like a device tree binding question but let me try
> to answer it.
>
> This is a problem - one of the challenges of driver model is that the
> UART gets further away from the reset vector.

This is a common problem for any complex embedded system. Nothing
special about u-boot here. So either we don't need anything because
everyone else has dealt with the problem in some way or we need a
common solution because we all have this problem.

> In U-Boot there is a single debug UART which can be supported by the
> various serial drivers (only one can be selected on each platform).
> There is a special debug_uart_init() function which is supposed to set
> up the UART for that platform. After that, and until driver model UART
> is up, console output can go through the debug UART.

So you don't need this for the common case of an early console, but
only for platforms needing some other platform specific setup?

 Before relocation we don't need every device. Also the CPU is often
 running slowly, perhaps without the cache enabled. SDRAM may not be
 available yet so space is short. We want to avoid starting up things
 that will not be used.

 So this property indicates that the device is needed before relocation
 and should be set up by driver model. We need it to avoid a very slow
 and memory-hungry startup.
>>
>> Can't the need for that property change over time? Either as more
>> drivers are converted to DM you need to add this or you add some
>> feature that depends on a driver (e.g. get a board rev or boot mode
>> from GPIO). You would have backwards compatibility issues with this.
>> I'm somewhat less worried about that for u-boot as we should be
>> bundling the dtb and bootloader rather than kerne

[PATCH 3/3] [media] c8sectpfe: Remove select on undefined LIBELF_32

[Peter Griffin]

LIBELF_32 is not defined in Kconfig, and is left over legacy
which is not required in the upstream driver, so remove it.

Signed-off-by: Peter Griffin 
Suggested-by: Valentin Rothberg 
---
 drivers/media/platform/sti/c8sectpfe/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig 
b/drivers/media/platform/sti/c8sectpfe/Kconfig
index d1bfd4c..b9ec667 100644
--- a/drivers/media/platform/sti/c8sectpfe/Kconfig
+++ b/drivers/media/platform/sti/c8sectpfe/Kconfig
@@ -1,7 +1,6 @@
 config DVB_C8SECTPFE
tristate "STMicroelectronics C8SECTPFE DVB support"
depends on DVB_CORE && I2C && (ARCH_STI || ARCH_MULTIPLATFORM)
-   select LIBELF_32
select FW_LOADER
select FW_LOADER_USER_HELPER_FALLBACK
select DEBUG_FS
-- 
1.9.1

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

[PATCH 2/3] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.

[Peter Griffin]

This patch adds in the required DT node for the c8sectpfe
Linux DVB demux driver which allows the tsin channels
to be used on an upstream kernel.

Signed-off-by: Peter Griffin 
---
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 38 
 1 file changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 62994ae..1bc018e 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -6,6 +6,10 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#include 
+#include 
+
 / {
soc {
sbc_serial0: serial@953 {
@@ -85,5 +89,39 @@
status = "okay";
};
 
+   c8sectpfe@08a2 {
+   compatible = "st,stih407-c8sectpfe";
+   status = "okay";
+   reg = <0x08a2 0x1>, <0x08a0 0x4000>;
+   reg-names = "c8sectpfe", "c8sectpfe-ram";
+
+   interrupts = <0 34 0>, <0 35 0>;
+   interrupt-names = "c8sectpfe-error-irq",
+ "c8sectpfe-idle-irq";
+
+   pinctrl-names   = "tsin0-serial", "tsin0-parallel",
+ "tsin3-serial", "tsin4-serial",
+ "tsin5-serial";
+
+   pinctrl-0   = <&pinctrl_tsin0_serial>;
+   pinctrl-1   = <&pinctrl_tsin0_parallel>;
+   pinctrl-2   = <&pinctrl_tsin3_serial>;
+   pinctrl-3   = <&pinctrl_tsin4_serial_alt3>;
+   pinctrl-4   = <&pinctrl_tsin5_serial_alt1>;
+
+   clocks = <&clk_s_c0_flexgen CLK_PROC_STFE>;
+   clock-names = "c8sectpfe";
+
+   /* tsin0 is TSA on NIMA */
+   tsin0: port@0 {
+
+   tsin-num = <0>;
+   serial-not-parallel;
+   i2c-bus = <&ssc2>;
+   rst-gpio = <&pio15 4 0>;
+
+   dvb-card = ;
+   };
+   };
};
 };
-- 
1.9.1

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

[PATCH 1/3] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3

[Peter Griffin]

Adding these properties makes the I2C bus to the demodulators much
more reliable, and we no longer suffer from I2C errors when tuning.

Signed-off-by: Peter Griffin 
---
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi 
b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index f589fe4..62994ae 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -27,12 +27,18 @@
};
};
 
-   i2c@9842000 {
+   ssc2: i2c@9842000 {
status = "okay";
+   clock-frequency = <10>;
+   st,i2c-min-scl-pulse-width-us = <0>;
+   st,i2c-min-sda-pulse-width-us = <5>;
};
 
-   i2c@9843000 {
+   ssc3: i2c@9843000 {
status = "okay";
+   clock-frequency = <10>;
+   st,i2c-min-scl-pulse-width-us = <0>;
+   st,i2c-min-sda-pulse-width-us = <5>;
};
 
i2c@9844000 {
-- 
1.9.1

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

[PATCH 0/3] c8sectpfe

[Peter Griffin]

Hi Mauro,

This series includes a couple of fixes for the c8sectpfe Linux dvb driver.

One was caused by omitting a patch from the original c8sectpfe series which
defined the ssc2 and ssc3 dt nodes, which was then used by the later DT patch.

This patch is included, along with the original patch which you reverted.

Also Valentin Rothberg spotted LIBELF32 Kconfig symbol I was selecting in the
Kconfig, this isn't required upstream and is left over legacy so I've removed
it.

Sorry for the delay in sending these fixes, I've been on holiday for the last
3 weeks.

kind regards,

Peter.

Peter Griffin (3):
  ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3
  ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.
  [media] c8sectpfe: Remove select on undefined LIBELF_32

 arch/arm/boot/dts/stihxxx-b2120.dtsi | 48 ++--
 drivers/media/platform/sti/c8sectpfe/Kconfig |  1 -
 2 files changed, 46 insertions(+), 3 deletions(-)

-- 
1.9.1

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

Re: [PATCH 1/1] of: to support binding numa node to root subnode(non-bus)

[Rob Herring]

On Mon, Aug 24, 2015 at 9:24 PM, Leizhen (ThunderTown)
 wrote:
>
>
> On 2015/8/24 21:25, Rob Herring wrote:
>> +benh
>>
>> On Mon, Aug 24, 2015 at 7:30 AM, Zhen Lei  wrote:
>>> If use of_platform_populate to scan dt-nodes and add devices, the
>>> subnode of root(such as /smmu), when being scanned and invoke
>>
>> You should have a bus as the sub-node of root rather than devices
>> directly off of root. You still have a problem though...
>
> But actually the parent of bus is also &platform_bus if we didn't have 
> special initialization.
> For example:
> The function of_platform_device_create_pdata invoke of_device_alloc first, 
> then invoke of_device_add.
> But in of_device_alloc, we can find that:
> dev->dev.parent = parent ? : &platform_bus;

This syntax is confusing, but in GCC this is the same as:

dev->dev.parent = parent ? parent : &platform_bus;

And parent is set for everything except nodes off of root.
/sys/devices/platform/* will show the hierarchy.

>>> of_device_add, the ofdev->dev.parent is always equal &platform_bus. So
>>> that, function set_dev_node will not be called. And in device_add,
>>> dev_to_node(parent) always return NUMA_NO_NODE.
>>>
>>> Signed-off-by: Zhen Lei 
>>> ---
>>>  drivers/base/core.c | 2 +-
>>>  drivers/of/device.c | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>> index dafae6d..5df4f46b 100644
>>> --- a/drivers/base/core.c
>>> +++ b/drivers/base/core.c
>>> @@ -1017,7 +1017,7 @@ int device_add(struct device *dev)
>>> dev->kobj.parent = kobj;
>>>
>>> /* use parent numa_node */
>>> -   if (parent)
>>> +   if (parent && (parent != &platform_bus))
>>
>> This is only fixing one specific case, but I think things are broken
>> for any case where the NUMA associativity if not set at the top level
>> bus node. I think this should be something like:
>>
>> if (parent && (dev_to_node(dev) != NO_NUMA_NODE))
>
> It seems a mistake, we should use equal sign.
> if (parent && (dev_to_node(dev) == NUMA_NO_NODE))

Ah, yes.

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

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Leo Yan]

On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote:
> On Tue, 2015-08-25 at 11:42 +0100, Mark Rutland wrote:
> > > > Are you then going to hack GRUB, release a special HiKey version of
> > > > GRUB, not support any other versions, and still can your firmware
> > > > UEFI?
> > > 
> > > I don't need to hack GRUB at all.
> > 
> > Then it is working for you by pure chance alone.
> > 
> > Please listen to the advice you are being given here; we're trying to
> > ensure that your platform functions (and continues to function) as best
> > it can.
> 
> Since we discussed a lot on this, let's make a conclusion on it.
> 
> 1. UEFI could append the reserved buffer in it's memory mapping.
> 2. These reserved buffer must be declared in DT, since we also need to
>support non-UEFI (uboot) at the same time.
> 3. Mailbox node should reference reserved buffer by phandle in DT. Then
>map the buffer as non-cacheable in driver.
> 4. These reserved buffer must use "no-map" property since it should be
>non-cacheable in driver.

For more specific discussion for DTS, i list two options at here;

- Option 1: just simply reserve memory regions through memory node,
  and mailbox node will directly use the buffer through reg ranges;

- Option 2: use reserved-memory and mailbox node will refer phandle
  of reserved-memory;

These two options both can work well with UEFI and Uboot, but option 1
is more simple and straightforward; so i personally prefer it. But
look forwarding your guys' suggestion.

Option 1:

memory@0 {
device_type = "memory";
reg = <0x 0x 0x 0x05e0>,
  <0x 0x05f0 0x 0x00eff000>,
  <0x 0x06e0 0x 0x0060f000>,
  <0x 0x0741 0x 0x38bf>;
};

[...]

mailbox: mailbox@f751 {
#mbox-cells = <1>;
compatible = "hisilicon,hi6220-mbox";
reg = <0x0 0xf751 0x0 0x1000>, /* IPC_S */
  <0x0 0x06dff800 0x0 0x0800>; /* Mailbox buffer */
interrupts = ;
};

Option 2:

memory@0 {
device_type = "memory";
reg = <0x0 0x0 0x0 0x4000>;
};

reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;

mcu_reserved: mcu_reserved@06dff000 {
no-map;
reg = <0x0 0x06dff000 0x0 0x1000>,  /* MCU mailbox 
buffer */
  <0x0 0x05e0 0x0 0x0010>,  /* MCU firmware 
buffer */
  <0x0 0x0740f000 0x0 0x1000>;  /* MCU firmware 
section */
};
};

[...]

mailbox: mailbox@f751 {
#mbox-cells = <1>;
compatible = "hisilicon,hi6220-mbox";
reg = <0x0 0xf751 0x0 0x1000>; /* IPC_S */
memory-region = <&mcu_reserved>;   /* Mailbox buffer */
interrupts = ;
};


> 5. A patch is necessary in kernel. If efi stub feature is enabled,
>arm kernel should not parse memory node or reserved memory buffer in
>DT any more. Arm kernel should either fetch memory information from 
>efi or DT. Currently arm kernel fetch both efi memory information and
>reserved buffer from DTB at the same time.
> 
> Do you agree on these points?
> 
> Regards
> Haojian
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Russell King - ARM Linux]

On Tue, Aug 25, 2015 at 04:21:51PM +0200, Thierry Reding wrote:
> You cited code from dw_hdmi.c earlier, it looks like it might be correct
> even though it doesn't cite a reference for why this was done. Perhaps
> someone on this thread, or someone involved with dw_hdmi can answer
> where that code came from.

dw_hdmi doesn't do any format conversion - it's hard coded to RGB, 8
bits per colour component.  That's a requirement for all HDMI sinks.

The reason it's hard-coded in dw_hdmi is that (a) no one has yet decided
its worth the effort to get the dw_hdmi hardware to do the colourspace
conversion to the YUV spaces and verify that it works, and (b) I really
don't see the point when we're talking about computer like devices which
work primerily with RGB and RGB is always supported by the sink.

As far as greater colour depths go, the driver came from the Freescale
iMX6 code base, and the hardware which feeds dw_hdmi can't do more than
8 bits per component - so going to 10, 12 or 16 bits per component is
beyond what iMX6 can cope with.  Hence, no one on the iMX6 side has a
need for the deep colour stuff.

In any case, I view this as a very low priority issue - it would be nice
to have audio support on HDMI for iMX6 at some point in the next 20 years,
preferably before the hardware becomes obsolete.  I've been maintaining
patches for this for 1.5 years now... how much longer is it going to take?
My pull request to David from 15th July was ignored.  My re-send of that
after he returned was ignored.  My reminder of it has been ignored.  What's
going on in DRM land?  It would be nice to get _some_ kind of feedback so
I know why they're not being taken, so I can fix whatever the issue is.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller

[Brian Norris]

On Tue, Aug 25, 2015 at 12:22:10PM +0200, Marek Vasut wrote:
> On Tuesday, August 25, 2015 at 12:17:37 PM, Cyrille Pitchen wrote:
> > If you don't mind, I'd rather keep some of these inline functions. I have
> > no strong justification, it's more a personal taste: it makes lines
> > shorter as it avoids the need to add "->regs + ".
> > Also it makes the code consistent with other Atmel drivers which already
> > use such wrappers.
> > 
> > However I'll fix the comment and remove the byte and word versions, which
> > are not used. So only qspi_readl() and qspi_writel() are left.
> > 
> > Does it sound good to you?
> 
> In my mind, seeing explicit readl_relaxed() somewhere is much easier to
> digest than seeing some wrapper, which I have to look up. But please do
> wait for others to voice their concern too, I might not be the best person
> to tell you what to do when it comes to wrapping IO accessors ;-)

I could go either way, but there are times where local wrapper I/O
accessors are useful. Case in point: it makes it really easy to make the
choice between readl() and readl_relaxed() in one place (i.e., the
discussion you had in another branch of this thread). That's been useful
for me on brcmnand, where certain platforms (big-endian MIPS) have
different assumptions about endianness than your average platform. Also,
it helps with things like what Robert Jarzmik is trying to do on
pxa3xx_nand -- add debug info to print every register read/write.

Also as Cyrille mentioned, personal taste is a factor.

Anyway, I'll go with whatever makes sense between y'all. I don't mind.

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

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Ard Biesheuvel]

On 25 August 2015 at 17:37, Leif Lindholm  wrote:
> On Tue, Aug 25, 2015 at 04:51:22PM +0200, Ard Biesheuvel wrote:
>> >>Arm kernel should either fetch memory information from
>> >>efi or DT.
>> >
>> > Absolutely.
>> >
>> >>Currently arm kernel fetch both efi memory information and
>> >>reserved buffer from DTB at the same time.
>> >
>> > No, it does not.
>>
>> It should not, but it does. Due to an oversight, the stub removes
>> /memreserve/ entries but ignores the reserved-memory node completely.
>
> Urgh.
>
>> This was reported here in fact
>>
>> http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742
>>
>> but there has not been a followup to this series.
>
> Are all of those patches still relevant, or did some of them go in
> already?
>

The first two patches are in v4.2-rc1 and up, the others should still
apply on top of that.

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

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Leif Lindholm]

On Tue, Aug 25, 2015 at 04:51:22PM +0200, Ard Biesheuvel wrote:
> >>Arm kernel should either fetch memory information from
> >>efi or DT.
> >
> > Absolutely.
> >
> >>Currently arm kernel fetch both efi memory information and
> >>reserved buffer from DTB at the same time.
> >
> > No, it does not.
> 
> It should not, but it does. Due to an oversight, the stub removes
> /memreserve/ entries but ignores the reserved-memory node completely.

Urgh.

> This was reported here in fact
> 
> http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742
> 
> but there has not been a followup to this series.

Are all of those patches still relevant, or did some of them go in
already?

Haojian: can you give that patch a spin and see if it does what you
need, combined with adding the reserved areas to the UEFI memory map?

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

[PATCH] arm64: dts: qcom: Add RNG device tree node

[Stanimir Varbanov]

Adds rng device tree node for msm8916 SoCs.

Signed-off-by: Stanimir Varbanov 
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi |7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi 
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0f49ebd0aa8b..060c6c53ff33 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -215,5 +215,12 @@
interrupt-controller;
#interrupt-cells = <4>;
};
+
+   rng@22000 {
+   compatible = "qcom,prng";
+   reg = <0x00022000 0x200>;
+   clocks = <&gcc GCC_PRNG_AHB_CLK>;
+   clock-names = "core";
+   };
};
 };
-- 
1.7.9.5

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

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Ard Biesheuvel]

On 25 August 2015 at 16:24, Leif Lindholm  wrote:
> On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote:
>> Since we discussed a lot on this, let's make a conclusion on it.
>>
>> 1. UEFI could append the reserved buffer in it's memory mapping.
>
> Yes. It needs to.
>
> (I will let Mark comment on points 2-4.)
>
>> 5. A patch is necessary in kernel. If efi stub feature is enabled,
>>arm kernel should not parse memory node or reserved memory buffer in
>>DT any more.
>
> This is already the case. The stub deletes any present memory nodes and
> reserved entries in drivers/firmware/efi/libstub/fdt.c:update_fdt().
>
> Then, during setup_arch(), arch/arm64/kernel/efi.c:efi_init() calls
> reserve_regions(), which adds only those memory regions available for
> use by Linux as RAM to memblock.
>
>>Arm kernel should either fetch memory information from
>>efi or DT.
>
> Absolutely.
>
>>Currently arm kernel fetch both efi memory information and
>>reserved buffer from DTB at the same time.
>
> No, it does not.
>

It should not, but it does. Due to an oversight, the stub removes
/memreserve/ entries but ignores the reserved-memory node completely.

This was reported here in fact

http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742

but there has not been a followup to this series.

I think it is fine to keep those memory reservations in the DT, but
you should simply understand that UEFI does not parse the DT, so you
need to tell it which memory it cannot touch. Otherwise, the firmware
itself or anything that executes under it (UEFI drivers, the UEFI
Shell, GRUB, the UEFI stub in the kernel) will think it is available
and may allocate it for its own use. This may include runtime services
regions that will remain reserved even after exiting boot services.

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

Re: [PATCH v2 16/16] mmc: host: omap_hsmmc: use "mmc_of_parse_voltage" to get ocr_avail

[Ulf Hansson]

On 3 August 2015 at 14:26, Kishon Vijay Abraham I  wrote:
> From: Roger Quadros 
>
> For platforms that doesn't have explicit regulator control in MMC,
> populate voltage-ranges in MMC device tree node and use
> mmc_of_parse_voltage to get ocr_avail

I don't like this.

If we are able to fetch the OCR mask via an external regulator, that
shall be done.

I think the mmc_of_parse_voltage() API and the corresponding DT
binding it parses, should be used for those HW when we don't have an
external regulator to use. For example if the MMC controller itself
somehow controls the voltage levels. Is that really the case for you?

Kind regards
Uffe

>
> Signed-off-by: Roger Quadros 
> Signed-off-by: Lokesh Vutla 
> Signed-off-by: Murali Karicheri 
> Signed-off-by: Franklin S Cooper Jr 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt  |2 ++
>  drivers/mmc/host/omap_hsmmc.c  |9 -
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt 
> b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index 76bf087..2408e87 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -22,6 +22,8 @@ ti,dual-volt: boolean, supports dual voltage cards
>  ti,non-removable: non-removable slot (like eMMC)
>  ti,needs-special-reset: Requires a special softreset sequence
>  ti,needs-special-hs-handling: HSMMC IP needs special setting for handling 
> High Speed
> +voltage-ranges: Specify the voltage range supported if regulator framework
> +isn't enabled.
>  dmas: List of DMA specifiers with the controller specific format
>  as described in the generic DMA client binding. A tx and rx
>  specifier is required.
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 15973f1..d884d8f 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -2184,7 +2184,13 @@ static int omap_hsmmc_probe(struct platform_device 
> *pdev)
> goto err_irq;
> }
>
> -   mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
> +   if (!mmc_pdata(host)->ocr_mask) {
> +   ret = mmc_of_parse_voltage(pdev->dev.of_node, 
> &mmc->ocr_avail);
> +   if (ret)
> +   goto err_parse_voltage;
> +   } else {
> +   mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
> +   }
>
> omap_hsmmc_disable_irq(host);
>
> @@ -2224,6 +2230,7 @@ static int omap_hsmmc_probe(struct platform_device 
> *pdev)
>
>  err_slot_name:
> mmc_remove_host(mmc);
> +err_parse_voltage:
> omap_hsmmc_reg_put(host);
>  err_irq:
> device_init_wakeup(&pdev->dev, false);
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] qcom: ipq4019: Add basic board/dts support for IPQ4019 SoC

[Jonas Gorski]

Hi,

On Tue, Aug 25, 2015 at 2:08 PM, Varadarajan Narayanan
 wrote:
> Add initial dts files and SoC support for IPQ4019
>
> Signed-off-by: Varadarajan Narayanan 
> ---
> Changes in v2:
>   - Added devicetree bindings documentation
>
> Changes in v3:
>   - Split 'gcnt' into a separate patch
>   - Added the new entries to Makefiles, Kconfig & board.c in sorted order
>   - Used "qcom,ipq40xx" instead of "qcom,ipq40xx-r3pc" in board.c
>
> Changes in v4:
>   - Renamed IPQ40xx as IPQ4019
>   - Moved kconfig and defconfig enablement to separate patch
>   - Dropped Documentation/devicetree/bindings/qcom.txt
>
>  arch/arm/boot/dts/Makefile  |  1 +
>  arch/arm/boot/dts/qcom-ipq4019-r3pc.dts | 34 ++
>  arch/arm/boot/dts/qcom-ipq4019.dtsi | 81 
> +
>  3 files changed, 116 insertions(+)
>  create mode 100644 arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
>  create mode 100644 arch/arm/boot/dts/qcom-ipq4019.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 246473a..ca603f3 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -474,6 +474,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
> qcom-apq8074-dragonboard.dtb \
> qcom-apq8084-ifc6540.dtb \
> qcom-apq8084-mtp.dtb \
> +   qcom-ipq4019-r3pc.dtb \
> qcom-ipq8064-ap148.dtb \
> qcom-msm8660-surf.dtb \
> qcom-msm8960-cdp.dtb \
> diff --git a/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts 
> b/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
> new file mode 100644
> index 000..e3855d7
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
> @@ -0,0 +1,34 @@
> +/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "qcom-ipq4019.dtsi"
> +
> +/ {
> +   model = "Qualcomm Technologies, Inc. IPQ4019 R3PC";
> +   compatible = "qcom,ipq4019-r3pc", "qcom,ipq4019";
> +
> +   memory {
> +   device_type = "memory";
> +   reg = <0x8000 0x2000>; /* 512MB */
> +   };
> +
> +   chosen {
> +   bootargs = "root=/dev/ram rw init=/init 
> initrd=0x8200,0x000E2246";
> +   stdout-path = "/serial@78b:115200";

I suggest adding an alias to the serial node, so you can just use the
alias name instead of the path (also I never tried using an actual
path, but wouldn't it be "/soc/serial@78b"?).

> +   };
> +
> +   soc {
> +   serial@78b {
> +   status = "ok";
> +   };
> +   };
> +};
> diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi 
> b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> new file mode 100644
> index 000..1be0322
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> @@ -0,0 +1,81 @@
> +/*
> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +/dts-v1/;
> +
> +#include "skeleton.dtsi"
> +
> +/ {
> +   model = "Qualcomm Technologies, Inc. IPQ4019";
> +   compatible = "qcom,ipq4019";
> +   interrupt-parent = <&intc>;
> +
> +   cpus {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   cpu@0 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a7";
> +   reg = <0x0>;
> +   };
> +
> +   cpu@1 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a7";
> +   reg = <0x1>;
> +   };
> +
> +   cpu@2 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a7";
> +   reg = <0x2>;
> +   };
> +
> +   cpu@3 {
> +   device_type = "cpu";
> +   compatible = "arm,cortex-a7";
> +   reg = <0x3>;
> +   };
> +   };
> +
> +   soc {
> +   #address-cells = <1>;
> +   #size-cells =

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Leif Lindholm]

On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote:
> Since we discussed a lot on this, let's make a conclusion on it.
> 
> 1. UEFI could append the reserved buffer in it's memory mapping.

Yes. It needs to.

(I will let Mark comment on points 2-4.)

> 5. A patch is necessary in kernel. If efi stub feature is enabled,
>arm kernel should not parse memory node or reserved memory buffer in
>DT any more.

This is already the case. The stub deletes any present memory nodes and
reserved entries in drivers/firmware/efi/libstub/fdt.c:update_fdt().

Then, during setup_arch(), arch/arm64/kernel/efi.c:efi_init() calls
reserve_regions(), which adds only those memory regions available for
use by Linux as RAM to memblock.

>Arm kernel should either fetch memory information from 
>efi or DT.

Absolutely.

>Currently arm kernel fetch both efi memory information and
>reserved buffer from DTB at the same time.

No, it does not.

Regards,

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

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Yakir Yang]

Hi Thierry,

在 2015/8/25 22:16, Thierry Reding 写道:

On Tue, Aug 25, 2015 at 09:48:01PM +0800, Yakir Yang wrote:

Hi Thierry & Rob,

在 2015/8/25 21:27, Rob Herring 写道:

On Tue, Aug 25, 2015 at 4:15 AM, Thierry Reding  wrote:

On Sun, Aug 23, 2015 at 06:23:14PM -0500, Rob Herring wrote:

On Wed, Aug 19, 2015 at 9:50 AM, Yakir Yang  wrote:

[...]

+   -analogix,link-rate:
+   max link rate supported by the eDP controller.
+   LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A,
+   LINK_RATE_5_40GBPS = 0x14

Same here. I'd rather see something like "link-rate-mbps" and use the
actual rate.

There is no need whatsoever to hard-code this in DT. (e)DP provides the
means to detect what rate the link supports and the specification
provides guidance on how to select an appropriate one.

Good, even better.

I do think we still need keep this DT prop yet.

I think drm_dp_help.c could get the "panel" max link-rate and lane-count,
but it's not enough, we still need knew the "eDP controller" max link-rate
and lane-count.

Let me show the exact example that happened in my side. When I connect
my board to my 2K DP-1.2 TV. Analogix dp driver would get the max link-rate
from dpcd, and the max link-rate is 5.4Gbps. So if I just set eDP controller
link-rate
to 5.4Gbps, the DP TV just broken, do not light up normally.

This reason why TV broken is the max link-rate which support by RK3288 eDP
controller is 2.7Gbps. Here are the exact words that RK3288 eDP TRM said:

*Compliant with DisplayPortTM Specification, Version 1.2.
Compliant with eDPTM Specification, Version 1.3.
HDCP v1.3 amendment for DisplayPortTM Revision 1.0.
Main link containing 4 physical lanes of 2.7/1.62 Gbps/lane
*
**


Beside I haven't found there are some registers would indicate the eDP
controller
max link-rate and lane-count, so this is why I still instance that we need
this DT
prop to indicata "Max rate controller support".

So, I wish you could agree with me on this point.

Your driver should know what link rates it supports and restrict itself
to use those. This is implied by the compatible string and doesn't need
to be duplicated into device tree.


Oh, yeah, good idea   :-D
Thanks for your point out.

- Yakir


Thierry



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

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Thierry Reding]

On Tue, Aug 25, 2015 at 10:03:52PM +0800, Yakir Yang wrote:
> Hi Thierry,
> 
> 在 2015/8/25 17:58, Thierry Reding 写道:
> >On Wed, Aug 19, 2015 at 09:50:34AM -0500, Yakir Yang wrote:
> >[...]
> >>+   -analogix,color-space:
> >>+   input video data format.
> >>+   COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
> >I don't think DT is an appropriate place to set this. To my knowledge
> >this depends on the display and/or mode, so I don't think hard-coding
> >it here is the right thing to do.
> 
> Yeah, same question with my previous reply ;)

I don't have an answer for you, unfortunately. But like I said,
hard-coding isn't going to work. What if, for example, you set this to a
fixed value and then you connect a monitor that doesn't support the
specific one you set?

You cited code from dw_hdmi.c earlier, it looks like it might be correct
even though it doesn't cite a reference for why this was done. Perhaps
someone on this thread, or someone involved with dw_hdmi can answer
where that code came from.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Thierry Reding]

On Tue, Aug 25, 2015 at 09:48:01PM +0800, Yakir Yang wrote:
> Hi Thierry & Rob,
> 
> 在 2015/8/25 21:27, Rob Herring 写道:
> >On Tue, Aug 25, 2015 at 4:15 AM, Thierry Reding  wrote:
> >>On Sun, Aug 23, 2015 at 06:23:14PM -0500, Rob Herring wrote:
> >>>On Wed, Aug 19, 2015 at 9:50 AM, Yakir Yang  wrote:
> >>[...]
> +   -analogix,link-rate:
> +   max link rate supported by the eDP controller.
> +   LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 
> 0x0A,
> +   LINK_RATE_5_40GBPS = 0x14
> >>>Same here. I'd rather see something like "link-rate-mbps" and use the
> >>>actual rate.
> >>There is no need whatsoever to hard-code this in DT. (e)DP provides the
> >>means to detect what rate the link supports and the specification
> >>provides guidance on how to select an appropriate one.
> >Good, even better.
> 
> I do think we still need keep this DT prop yet.
> 
> I think drm_dp_help.c could get the "panel" max link-rate and lane-count,
> but it's not enough, we still need knew the "eDP controller" max link-rate
> and lane-count.
> 
> Let me show the exact example that happened in my side. When I connect
> my board to my 2K DP-1.2 TV. Analogix dp driver would get the max link-rate
> from dpcd, and the max link-rate is 5.4Gbps. So if I just set eDP controller
> link-rate
> to 5.4Gbps, the DP TV just broken, do not light up normally.
> 
> This reason why TV broken is the max link-rate which support by RK3288 eDP
> controller is 2.7Gbps. Here are the exact words that RK3288 eDP TRM said:
> 
> *Compliant with DisplayPortTM Specification, Version 1.2.
> Compliant with eDPTM Specification, Version 1.3.
> HDCP v1.3 amendment for DisplayPortTM Revision 1.0.
> Main link containing 4 physical lanes of 2.7/1.62 Gbps/lane
> *
> **
> 
> 
> Beside I haven't found there are some registers would indicate the eDP
> controller
> max link-rate and lane-count, so this is why I still instance that we need
> this DT
> prop to indicata "Max rate controller support".
> 
> So, I wish you could agree with me on this point.

Your driver should know what link rates it supports and restrict itself
to use those. This is implied by the compatible string and doesn't need
to be duplicated into device tree.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Sudeep Holla]

On 25/08/15 15:04, Leo Yan wrote:

Hi Sudeep,

On Tue, Aug 25, 2015 at 12:36:12PM +0100, Sudeep Holla wrote:



On 19/08/15 10:37, Leo Yan wrote:

On Hi6220, below memory regions in DDR have specific purpose:

   0x05e0, - 0x05ef,: For MCU firmware using at runtime;
   0x0740,f000 - 0x0740,: For MCU firmware's section;
   0x06df,f000 - 0x06df,: For mailbox message data.



Unless I am reading the DTS file completely wrong, I don't think the
above memory regions are in DDR as per the memory node.


i'm not sure if understand correctly for your question; Hikey board
has DDR 1GB@0x0, but there have some memory regions are used for MCU.



Ah, I misread the address range, left the leading zero and assumed they
are not in DDR range. Sorry for the noise.

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

omap_wdt: node present in device-tree but kmod is not autoloaded

[Paolo Pisati]

The question is two-folded, hence the crossposting.

I have a kernel that runs perfectly on my beaglebone black, except for one thing
- the kmod for the watchdog is not autoloaded upon boot albeit the device-tree
node is present:

ubuntu@beaglebone:~$ dmesg | grep wdt
ubuntu@beaglebone:~$ ll /dev/watch* 
ls: cannot access /dev/watch*: No such file or directory
ubuntu@beaglebone:~$ ls -la /proc/device-tree/ocp/wdt\@44e35000/
total 0
drwxr-xr-x  2 root root  0 Aug 25 13:43 .
drwxr-xr-x 54 root root  0 Aug 25 13:43 ..
-r--r--r--  1 root root 13 Aug 25 13:43 compatible
-r--r--r--  1 root root  4 Aug 25 13:43 interrupts
-r--r--r--  1 root root  4 Aug 25 13:43 name
-r--r--r--  1 root root  8 Aug 25 13:43 reg
-r--r--r--  1 root root 10 Aug 25 13:43 ti,hwmods
ubuntu@beaglebone:~$ cat /proc/device-tree/ocp/wdt\@44e35000/compatible 
ti,omap3-wdt
ubuntu@beaglebone:~$ ls -la /sys/bus/platform/devices/ocp/*wdt*
total 0
drwxr-xr-x  3 root root0 Aug 25 13:49 ./
drwxr-xr-x 33 root root0 Aug 25 13:49 ../
-rw-r--r--  1 root root 4096 Aug 25 13:49 driver_override
-r--r--r--  1 root root 4096 Aug 25 13:49 modalias
lrwxrwxrwx  1 root root0 Aug 25 13:49 of_node ->
../../../../firmware/devicetree/base/ocp/wdt@44e35000/
drwxr-xr-x  2 root root0 Aug 25 13:49 power/
lrwxrwxrwx  1 root root0 Aug 25 13:49 subsystem -> ../../../../bus/platform/
-rw-r--r--  1 root root 4096 Aug 25 13:49 uevent

and if i manually load the module:

ubuntu@beaglebone:~$ sudo modprobe omap_wdt
ubuntu@beaglebone:~$ dmesg | grep wdt
[  388.104283] omap_wdt: OMAP Watchdog Timer Rev 0x01: initial timeout 60 sec
ubuntu@beaglebone:~$ ls -la /dev/watchdog*
crw--- 1 root root  10, 130 Aug 25 13:46 /dev/watchdog
crw--- 1 root root 251,   0 Aug 25 13:46 /dev/watchdog0
ubuntu@beaglebone:~$ ls -la /sys/bus/platform/devices/ocp/*wdt* 

total 0
drwxr-xr-x  3 root root0 Aug 25 13:53 .
drwxr-xr-x 33 root root0 Aug 25 13:49 ..
lrwxrwxrwx  1 root root0 Aug 25 13:53 driver ->
../../../../bus/platform/drivers/omap_wdt
-rw-r--r--  1 root root 4096 Aug 25 13:49 driver_override
-r--r--r--  1 root root 4096 Aug 25 13:49 modalias
lrwxrwxrwx  1 root root0 Aug 25 13:49 of_node ->
../../../../firmware/devicetree/base/ocp/wdt@44e35000
drwxr-xr-x  2 root root0 Aug 25 13:49 power
lrwxrwxrwx  1 root root0 Aug 25 13:49 subsystem -> ../../../../bus/platform
-rw-r--r--  1 root root 4096 Aug 25 13:49 uevent

and then the device works as expected.

So here are my questions:

1) My understanding was that, once a device node was present in the device-tree
and there was a compatible driver available, it would have been autoloaded, so 
why it's
not happening here?

2) And more importantly, how do i debug these situations?
Building a kernel with the driver built-in, and comparing the two dmesgs, didn't
show any noticeable difference either.

Any idea?
-- 
bye,
p.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Leo Yan]

Hi Sudeep,

On Tue, Aug 25, 2015 at 12:36:12PM +0100, Sudeep Holla wrote:
> 
> 
> On 19/08/15 10:37, Leo Yan wrote:
> >On Hi6220, below memory regions in DDR have specific purpose:
> >
> >   0x05e0, - 0x05ef,: For MCU firmware using at runtime;
> >   0x0740,f000 - 0x0740,: For MCU firmware's section;
> >   0x06df,f000 - 0x06df,: For mailbox message data.
> >
> 
> Unless I am reading the DTS file completely wrong, I don't think the
> above memory regions are in DDR as per the memory node.

i'm not sure if understand correctly for your question; Hikey board
has DDR 1GB@0x0, but there have some memory regions are used for MCU.

So this patch is to reserve these memory regions so that make sure
kernel will not map and allocate them.

Will remove these memory regions from memory node in next version.

> >This patch reserves these memory regions and add device node for
> >mailbox in dts.
> >
> >Signed-off-by: Leo Yan 
> >---
> >  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 20 +---
> >  arch/arm64/boot/dts/hisilicon/hi6220.dtsi  |  8 
> >  2 files changed, 25 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts 
> >b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> >index e36a539..d5470d3 100644
> >--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> >+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
> >@@ -7,9 +7,6 @@
> >
> >  /dts-v1/;
> >
> >-/*Reserved 1MB memory for MCU*/
> >-/memreserve/ 0x05e0 0x0010;
> >-
> >  #include "hi6220.dtsi"
> >
> >  / {
> >@@ -28,4 +25,21 @@
> > device_type = "memory";
> > reg = <0x0 0x0 0x0 0x4000>;
> > };
> 
> I have no access to the spec, but I read this as 1GB RAM @0x0
> Unless this entry is completely wrong, what your commit log claims is
> incorrect. If this entry is wrong I wonder how is it booting with this
> DT then.

Do you mean should remove all reserved memory regions from memory
node? Will submit next version's patch for this.

Kernel can boot successfully on Hikey with this patch [1].

[1] https://github.com/96boards/linux

> >+
> >+reserved-memory {
> >+#address-cells = <2>;
> >+#size-cells = <2>;
> >+ranges;
> >+
> >+mcu-buf@05e0 {
> >+no-map;
> >+reg = <0x0 0x05e0 0x0 0x0010>,  /* MCU firmware 
> >buffer */
> >+  <0x0 0x0740f000 0x0 0x1000>;  /* MCU firmware 
> >section */
> 
> So I don't see how can this be part of DDR ? Or at-least part of DDR
> that's mapped by kernel ?

Here use reserved-memory node to remove these regions from memblock
during kernel's boot up; kernel also will not map for them with
property "no-map".

I think this is the same question which have been brought up by Mark
in his early mail and suggested to use UEFI to do this.

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Yakir Yang]

Hi Thierry,

在 2015/8/25 17:58, Thierry Reding 写道:

On Wed, Aug 19, 2015 at 09:50:34AM -0500, Yakir Yang wrote:
[...]

+   -analogix,color-space:
+   input video data format.
+   COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2

I don't think DT is an appropriate place to set this. To my knowledge
this depends on the display and/or mode, so I don't think hard-coding
it here is the right thing to do.


Yeah, same question with my previous reply ;)

Thanks,
- Yakir


Thierry



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

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Yakir Yang]

Hi Thierry,

在 2015/8/25 18:06, Thierry Reding 写道:

On Tue, Aug 25, 2015 at 05:41:19PM +0800, Yakir Yang wrote:

Hi Thierry,

在 2015/8/25 17:12, Thierry Reding 写道:

On Mon, Aug 24, 2015 at 09:48:27AM -0500, Rob Herring wrote:

On Mon, Aug 24, 2015 at 7:57 AM, Russell King - ARM Linux
 wrote:

On Sun, Aug 23, 2015 at 06:23:14PM -0500, Rob Herring wrote:

On Wed, Aug 19, 2015 at 9:50 AM, Yakir Yang  wrote:

+   -analogix,color-depth:
+   number of bits per colour component.
+   COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3

This seems pretty generic. Just use 6, 8, 10, or 12 for values. And
drop the vendor prefix.

Please think about this some more.  What does "color-depth" mean?  Does it
mean the number of bits per colour _component_, or does it mean the total
number of bits to represent a particular colour.  It's confusing as it
stands.

Then "component-color-bpp" perhaps?

There should be no need to have this in DT at all. The BPC is a property
of the attached panel and it should come from the panel (either the
panel driver or parsed from EDID if available).

Actually I have send an email about this one to you in version 2, just past
from that email:

"samsung,color_space" and "samsung,color-depth"

The drm_display_info's color_formats and bpc indicate the monitor display
ability, but
the edp driver could not take it as input video format directly.

For example, with my DP TV I would found "RGB444 & YCRCB422 & & YCRCB444"
support in drm_display_info.color_formats and 16bit bpc support, but RK3288
crtc
driver could only output RGB & ITU formats, so finally analogix_dp-rockchip
driver
config crtc to RGBaaa 10bpc mode.

In this sutiation, the analogix_dp core driver would pazzled by the
drm_display_info,
can't chose the right color_space and bpc.

And this is the place that confused me, wish you could give some ideas about
this one :-)

Your display driver should choose whatever it is capable of outputting.
If the display reports that it can do 16 bits-per-color, but your
display driver can't do it, then it should choose a configuration that
it supports. Similarily for the color encodings. If you can't generate
YCrCb444 with your hardware, then it's the driver's job to know about
that and select the next appropriate configuration.

But hard-coding this is not the right solution because the value in DT
may end up conflicting with what the display reports.


Yeah, thanks for your explain, you are right. It's the best way to get 
"color-depth"

and "color-space" from display driver, not to hard-code in DT prop.

But if the common analogix-dp driver want to get those values, then 
those values
should come from the common drm struct data. Personally I think "struct 
drm_crtc"

is the best place that should indicate the output ability of SoC vop/lcdc.

But I haven't find out there are some place to store those message for 
now (I don't
think it's good to modify the original color-space and color-bpc which 
parsed from

monitor edid).

So could you share sme ideas about this, and I would rather to talk with 
Mark (Author

of rockchip drm driver) to find out the better way to fix this one.

Besides, I would appreciate very much if you can share some ideas about 
how Exynos

handler with this problem ;)

Thanks,
- Yakir

Thierry



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

Re: [PATCH v2] mmc: sunxi: Don't start commands while the card is busy

[Ulf Hansson]

On 25 August 2015 at 14:09, Hans de Goede  wrote:
> Hi,
>
>
> On 25-08-15 14:05, Ulf Hansson wrote:
>>
>> On 1 August 2015 at 11:01, Hans de Goede  wrote:
>>>
>>> Hi,
>>>
>>> On 21-07-15 14:15, Ulf Hansson wrote:


 On 10 July 2015 at 17:14, Hans de Goede  wrote:
>
>
> Some sdio wifi modules have not been working reliable with the
> sunxi-mmc
> host code. This turns out to be caused by it starting new commands
> while
> the card signals that it is still busy processing a previous command.



 Which are those commands?
>>>
>>>
>>>
>>> The code were things get stuck when not waiting for the busy signal uses
>>> the following sdio helper functions:
>>>
>>> sdio_readb/sdio_writeb
>>> sdio_f0_readb/sdio_f0_writeb
>>> sdio_readw/sdio_writew
>>> sdio_readl/sdio_writel
>>> sdio_readsb
>>> sdio_memcpy_fromio/sdio_memcpy_toio
>>>
>>> And directly issues the following command:
>>>
>>>  mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
>>>  mmc_cmd.opcode = SD_IO_RW_EXTENDED;
>>>  mmc_cmd.arg = write ? 1<<31 : 0;/* write flag  */
>>>  mmc_cmd.arg |= (fn & 0x7) << 28;/* SDIO func num */
>>>  mmc_cmd.arg |= 1<<27;   /* block mode */
>>>  /* for function 1 the addr will be incremented */
>>>  mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
>>>  mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
>>>
>>> I do not know if the busy wait is necessary for all
>>> of these, but the hack in the android kernel code,
>>> which inserts calls to a wait_card_busy function directly
>>> into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>>
>>> Does so for all of these.
>>>
 Or more interesting, which response types do

 these commands expect?
 I don't think this is a sunxi specific issue, I have seen similar
 issues for other host controllers.
>>>
>>>
>>>
>>> Agreed, recent dw-mmc patches address the same issue, also involving
>>> broadcom sdio wifi chips.
>>>
 I am thinking that perhaps this should be managed by the mmc core
 instead of local hacks to sunxi. Potentially we could make the core to
 invoke the already existing host_ops->card_busy() callback when
 needed...
>>>
>>>
>>>
>>> I'm fine with solving this either way, implementing host_ops->card_busy()
>>> for sunxi is easy, and if the core os modified to call that function
>>> before issue sdio io ops (which seems to be the common thing here),
>>> then that indeed is better then having the sunxi code always do
>>> the busy check.
>>
>>
>> Okay, so let's make the core to call the ->card_busy() callback and
>> then it's up to each host driver to implement that callback.
>
>
> Ok, do you plan to do a patch for this, or do you expect to get
> one submitted to you ?

I prefer to get a patch submitted to me. My bandwidth has been limited
and still have a lot to catch up with...

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

Myir AM437x Rico board support for Linux 4.1 kernel

[Pavel Pisa]

Hello everybody,

I have been asked to help with Linux on Myir Rico board.
I have decided to try actual stable kernel from Ti tree

  git://git.ti.com/ti-linux-kernel/ti-linux-kernel.git
 branch ti-linux-4.1.y

instead of delivered dated 3.12.10 one to solve some issues
and have chance to port possible enhancements to mainline.

I have successfully prepared environment which loads kernel
over TFTP and boot actual Debian Jessie armhf from NFS export.

Because I have not found up to date device tree for this
board I have analyzed delivered sources against best matching
Ti kernel and then prepared DTS which works on the board
and I want to know if somebody works on this support, can help
to finish/better tune parameters and if there is chance
that configs are included into mainline or some other public
repository. Because I do not know who is working on this support
and who is right maintainer I send only pointers to my files.

http://cmp.felk.cvut.cz/~pisa/arm/myir-rico/am437x-myir-ricoboard.dts
 based on Ti am437x-gp-evm.dts

http://cmp.felk.cvut.cz/~pisa/arm/myir-rico/am437x-myir-ricoboard-hdmi.dts
 based on Ti am437x-gp-evm-hdmi.dts

http://cmp.felk.cvut.cz/~pisa/arm/myir-rico/am437x-myir-ricoboard_defconfig

If there is interrest for discussion and inclusion, I send usual
inlined GIT patches series.

As for DTS, I expect that the regulators section does not match correctly
Rico board HW but it works for what client company needs
  evm_v3_3d: fixedregulator-v3_3d
  vtt_fixed: fixedregulator-vtt

Ethernet, serials, cameras, local MTD, SDcard and most of other devices works.
SPI Flash does not work. I have not invested time to that.

The client needs to connect special camera (infrared) to the VPFE so I have
spent some time on AM437x VPFE support enhancement.

I have added V4L2_PIX_FMT_Y10 pixel format and some minimal/stub 
implementation of vpfe_enum_frameintervals, vpfe_g_parm, vvpfe_s_parm, 
pfe_s_parm and I would be happy if my work can help to somebody other
as well. But work has been driven strict deadline to provide working
solution in one week so if there is interrest then most has to be
rewritten/clean up.

Generally, I am quite pleased by Ti's Linux tree state and support.
That week has been my first time with AM chips we used other archs
(powerpc and multiple i.MXs for most of our other projects till now).

Probably off topic for these lists but what are suggested options
for graphic configuration when Qt4/E or Qt5 is run on framebuffer.
Is it better to use CONFIG_DRM_OMAP or CONFIG_FB_OMAP2 ?
Is SGX driver available for 4.1 Linux kernel (reply offlist, please,
if proprietary only)? Are there other possibilities how to use
OMAP AM437xx hardware to accelerate video color and formats
transformations? Can be VPFE planes used somehow to embed
video in Qt application? I, personally, prefer strongly
opensource only options but target company has probably minimal
requirements in this area.

Thanks for feedback,

Pavel Pisa
e-mail: p...@cmp.felk.cvut.cz
www:http://cmp.felk.cvut.cz/~pisa
university: http://dce.fel.cvut.cz/
company:http://www.pikron.com/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Haojian Zhuang]

On Tue, 2015-08-25 at 11:42 +0100, Mark Rutland wrote:
> > > Are you then going to hack GRUB, release a special HiKey version of
> > > GRUB, not support any other versions, and still can your firmware
> > > UEFI?
> > 
> > I don't need to hack GRUB at all.
> 
> Then it is working for you by pure chance alone.
> 
> Please listen to the advice you are being given here; we're trying to
> ensure that your platform functions (and continues to function) as best
> it can.

Since we discussed a lot on this, let's make a conclusion on it.

1. UEFI could append the reserved buffer in it's memory mapping.
2. These reserved buffer must be declared in DT, since we also need to
   support non-UEFI (uboot) at the same time.
3. Mailbox node should reference reserved buffer by phandle in DT. Then
   map the buffer as non-cacheable in driver.
4. These reserved buffer must use "no-map" property since it should be
   non-cacheable in driver.
5. A patch is necessary in kernel. If efi stub feature is enabled,
   arm kernel should not parse memory node or reserved memory buffer in
   DT any more. Arm kernel should either fetch memory information from 
   efi or DT. Currently arm kernel fetch both efi memory information and
   reserved buffer from DTB at the same time.

Do you agree on these points?

Regards
Haojian

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

Re: [PATCH v3 06/14] Documentation: drm/bridge: add document for analogix_dp

[Rob Herring]

On Tue, Aug 25, 2015 at 4:15 AM, Thierry Reding  wrote:
> On Sun, Aug 23, 2015 at 06:23:14PM -0500, Rob Herring wrote:
>> On Wed, Aug 19, 2015 at 9:50 AM, Yakir Yang  wrote:
> [...]
>> > +   -analogix,link-rate:
>> > +   max link rate supported by the eDP controller.
>> > +   LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 
>> > 0x0A,
>> > +   LINK_RATE_5_40GBPS = 0x14
>>
>> Same here. I'd rather see something like "link-rate-mbps" and use the
>> actual rate.
>
> There is no need whatsoever to hard-code this in DT. (e)DP provides the
> means to detect what rate the link supports and the specification
> provides guidance on how to select an appropriate one.

Good, even better.

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

Re: [PATCH v1 1/3] dt-bindings: mailbox: Document Hi6220 mailbox driver

[Leo Yan]

Hi Sudeep,

Thanks for review, please see below comment.

On Tue, Aug 25, 2015 at 12:17:20PM +0100, Sudeep Holla wrote:
> On 19/08/15 10:37, Leo Yan wrote:
> >Document the new compatible for Hisilicon Hi6220 mailbox driver.
> >
> >Signed-off-by: Leo Yan 
> >---
> >  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 57 
> > ++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> >
> >diff --git 
> >a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt 
> >b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> >new file mode 100644
> >index 000..3dfb0b0
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
> >@@ -0,0 +1,57 @@
> >+Hisilicon Hi6220 Mailbox Driver
> >+===
> >+
> >+Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
> >+is unidirectional with a maximum message size of 8 words. I/O is
> >+performed using register access (there is no DMA) and the cell
> >+raises an interrupt when messages are received.
> >+
> >+Mailbox Device Node:
> >+
> >+
> >+Required properties:
> >+
> >+- compatible:   Shall be "hisilicon,hi6220-mbox"
> >+- reg:  Contains the mailbox register address range 
> >(base
> >+address and length); the first item is for IPC
> >+registers, the second item is shared buffer for
> >+slots.
> 
> Not sure if the shared buffer needs to be part of the controller binding
> as it's not related to it. It's just agreement between the endpoints of
> this mailbox on particular SoC and IMO has to part of the client binding.

Yes, we need distinguish the buffer is really used for channel's
management or just only used for client. Here "shared buffer" is
used for channels' state machine, mode and raw data with 8 words.

So mailbox driver just read/write raw data according to client's
requirement, client will define their specific format for data
transcation.

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mmc: sunxi: Don't start commands while the card is busy

[Hans de Goede]

Hi,

On 25-08-15 14:05, Ulf Hansson wrote:

On 1 August 2015 at 11:01, Hans de Goede  wrote:

Hi,

On 21-07-15 14:15, Ulf Hansson wrote:


On 10 July 2015 at 17:14, Hans de Goede  wrote:


Some sdio wifi modules have not been working reliable with the sunxi-mmc
host code. This turns out to be caused by it starting new commands while
the card signals that it is still busy processing a previous command.



Which are those commands?



The code were things get stuck when not waiting for the busy signal uses
the following sdio helper functions:

sdio_readb/sdio_writeb
sdio_f0_readb/sdio_f0_writeb
sdio_readw/sdio_writew
sdio_readl/sdio_writel
sdio_readsb
sdio_memcpy_fromio/sdio_memcpy_toio

And directly issues the following command:

 mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
 mmc_cmd.opcode = SD_IO_RW_EXTENDED;
 mmc_cmd.arg = write ? 1<<31 : 0;/* write flag  */
 mmc_cmd.arg |= (fn & 0x7) << 28;/* SDIO func num */
 mmc_cmd.arg |= 1<<27;   /* block mode */
 /* for function 1 the addr will be incremented */
 mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
 mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;

I do not know if the busy wait is necessary for all
of these, but the hack in the android kernel code,
which inserts calls to a wait_card_busy function directly
into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

Does so for all of these.


Or more interesting, which response types do

these commands expect?
I don't think this is a sunxi specific issue, I have seen similar
issues for other host controllers.



Agreed, recent dw-mmc patches address the same issue, also involving
broadcom sdio wifi chips.


I am thinking that perhaps this should be managed by the mmc core
instead of local hacks to sunxi. Potentially we could make the core to
invoke the already existing host_ops->card_busy() callback when
needed...



I'm fine with solving this either way, implementing host_ops->card_busy()
for sunxi is easy, and if the core os modified to call that function
before issue sdio io ops (which seems to be the common thing here),
then that indeed is better then having the sunxi code always do
the busy check.


Okay, so let's make the core to call the ->card_busy() callback and
then it's up to each host driver to implement that callback.


Ok, do you plan to do a patch for this, or do you expect to get
one submitted to you ?

Regards,

Hans



As an optimization, we might consider (in a separate step) to add
MMC_RSP_BUSY to the response type. I realize that would somewhat be a
violation of the spec, but apparently the SDIO spec isn't really clear
on this area.




Within this context, could I ask whether you controller supports IRQ
based HW-busy detection? Or you need polling of the status register?



I'm afraid that we need to poll the status register. I've been unable
to find an irq flag corresponding to this.


Okay, thanks for the info.

[...]

Kind regards
Uffe


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

Re: [PATCH 1/4] mmc: mediatek: Add online-tuning support of EMMC/SD

[Ulf Hansson]

On 17 August 2015 at 14:01, Chaotian Jing  wrote:
> Hi Ulf,
> Thanks, please see my comment:
> On Mon, 2015-08-17 at 13:31 +0200, Ulf Hansson wrote:
>> On 12 August 2015 at 10:24, Chaotian Jing  wrote:
>> > Schedule a workqueue to do tuning when CRC error
>> > Call mmc_hw_reset to re-init card when data timeout
>>
>> Thanks to Adrian Hunter, the mmc core already supports re-tuning for
>> the above scenarios through the mmc_retune_*() APIs.
>>
>> SDHCI driver has already adopted to use that feature, you should do
>> that for the mtk-sd driver as well.
>>
>> Kind regards
>> Uffe
>>
> I also noticed that the mmc core already supports re-tuning, but it is
> not suitable for our host.
> For EMMC, the CMD21 only support HS200/HS400 mode, for SD card, CMD19
> only support SDR50/SDR104 mode, but in our host, even 50Mhz clock
> frequency may also occur CRC error, Cannot find a parameter that can
> cover all SD cards which running at 50Mhz, even 25Mhz, will occur CRC
> error for stress test, DDR50 mode is worse.

I don't follow. You may run for example HS200 in lower speed, nothing
will prevent tuning and re-tuning from happen for these scenarios.

Or you are talking about other speed modes than HS200/400 and
SDR50/104? If so, which speed modes are these?

BTW, there are currently a patch being discussed which is about adding
tuning for DDR mode. Please have look.
http://www.spinics.net/lists/arm-kernel/msg438434.html

Regarding re-tuning on CRC errors, that's already supported by the mmc
core. More precisely when a host driver returns -EILSEQ for a request.

> By the way,there are too many tune parameters need try for response,
> read data, write crc status CRC error, these parameters are
> multidimensional, it is hard to find a best parameter, and, try
> thousands of parameters will take long time.

As I see, it's your responsibility from the host driver to propagate
the proper error code towards the mmc core. If you encounter an error
that you want to trigger a retune, just return -EILSEQ.

Moreover, if you see a need to extend the tuning/re-tuning support in
the mmc core to suit your need - I am definitely open to look into
that. More importantly, I don't want to see host specific hacks trying
to deal with this.

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

Re: [RFC PATCH v1] mmc: sdhci-of-arasan: Add the support for sdhci-5.1

[Ulf Hansson]

On 18 August 2015 at 01:40, Shawn Lin  wrote:
> Hi Ulf,
>
>
> On 2015/8/12 13:07, Michal Simek wrote:
>>
>> +linux-mmc
>>
>> On 08/11/2015 04:53 PM, Michal Simek wrote:
>>>
>>> On 08/11/2015 09:46 AM, Shawn Lin wrote:

 This patch adds the compatible string in sdhci-of-arasan.c to
 support sdhci-arasan5.1 version of controller. No documented
 controller IP version is found in the TRM, so we use ths version
 of command queueing engine integrated into this controller by arasan
 to specify our controller.

 Signed-off-by: Shawn Lin 

 ---

 Changes in v1:
 - Remove redundant SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN for "arasan,
 sdhci-5.1" since
SDHCI will check "host->max_clk == 0" and let driver get it from
 host->ops->get_max_clock.

   Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 2 +-
   drivers/mmc/host/sdhci-of-arasan.c | 1 +
   2 files changed, 2 insertions(+), 1 deletion(-)

 diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
 b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
 index 7e94903..da541c3 100644
 --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
 +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
 @@ -9,7 +9,7 @@ Device Tree Bindings for the Arasan SDHCI Controller

   Required Properties:
 - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
 -'arasan,sdhci-4.9a'
 +'arasan,sdhci-4.9a' or 'arasan,sdhci-5.1'
 - reg: From mmc bindings: Register location and length.
 - clocks: From clock bindings: Handles to clock inputs.
 - clock-names: From clock bindings: Tuple including "clk_xin" and
 "clk_ahb"
 diff --git a/drivers/mmc/host/sdhci-of-arasan.c
 b/drivers/mmc/host/sdhci-of-arasan.c
 index ef5a7d2..75379cb 100644
 --- a/drivers/mmc/host/sdhci-of-arasan.c
 +++ b/drivers/mmc/host/sdhci-of-arasan.c
 @@ -217,6 +217,7 @@ static int sdhci_arasan_remove(struct
 platform_device *pdev)

   static const struct of_device_id sdhci_arasan_of_match[] = {
 { .compatible = "arasan,sdhci-8.9a" },
 +   { .compatible = "arasan,sdhci-5.1" },
 { .compatible = "arasan,sdhci-4.9a" },
 { }
   };

>>>
>>> Acked-by: Michal Simek 
>
>
> Could you pull this patch into your repository?
>

Thanks, applied for next!

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

[PATCH v4] qcom: ipq4019: Add basic board/dts support for IPQ4019 SoC

[Varadarajan Narayanan]

Add initial dts files and SoC support for IPQ4019

Signed-off-by: Varadarajan Narayanan 
---
Changes in v2:
  - Added devicetree bindings documentation

Changes in v3:
  - Split 'gcnt' into a separate patch
  - Added the new entries to Makefiles, Kconfig & board.c in sorted order
  - Used "qcom,ipq40xx" instead of "qcom,ipq40xx-r3pc" in board.c

Changes in v4:
  - Renamed IPQ40xx as IPQ4019
  - Moved kconfig and defconfig enablement to separate patch
  - Dropped Documentation/devicetree/bindings/qcom.txt

 arch/arm/boot/dts/Makefile  |  1 +
 arch/arm/boot/dts/qcom-ipq4019-r3pc.dts | 34 ++
 arch/arm/boot/dts/qcom-ipq4019.dtsi | 81 +
 3 files changed, 116 insertions(+)
 create mode 100644 arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
 create mode 100644 arch/arm/boot/dts/qcom-ipq4019.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 246473a..ca603f3 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -474,6 +474,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
qcom-apq8074-dragonboard.dtb \
qcom-apq8084-ifc6540.dtb \
qcom-apq8084-mtp.dtb \
+   qcom-ipq4019-r3pc.dtb \
qcom-ipq8064-ap148.dtb \
qcom-msm8660-surf.dtb \
qcom-msm8960-cdp.dtb \
diff --git a/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts 
b/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
new file mode 100644
index 000..e3855d7
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-ipq4019-r3pc.dts
@@ -0,0 +1,34 @@
+/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "qcom-ipq4019.dtsi"
+
+/ {
+   model = "Qualcomm Technologies, Inc. IPQ4019 R3PC";
+   compatible = "qcom,ipq4019-r3pc", "qcom,ipq4019";
+
+   memory {
+   device_type = "memory";
+   reg = <0x8000 0x2000>; /* 512MB */
+   };
+
+   chosen {
+   bootargs = "root=/dev/ram rw init=/init 
initrd=0x8200,0x000E2246";
+   stdout-path = "/serial@78b:115200";
+   };
+
+   soc {
+   serial@78b {
+   status = "ok";
+   };
+   };
+};
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi 
b/arch/arm/boot/dts/qcom-ipq4019.dtsi
new file mode 100644
index 000..1be0322
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+/dts-v1/;
+
+#include "skeleton.dtsi"
+
+/ {
+   model = "Qualcomm Technologies, Inc. IPQ4019";
+   compatible = "qcom,ipq4019";
+   interrupt-parent = <&intc>;
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x0>;
+   };
+
+   cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x1>;
+   };
+
+   cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x2>;
+   };
+
+   cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x3>;
+   };
+   };
+
+   soc {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+   compatible = "simple-bus";
+
+   intc: interrupt-controller@b00 {
+   compatible = "qcom,msm-qgic2";
+   interrupt-controller;
+   #interrupt-cells = <3>;
+   reg = <0x0b00 0x1000>,
+   <0x0b002000 0x1000>;
+   };
+
+   serial@78b {
+   compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
+   

Re: [PATCH v2 0/3] introduce driver for the Atmel SDMMC

[Ulf Hansson]

On 29 July 2015 at 16:22, Ludovic Desroches  wrote:
> Hi,
>
> This set of patches introduce a driver for the new Atmel SDMMC device avaible
> on SAMA5D2 SoC.
>
> There is also a resend of an old patch which has not been taken. Ulf asked
> for some reviews since it could impact all sdhci devices but nobody did it...
> This patch is not necessary for patch 2/3. It only fixes a special use case. 
> If
> there are objections about it, drop it, I don't want to delay the Atmel SDMMC
> driver inclusion only for this patch.
>
> Changes:
> - from v1:
>   - update license
>
> Ludovic Desroches (3):
>   mmc: sdhci: switch from programmable clock mode to divided one if
> needed
>   mmc: sdhci-of-at91: introduce driver for the Atmel SDMMC
>   MAINTAINERS: add entry for Atmel sdhci-of-at91 driver
>
>  .../devicetree/bindings/mmc/sdhci-atmel.txt|  21 +++
>  MAINTAINERS|   6 +
>  drivers/mmc/host/Kconfig   |   8 +
>  drivers/mmc/host/Makefile  |   1 +
>  drivers/mmc/host/sdhci-of-at91.c   | 192 
> +
>  drivers/mmc/host/sdhci.c   |  29 +++-
>  6 files changed, 248 insertions(+), 9 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-atmel.txt
>  create mode 100644 drivers/mmc/host/sdhci-of-at91.c
>
> --
> 2.5.0
>

Thanks, applied all three patches for next!

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

Re: [PATCH v6 0/6] mmc: imx: a few fixes and new feature

[Ulf Hansson]

On 11 August 2015 at 13:38, Haibo Chen  wrote:
> Changes for v6:
> -remove duplicate code in esdhc_set_uhs_signaling().
> -fix a typo for patch-2.
> -make commit log of patch-3 more specific.
>
> Haibo Chen (6):
>   mmc: sdhci-esdhc-imx: add imx7d support and support HS400
>   mmc: sdhci-esdhc-imx: add tuning-step setting support
>   mmc: sdhci-esdhc-imx: add imx7d support in bingding doc
>   ARM: dts: imx7d-sdb: add eMMC5.0 support
>   mmc: sdhci-esdhc-imx: set back the burst_length_enable bit to 1
>   mmc: sdhci-esdhc-imx: change default watermark level and burst length
>
>  .../devicetree/bindings/mmc/fsl-imx-esdhc.txt  |   6 ++
>  arch/arm/boot/dts/imx7d-sdb.dts|  13 +++
>  drivers/mmc/host/sdhci-esdhc-imx.c | 114 
> -
>  include/linux/platform_data/mmc-esdhc-imx.h|   1 +
>  4 files changed, 130 insertions(+), 4 deletions(-)
>
> --
> 1.9.1
>

I have applied this for next - except patch4 as it needs and ack from
the IMX SOC maintainer, or it may be better to take that patch through
ARM soc...

Regarding patch3, which document the new bindings and the compatible
string, I moved this to be the first patch and fixed the spelling of
the commit message header.

Thanks!

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

Re: [PATCH v2] mmc: sunxi: Don't start commands while the card is busy

[Ulf Hansson]

On 1 August 2015 at 11:01, Hans de Goede  wrote:
> Hi,
>
> On 21-07-15 14:15, Ulf Hansson wrote:
>>
>> On 10 July 2015 at 17:14, Hans de Goede  wrote:
>>>
>>> Some sdio wifi modules have not been working reliable with the sunxi-mmc
>>> host code. This turns out to be caused by it starting new commands while
>>> the card signals that it is still busy processing a previous command.
>>
>>
>> Which are those commands?
>
>
> The code were things get stuck when not waiting for the busy signal uses
> the following sdio helper functions:
>
> sdio_readb/sdio_writeb
> sdio_f0_readb/sdio_f0_writeb
> sdio_readw/sdio_writew
> sdio_readl/sdio_writel
> sdio_readsb
> sdio_memcpy_fromio/sdio_memcpy_toio
>
> And directly issues the following command:
>
> mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
> mmc_cmd.opcode = SD_IO_RW_EXTENDED;
> mmc_cmd.arg = write ? 1<<31 : 0;/* write flag  */
> mmc_cmd.arg |= (fn & 0x7) << 28;/* SDIO func num */
> mmc_cmd.arg |= 1<<27;   /* block mode */
> /* for function 1 the addr will be incremented */
> mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
> mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;
>
> I do not know if the busy wait is necessary for all
> of these, but the hack in the android kernel code,
> which inserts calls to a wait_card_busy function directly
> into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>
> Does so for all of these.
>
>> Or more interesting, which response types do
>>
>> these commands expect?
>> I don't think this is a sunxi specific issue, I have seen similar
>> issues for other host controllers.
>
>
> Agreed, recent dw-mmc patches address the same issue, also involving
> broadcom sdio wifi chips.
>
>> I am thinking that perhaps this should be managed by the mmc core
>> instead of local hacks to sunxi. Potentially we could make the core to
>> invoke the already existing host_ops->card_busy() callback when
>> needed...
>
>
> I'm fine with solving this either way, implementing host_ops->card_busy()
> for sunxi is easy, and if the core os modified to call that function
> before issue sdio io ops (which seems to be the common thing here),
> then that indeed is better then having the sunxi code always do
> the busy check.

Okay, so let's make the core to call the ->card_busy() callback and
then it's up to each host driver to implement that callback.

As an optimization, we might consider (in a separate step) to add
MMC_RSP_BUSY to the response type. I realize that would somewhat be a
violation of the spec, but apparently the SDIO spec isn't really clear
on this area.

>
>> Within this context, could I ask whether you controller supports IRQ
>> based HW-busy detection? Or you need polling of the status register?
>
>
> I'm afraid that we need to poll the status register. I've been unable
> to find an irq flag corresponding to this.

Okay, thanks for the info.

[...]

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

Re: Re: [v4,1/6] mmc: dt: add DT binding for big endian controller

[Ulf Hansson]

On 25 August 2015 at 12:27, Lu Y.B.  wrote:
> Please see my comments below.
>
>> -Original Message-
>> From: Li Yang-Leo-R58472
>> Sent: Tuesday, August 25, 2015 6:56 AM
>> To: Lu Yangbo-B47093
>> Cc: linux-...@vger.kernel.org; ulf.hans...@linaro.org;
>> devicetree@vger.kernel.org
>> Subject: Re: [v4,1/6] mmc: dt: add DT binding for big endian controller
>>
>> > Add specification of 'big-endian' property. if the controller is big
>> > endian mode, specify this property in device tree node.
>> > The default endian mode is little endian.
>>
>> You shouldn't make the default endian mode to be little endian because in
>> the original binding without this new property we are actually expecting
>> big-endian hardware.  Making little-endian the default is breaking the
>> compatibility with the original binding.
>>
>> Regards,
>> Leo
>
> Thanks a lot, Leo.
>
> Hi Uffe,
>
> Since it doesn't make sense to extend the common MMC DT parser, 
> mmc_of_parse(), to fetch the endian mode,
> I'd like to move endian fetching code and all LE/BE accessors definition to 
> eSDHC driver.

Please them parts of the sdhci generic code, as library functions.

>
> And the default endian would be set to big-endian again for eSDHC.

That's okay, just make sure it's properly documented in the DT binding
doc for esdhc.

>
> Use function pointers that are assigned in probe according endian mode to 
> avoid checking endian every time.
> I'd also like to separate eMMC DDR mode patch from this patchset. Would send 
> it with other UHS-1 speed mode patches together.

Makes perfect sense!

So in your sdhci host specific driver, you will find out the endian
type and make use of the corresponding library functions gets assigned
in the host ops which is provided to sdhci_pltfm_init.

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

Re: [PATCH v3 6/8] scsi: ufs: make the UFS variant a platform device

[ygardi]

Reviewed-by: Akinobu Mita 


> This change turns the UFS variant (SCSI_UFS_QCOM) into a UFS
> a platform device.
> In order to do so a few additional changes are required:
> 1. The ufshcd-pltfrm is no longer serves as a platform device.
>Now it only serves as a group of platform APIs such as PM APIs
>(runtime suspend/resume, system suspend/resume etc), parsers of
>clocks, regulators and pm_levels from DT.
> 2. What used to be the old platform "probe" is now "only"
>a pltfrm_init() routine, that does exactly the same, but only
>being called by the new probe function of the UFS variant.
>
> Signed-off-by: Yaniv Gardi 
>
> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt  |  2 +-
>  drivers/scsi/ufs/ufs-qcom.c| 78
> +-
>  drivers/scsi/ufs/ufshcd-pltfrm.c   | 92
> ++
>  drivers/scsi/ufs/ufshcd-pltfrm.h   | 41 ++
>  drivers/scsi/ufs/ufshcd.c  | 10 +++
>  drivers/scsi/ufs/ufshcd.h  |  1 +
>  6 files changed, 152 insertions(+), 72 deletions(-)
>  create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.h
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index 5357919..b39e765 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -4,7 +4,7 @@ UFSHC nodes are defined to describe on-chip UFS host
> controllers.
>  Each UFS controller instance should have its own node.
>
>  Required properties:
> -- compatible: compatible list, contains "jedec,ufs-1.1"
> +- compatible: compatible list, contains "jedec,ufs-1.1" or
> "qcom,ufshc"
>  - interrupts: 
>  - reg   : 
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 329ac84..8027435 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -19,6 +19,7 @@
>
>  #include 
>  #include "ufshcd.h"
> +#include "ufshcd-pltfrm.h"
>  #include "unipro.h"
>  #include "ufs-qcom.h"
>  #include "ufshci.h"
> @@ -1036,7 +1037,7 @@ void ufs_qcom_clk_scale_notify(struct ufs_hba *hba)
>   * The variant operations configure the necessary controller and PHY
>   * handshake during initialization.
>   */
> -static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
> +static struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
>   .name   = "qcom",
>   .init   = ufs_qcom_init,
>   .exit   = ufs_qcom_exit,
> @@ -1050,4 +1051,79 @@ static const struct ufs_hba_variant_ops
> ufs_hba_qcom_vops = {
>   .resume = ufs_qcom_resume,
>  };
>
> +/**
> + * ufs_qcom_probe - probe routine of the driver
> + * @pdev: pointer to Platform device handle
> + *
> + * Always return 0
> + */
> +static int ufs_qcom_probe(struct platform_device *pdev)
> +{
> + int err;
> + struct device *dev = &pdev->dev;
> + struct ufs_hba *hba;
> +
> + /* Perform generic probe */
> + err = ufshcd_pltfrm_init(pdev, &ufs_hba_qcom_vops);
> + if (err) {
> + dev_err(dev, "ufshcd_pltfrm_init() failed %d\n", err);
> + goto out;
> + }
> +
> + hba = platform_get_drvdata(pdev);
> + if (unlikely(!hba)) {
> + dev_err(dev, "no hba structure after successful probing\n");
> + goto dealloc_host;
> + }
> +
> + return 0;
> +
> +dealloc_host:
> + /* disconnect the bind between the qcom host and the hba */
> + ufshcd_set_variant(hba, NULL);
> + ufshcd_dealloc_host(hba);
> +out:
> + return err;
> +}
> +
> +/**
> + * ufs_qcom_remove - set driver_data of the device to NULL
> + * @pdev: pointer to platform device handle
> + *
> + * Always return 0
> + */
> +static int ufs_qcom_remove(struct platform_device *pdev)
> +{
> + struct ufs_hba *hba =  platform_get_drvdata(pdev);
> +
> + pm_runtime_get_sync(&(pdev)->dev);
> + ufshcd_remove(hba);
> + return 0;
> +}
> +
> +static const struct of_device_id ufs_qcom_of_match[] = {
> + { .compatible = "qcom,ufshc"},
> + {},
> +};
> +
> +static const struct dev_pm_ops ufs_qcom_pm_ops = {
> + .suspend= ufshcd_pltfrm_suspend,
> + .resume = ufshcd_pltfrm_resume,
> + .runtime_suspend = ufshcd_pltfrm_runtime_suspend,
> + .runtime_resume  = ufshcd_pltfrm_runtime_resume,
> + .runtime_idle= ufshcd_pltfrm_runtime_idle,
> +};
> +
> +static struct platform_driver ufs_qcom_pltform = {
> + .probe  = ufs_qcom_probe,
> + .remove = ufs_qcom_remove,
> + .shutdown = ufshcd_pltfrm_shutdown,
> + .driver = {
> + .name   = "ufshcd-qcom",
> + .pm = &ufs_qcom_pm_ops,
> + .of_match_table = of_match_ptr(ufs_qcom_of_match),
> + },
> +};
> +module_platform_driver(ufs_qcom_pltform);
> +
>  MODULE_LICENSE("GPL v2");
> diff 

Re: [PATCH v2 00/16] omap_hsmmc: regulator usage cleanup and fixes

[Ulf Hansson]

On 3 August 2015 at 14:26, Kishon Vijay Abraham I  wrote:
> Changes from v1:
> *) return on -EPROBE_DEFER and other fatal errors. (Don't return only
>if the return value is -ENODEV)
> *) Remove the beagle x15 dts patch. It can be part of a different
>series.
> *) Avoid using regulator_is_enabled for vqmmc since if the regulator
>is shared and the other users are not using regulator_is_enabled
>then there can be unbalanced regulator_enable/regulator_disable
>
> This patch series does the following
> *) Uses devm_regulator_get_optional() for vmmc and then removes the
>CONFIG_REGULATOR check altogether.
> *) return on -EPROBE_DEFER and any other fatal errors
> *) enable/disable vmmc_aux regulator based on prior state
>
> I've pushed this patch series to
> git://git.ti.com/linux-phy/linux-phy.git mmc_regulator_cleanup_fixes_v2
>
> Please note the branch also has the pbias fixes [1] & [2].
> [1] -> https://lkml.org/lkml/2015/7/27/358
> [2] -> https://lkml.org/lkml/2015/7/27/391
>
> This series is in preparation for implementing the voltage switch
> sequence so that UHS cards can be supported.
>
> Did basic read/write test in J6, J6 Eco, Beagle-x15, AM437x EVM,
> Beaglebone black, OMAP5 uEVM and OMAP4 PANDA.
>
> Kishon Vijay Abraham I (15):
>   mmc: host: omap_hsmmc: use devm_regulator_get_optional() for vmmc
>   mmc: host: omap_hsmmc: return on fatal errors from omap_hsmmc_reg_get
>   mmc: host: omap_hsmmc: cleanup omap_hsmmc_reg_get()
>   mmc: host: omap_hsmmc: use the ocrmask provided by the vmmc regulator
>   mmc: host: omap_hsmmc: use mmc_host's vmmc and vqmmc
>   mmc: host: omap_hsmmc: remove unnecessary pbias set_voltage
>   mmc: host: omap_hsmmc: return error if any of the regulator APIs fail
>   mmc: host: omap_hsmmc: add separate functions for enable/disable
> supply
>   mmc: host: omap_hsmmc: add separate function to set pbias
>   mmc: host: omap_hsmmc: avoid pbias regulator enable on power off
>   mmc: host: omap_hsmmc: don't use ->set_power to set initial regulator
> state
>   mmc: host: omap_hsmmc: enable/disable vmmc_aux regulator based on
> previous state
>   mmc: host: omap_hsmmc: use regulator_is_enabled to find pbias status
>   mmc: host: omap_hsmmc: use ios->vdd for setting vmmc voltage
>   mmc: host: omap_hsmmc: remove CONFIG_REGULATOR check
>
> Roger Quadros (1):
>   mmc: host: omap_hsmmc: use "mmc_of_parse_voltage" to get ocr_avail
>
>  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt  |2 +
>  drivers/mmc/host/omap_hsmmc.c  |  340 
> +---
>  2 files changed, 224 insertions(+), 118 deletions(-)
>
> --
> 1.7.9.5
>

I tried to apply this on top of my next branch, but it didn't work.
Could you please rebase and send a v3!?
While you do that, please add Tony's tested-by tag as well.

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

Re: [PATCH v8 1/2] mfd: devicetree: add bindings for Atmel Flexcom

[Cyrille Pitchen]

Hi all,

any feedback on this kind of DT bindings?
Is it a proper way of using the "ranges" property?

Best regards,

Cyrille

Le 24/07/2015 17:08, Lee Jones a écrit :
> DT chaps,
> 
> Please can you take a look at this binding.
> 
> In particular the use of the 'ranges' property to store device 'mode'.
> 
>> This patch documents the DT bindings for the Atmel Flexcom which will be
>> introduced by sama5d2x SoCs. These bindings will be used by the actual
>> Flexcom driver to be sent in another patch.
>>
>> Signed-off-by: Cyrille Pitchen 
>> Acked-by: Nicolas Ferre 
>> ---
>>  .../devicetree/bindings/mfd/atmel-flexcom.txt  | 68 
>> ++
>>  1 file changed, 68 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt 
>> b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>> new file mode 100644
>> index ..588d527dbfa7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>> @@ -0,0 +1,68 @@
>> +* Device tree bindings for Atmel Flexcom (Flexible Serial Communication 
>> Unit)
>> +
>> +The Atmel Flexcom is just a wrapper which embeds a SPI controller, an I2C
>> +controller and an USART. Only one function can be used at a time and is 
>> chosen
>> +at boot time according to the device tree.
>> +
>> +Required properties:
>> +- compatible:   Should be "atmel,sama5d2-flexcom"
>> +- reg:  Should be the pair (offset, size) for the 
>> Flexcom
>> +dedicated I/O registers (without USART, TWI or SPI
>> +registers).
>> +- clocks:   Should be the Flexcom peripheral clock from PMC.
>> +- #address-cells:   Should be <2>
>> +- #size-cells:  Should be <1>
>> +- ranges:   Should be a list of ranges.
>> +One range per peripheral wrapped by the Flexcom. So each
>> +range is a triplet (child_addr, parent_addr, size). The
>> +first u32 of "child_addr" is the value to be set in the
>> +Operating Mode bitfield of the Flexcom Mode Register.
>> +Then "parent_addr" stores the base address of the
>> +corresponding peripheral in the system memory. Finally,
>> +"size" if the size of the memory region of this
>> +peripheral.
>> +
>> +Required child:
>> +A single available child for the serial controller to enable.
>> +
>> +Required properties of this child:
>> +- reg:  Should be a pair (child_addr, size) with 
>> child_addr
>> +matching one of the parent ranges.
>> +- clocks:   Should be the very same phandle as for the parent's one.
>> +
>> +Other properties remain unchanged. See documentation of the respective 
>> device:
>> +- ../serial/atmel-usart.txt
>> +- ../spi/spi_atmel.txt
>> +- ../i2c/i2c-at91.txt
>> +
>> +Example:
>> +
>> +flexcom@f8034000 {
>> +compatible = "atmel,sama5d2-flexcom";
>> +reg = <0xf8034000 0x200>;
>> +clocks = <&flx0_clk>;
>> +#address-cells = <2>;
>> +#size-cells = <1>;
>> +ranges = <1 0 0xf8034200 0x200  /* opmode 1: USART */
>> +  2 0 0xf8034400 0x200  /* opmode 2: SPI */
>> +  3 0 0xf8034600 0x200>;/* opmode 3: I2C */
>> +
>> +spi@2,0 {
>> +compatible = "atmel,at91rm9200-spi";
>> +reg = <2 0 0x200>;
>> +interrupts = <19 IRQ_TYPE_LEVEL_HIGH 7>;
>> +pinctrl-names = "default";
>> +pinctrl-0 = <&pinctrl_flx0_default>;
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +clocks = <&flx0_clk>;
>> +clock-names = "spi_clk";
>> +atmel,fifo-size = <32>;
>> +
>> +mtd_dataflash@0 {
>> +compatible = "atmel,at25f512b";
>> +reg = <0>;
>> +spi-max-frequency = <2000>;
>> +};
>> +};
>> +};
> 

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

Re: [PATCH v5 2/5] pci:host: Add Altera PCIe host controller driver

[Marc Zyngier]

On 25/08/15 10:35, Ley Foon Tan wrote:
> This patch adds the Altera PCIe host controller driver.
> 
> Signed-off-by: Ley Foon Tan 
> ---
>  drivers/pci/host/Kconfig   |   7 +
>  drivers/pci/host/Makefile  |   1 +
>  drivers/pci/host/pcie-altera.c | 588 
> +
>  3 files changed, 596 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-altera.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 675c2d1..4b4754a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -145,4 +145,11 @@ config PCIE_IPROC_BCMA
> Say Y here if you want to use the Broadcom iProc PCIe controller
> through the BCMA bus interface
>  
> +config PCIE_ALTERA
> + tristate "Altera PCIe controller"
> + depends on ARCH_SOCFPGA
> + help
> +   Say Y here if you want to enable PCIe controller support for Altera
> +   SoCFPGA family of SoCs.
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 140d66f..6954f76 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> +obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
> new file mode 100644
> index 000..c168ec7
> --- /dev/null
> +++ b/drivers/pci/host/pcie-altera.c
> @@ -0,0 +1,588 @@
> +/*
> + * Copyright Altera Corporation (C) 2013-2015. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define A2P_ADDR_MAP_LO0 0x1000
> +#define A2P_ADDR_MAP_HI0 0x1004
> +#define RP_TX_REG0   0x2000
> +#define RP_TX_REG1   0x2004
> +#define RP_TX_CNTRL  0x2008
> +#define RP_TX_EOP0x2
> +#define RP_TX_SOP0x1
> +#define RP_RXCPL_STATUS  0x2010
> +#define RP_RXCPL_EOP 0x2
> +#define RP_RXCPL_SOP 0x1
> +#define RP_RXCPL_REG00x2014
> +#define RP_RXCPL_REG10x2018
> +#define P2A_INT_STATUS   0x3060
> +#define P2A_INT_STS_ALL  0xF
> +#define P2A_INT_ENABLE   0x3070
> +#define P2A_INT_ENA_ALL  0xF
> +#define RP_LTSSM 0x3C64
> +#define LTSSM_L0 0xF
> +
> +/* TLP configuration type 0 and 1 */
> +#define TLP_FMTTYPE_CFGRD0   0x04/* Configuration Read Type 0 */
> +#define TLP_FMTTYPE_CFGWR0   0x44/* Configuration Write Type 0 */
> +#define TLP_FMTTYPE_CFGRD1   0x05/* Configuration Read Type 1 */
> +#define TLP_FMTTYPE_CFGWR1   0x45/* Configuration Write Type 1 */
> +#define TLP_PAYLOAD_SIZE 0x01
> +#define TLP_READ_TAG 0x1D
> +#define TLP_WRITE_TAG0x10
> +#define TLP_CFG_DW0(fmttype) (((fmttype) << 24) | TLP_PAYLOAD_SIZE)
> +#define TLP_CFG_DW1(reqid, tag)  (((reqid) << 16) | (tag << 8) | 
> 0xF)
> +#define TLP_CFG_DW2(bus, devfn, offset)  \
> + (((bus) << 24) | ((devfn) << 16) | (offset))
> +#define TLP_REQ_ID(bus, devfn)   (((bus) << 8) | (devfn))
> +#define TLP_COMPL_STATUS(hdr)(((hdr) & 0xE0) >> 13)
> +#define TLP_HDR_SIZE 3
> +#define TLP_LOOP 10
> +
> +#define INTX_NUM 4
> +
> +/* Address translation table entry size */
> +#define ATT_ENTRY_SIZE   8
> +
> +#define DWORD_MASK   3
> +
> +struct altera_pcie {
> + struct platform_device  *pdev;
> + void __iomem*cra_base;
> + int irq;
> + u8  root_bus_nr;
> + struct irq_domain   *irq_domain;
> + struct resource bus_range;
> + struct list_headresources;
> +};
> +
> +struct tlp_rp_regpair_t {
> + u32 ctrl;
> + u32 reg0;
> + u32 reg1;
> +};
> +
> +static void altera

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Sudeep Holla]

On 19/08/15 10:37, Leo Yan wrote:

On Hi6220, below memory regions in DDR have specific purpose:

   0x05e0, - 0x05ef,: For MCU firmware using at runtime;
   0x0740,f000 - 0x0740,: For MCU firmware's section;
   0x06df,f000 - 0x06df,: For mailbox message data.



Unless I am reading the DTS file completely wrong, I don't think the 
above memory regions are in DDR as per the memory node.



This patch reserves these memory regions and add device node for
mailbox in dts.

Signed-off-by: Leo Yan 
---
  arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 20 +---
  arch/arm64/boot/dts/hisilicon/hi6220.dtsi  |  8 
  2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts 
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index e36a539..d5470d3 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -7,9 +7,6 @@

  /dts-v1/;

-/*Reserved 1MB memory for MCU*/
-/memreserve/ 0x05e0 0x0010;
-
  #include "hi6220.dtsi"

  / {
@@ -28,4 +25,21 @@
device_type = "memory";
reg = <0x0 0x0 0x0 0x4000>;
};


I have no access to the spec, but I read this as 1GB RAM @0x0
Unless this entry is completely wrong, what your commit log claims is
incorrect. If this entry is wrong I wonder how is it booting with this
DT then.


+
+   reserved-memory {
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   mcu-buf@05e0 {
+   no-map;
+   reg = <0x0 0x05e0 0x0 0x0010>,/* MCU 
firmware buffer */
+ <0x0 0x0740f000 0x0 0x1000>;/* MCU 
firmware section */


So I don't see how can this be part of DDR ? Or at-least part of DDR
that's mapped by kernel ?

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

Re: [PATCH v5 3/5] pci: altera: Add Altera PCIe MSI driver

[Marc Zyngier]

On 25/08/15 10:35, Ley Foon Tan wrote:
> This patch adds Altera PCIe MSI driver. This soft IP supports configurable
> number of vectors, which is a dts parameter.
> 
> Signed-off-by: Ley Foon Tan 
> ---
>  drivers/pci/host/Kconfig   |   8 +
>  drivers/pci/host/Makefile  |   1 +
>  drivers/pci/host/pcie-altera-msi.c | 322 
> +
>  3 files changed, 331 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-altera-msi.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 4b4754a..d28cc6d 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -152,4 +152,12 @@ config PCIE_ALTERA
> Say Y here if you want to enable PCIe controller support for Altera
> SoCFPGA family of SoCs.
>  
> +config PCIE_ALTERA_MSI
> + bool "Altera PCIe MSI feature"
> + depends on PCI_MSI
> + select PCI_MSI_IRQ_DOMAIN
> + help
> +   Say Y here if you want PCIe MSI support for the Altera SocFPGA SoC.
> +   This MSI driver supports Altera MSI to GIC controller IP.
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 6954f76..6c4913d 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> +obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> diff --git a/drivers/pci/host/pcie-altera-msi.c 
> b/drivers/pci/host/pcie-altera-msi.c
> new file mode 100644
> index 000..d3ad96f
> --- /dev/null
> +++ b/drivers/pci/host/pcie-altera-msi.c
> @@ -0,0 +1,322 @@
> +/*
> + * Copyright Altera Corporation (C) 2013-2015. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MSI_STATUS   0x0
> +#define MSI_ERROR0x4
> +#define MSI_INTMASK  0x8
> +
> +#define MAX_MSI_VECTORS  32
> +struct altera_msi {
> + DECLARE_BITMAP(used, MAX_MSI_VECTORS);
> + struct mutexlock;   /* proctect used variable */
> + struct platform_device  *pdev;
> + struct irq_domain   *msi_domain;
> + struct irq_domain   *inner_domain;
> + void __iomem*csr_base;
> + void __iomem*vector_base;
> + phys_addr_t vector_phy;
> + u32 num_of_vectors;
> + int irq;
> +};
> +
> +static inline void msi_writel(struct altera_msi *msi, u32 value, u32 reg)
> +{
> + writel_relaxed(value, msi->csr_base + reg);
> +}
> +
> +static inline u32 msi_readl(struct altera_msi *msi, u32 reg)
> +{
> + return readl_relaxed(msi->csr_base + reg);
> +}
> +
> +static void altera_msi_isr(unsigned int irq, struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct altera_msi *msi;
> + unsigned long status;
> + u32 num_of_vectors;
> + u32 bit;
> + u32 virq;
> +
> + chained_irq_enter(chip, desc);
> + msi = irq_desc_get_handler_data(desc);
> + num_of_vectors = msi->num_of_vectors;
> +
> + do {
> + status = msi_readl(msi, MSI_STATUS);
> + if (!status)
> + break;
> +
> + do {
> + bit = find_first_bit(&status, num_of_vectors);
> + /* Dummy read from vector to clear the interrupt */
> + readl_relaxed(msi->vector_base + (bit * sizeof(u32)));
> +
> + virq = irq_find_mapping(msi->inner_domain, bit);
> + if (virq && test_bit(bit, msi->used))

I still wonder under which circumstances you can obtain a valid virq and
yet not find the bit in the bitmap. Unless you have a scenario when that
can happen, I suggest you remove that test.

> + generic_handle_irq(virq);
> + else
> + dev_err(&msi->pdev->dev, "unexpected MSI\n");
> +
> + /* Clear the bit from status and repeat without reading
> +  * again status register. */
> + __clear_bi

Re: [PATCH linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller

[Cyrille Pitchen]

Hi!

Le 25/08/2015 03:44, Bean Huo 霍斌斌 (beanhuo) a écrit :
> 
>> +nor->read_reg = atmel_qspi_read_reg;
>> +nor->write_reg = atmel_qspi_write_reg;
>> +nor->read = atmel_qspi_read;
>> +nor->write = atmel_qspi_write;
>> +nor->erase = atmel_qspi_erase;
>> +nor->set_protocol = atmel_qspi_set_protocol;
> 
> This is very good, the structure of spi_nor should add a hook function to 
> notify spi controller 
> That spi nor transfer protocol already changed.
> 
>> +
>> +if (of_modalias_node(child, modalias, sizeof(modalias)) < 0) {
>> +err = -ENODEV;
>> +goto release_channel;
>> +}
>> +
>> +err = of_property_read_u32(child, "spi-max-frequency", &aq->clk_rate);
>> +if (err < 0)
>> +goto release_channel;
>> +
>> +err = atmel_qspi_init(aq);
>> +if (err)
>> +goto release_channel;
>> +
>> +nor->dev->of_node = child;
>> +err = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>>  goto release_channel;
>> +
> 
> 
> ...
> 
>> static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)  {
>   return mtd->priv;
>> @@ -944,6 +960,11 @@ static int micron_quad_enable(struct spi_nor *nor)
>>  return ret;
>>  }
>>
>> +/* switch protocol to Quad CMD 4-4-4 */
>> +ret = spi_nor_set_protocol(nor, SPI_PROTO_4_4_4);
>> +if (ret)
>> +return ret;
>> +
> 
> This make sense,from spi nor side,once its protocol being changed,
> Mtd layer must notify this status to spi nor controller immediately,
> And spi nor controller also should re-adjust its protocol.
> Otherwise, following reading SR operation will fail. 
> 
>>
>>  ret = spi_nor_wait_till_ready(nor);
>>  if (ret)
>>  return ret;
> 
> If my ack has any value in here, feel free to add it.
> 
> Acked-by: Bean Huo 
> 

Since your comments deal with the protocol change, I'll add your ack to the
first patch of the series:
"mtd: spi-nor: notify (Q)SPI controller about protocol change"

Thanks for your review!

Best regards,

Cyrille

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

Re: [PATCH v1 1/3] dt-bindings: mailbox: Document Hi6220 mailbox driver

[Sudeep Holla]

On 19/08/15 10:37, Leo Yan wrote:

Document the new compatible for Hisilicon Hi6220 mailbox driver.

Signed-off-by: Leo Yan 
---
  .../bindings/mailbox/hisilicon,hi6220-mailbox.txt  | 57 ++
  1 file changed, 57 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt

diff --git 
a/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt 
b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
new file mode 100644
index 000..3dfb0b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/hisilicon,hi6220-mailbox.txt
@@ -0,0 +1,57 @@
+Hisilicon Hi6220 Mailbox Driver
+===
+
+Hisilicon Hi6220 mailbox supports up to 32 channels. Each channel
+is unidirectional with a maximum message size of 8 words. I/O is
+performed using register access (there is no DMA) and the cell
+raises an interrupt when messages are received.
+
+Mailbox Device Node:
+
+
+Required properties:
+
+- compatible:  Shall be "hisilicon,hi6220-mbox"
+- reg: Contains the mailbox register address range (base
+   address and length); the first item is for IPC
+   registers, the second item is shared buffer for
+   slots.


Not sure if the shared buffer needs to be part of the controller binding
as it's not related to it. It's just agreement between the endpoints of
this mailbox on particular SoC and IMO has to part of the client binding.

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

Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node

[Mark Rutland]

Hi,

On Tue, Aug 25, 2015 at 09:04:45AM +0100, Haojian Zhuang wrote:
> On Mon, 2015-08-24 at 13:48 +0100, Mark Rutland wrote:
> > > > > > I don't see why you need reserved-memory here, given you're not 
> > > > > > referring to
> > > > > > these regions by phandle anyway.
> > > > > 
> > > > > - Now we have enabled EFI_STUB, so the memory node will be removed in
> > > > >   kernel:
> > > > > efi_entry()
> > > > >   \-> allocate_new_fdt_and_exit_boot()
> > > > > \-> update_fdt();
> > > > > 
> > > > >   Finally in kernel it cannot use memory node to carve out reseved
> > > > >   memory regions.
> > > > > 
> > > > > - On the other hand, DTS's the memory node is to "describes the
> > > > >   physical memory layout for the system"; so it's better to use it 
> > > > > only
> > > > >   to describe the hardware info for memory. We can use reserved-memory
> > > > >   to help manage the memory regions which are reserved from software
> > > > >   perspective.
> > > > 
> > > > The fact that you have no-map means that the memory should not be
> > > > described to the kernel as mappable in the first place. It's wrong to
> > > > place such memory in the memory node, even if listed in reserved-memory.
> > > > 
> > > > If your EFI memory map describes the memory as mappable, it is wrong.
> > > 
> > > When kernel is working, kernel will create its own page table based on
> > > UEFI memory map. Since it's reserved in DTS file as Leo's patch, it'll
> > > be moved to reserved memblock. Why is it wrong?
> > 
> > That is a _Linux_ detail, not a _UEFI_ detail.
> > 
> > Anything which only handles UEFI and knows nothing of reserved-memory
> > (e.g. GRUB) will be broken and/or break the Linux use of the region, as
> > it will happily try to allocate memory in the region (and could even
> > decide to reserve it permanently for its own usage).
> > 
> > If the memory is truly specific to the mailbox, then UEFI needs to know
> > that it is reserved as such. If it is not, then it need not be described
> > in the DT and can be allocated dynamically by the kernel.
> 
> No. We support both UEFI and uboot on hikey. We must reserve these
> mailbox buffer in DT. Otherwise, it's a problem to support uboot.

The same solution applies to both: don't describe the memory in the
first place. Don't place it in a memory node, and don't give it
attributes allowing it to be mapped and used in the UEFI memory map.

> We should only deliver one kernel and one DTB to support both UEFI and
> uboot.

I do not disagree with this point generally, but it's irrelevant to the
point at hand.

> And mailbox driver is already upgraded from beginning. Nobody can say
> that it won't be upgraded again in the future.

This doesn't follow. If you want to pointlessly change things, you will
encounter pain. That's not an argument for hacking a partial solution
into the DT and ignoring the problems this causes.

As I tried to ask earlier, how is the mailbox memory used? Does the
kernel configure the address in the hardware, or is this pre-configured?
Could the kernel choose a region of memory to use dynamically?

> By the way, UEFI is loaded in the upper memory region of hikey. It won't
> break anything in linux kernel.

The code might be there, but UEFI can make use of any memory it knows
about for stacks, pool allocations and so on. It needs to be prevented
from using the mailbox memory.

> > > In the second, UEFI is firmware. When it's stable, nobody should change
> > > it without any reason. These reserved memory are used in mailbox driver.
> > > Look. It's driver, so it could be changed at any time. Why do you want
> > > to UEFI knowing this memory range? Do you hope UEFI to change when
> > > mailbox driver is changed?
> > 
> > It shouldn't need to change if that memory is truly reserved for the
> > sole use of the mailbox. If that's not the case then we have a different
> > issue.
> > 
> > If the memory range to use can be allocated by the driver, then I don't
> > see why it should be described in reserved-memory. It certainly should
> > not require a no-map attribute.
> > 
> > Additionally, the driver needs to ensure that the requisite cache
> > maintenance takes place prior to the use of the memory region given
> > prior agents may have ampped it as cacheable, leaving stale (perhaps
> > dirty) lines in the caches.
> > 
> 
> Since those mailbox buffer is declared as reserved with "no-map"
> property, it means that linux kernel won't create the page table of
> them.

I am well aware of how this works. Please re-read my replies, this is
not the issue I describe.

> The meaning of "no-map" is removing it from memory memblock.
> setup_arch()
>   |
>   ---> efi_init() -- Get memory map information from UEFI
>   |
>   ---> arm64_memblock_init()
>   |  |
>   |  ---> early_init_fdt_scan_reserved_mem()
>   |   Get reserved memory buffer from DT. Split memory
>   |   memblock according to reserved buffer.
>   ---> paging_init()
>