* Re: [PATCH 3/8] ARM: dts: armada-388-clearfog: Utilize new DSA binding
From: Florian Fainelli @ 2017-01-04 17:27 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring, Mark Rutland,
Russell King,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <87shoyeo2j.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 01/04/2017 09:23 AM, Gregory CLEMENT wrote:
> Hi Florian,
>
> On lun., janv. 02 2017, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Utilize the new DSA binding, introduced with commit 8c5ad1d6179d ("net:
>> dsa: Document new binding"). The legacy binding node is kept included, but is
>> marked disabled.
>>
>
> I tested this patch on mvebu/dt (I needed to reduce the context to apply
> the patch due to the changes made by Russell King on this file). I also
> set the status of the old binding to "disable" (instead of "okay").
Yes, that needs fixing, thanks for mentioning that.
>
> It seems to work with the limited test did:
> ifconfig eth1 up
> udhcpc -i lan1
> iperf -c mylaptop
>
> (same for lan4)
>
> However is there a way to be sure that the new binding is used?
The best way is probably to make sure that your switch device appears
parented to the MDIO bus driver under /sys/class/mdio_bus/*mvmdio*.
Alternatively, if you see a message like:
DSA: switch 0 0 parsed
in your dmesg, that would also be indicative of using the new binding
and corresponding code.
Thanks a lot for trying that out!
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/8] ARM: dts: armada-388-clearfog: Utilize new DSA binding
From: Gregory CLEMENT @ 2017-01-04 17:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring, Mark Rutland,
Russell King,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20170102022249.10657-4-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Florian,
On lun., janv. 02 2017, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Utilize the new DSA binding, introduced with commit 8c5ad1d6179d ("net:
> dsa: Document new binding"). The legacy binding node is kept included, but is
> marked disabled.
>
I tested this patch on mvebu/dt (I needed to reduce the context to apply
the patch due to the changes made by Russell King on this file). I also
set the status of the old binding to "disable" (instead of "okay").
It seems to work with the limited test did:
ifconfig eth1 up
udhcpc -i lan1
iperf -c mylaptop
(same for lan4)
However is there a way to be sure that the new binding is used?
Gregory
> Signed-off-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/armada-388-clearfog.dts | 65 +++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/arch/arm/boot/dts/armada-388-clearfog.dts b/arch/arm/boot/dts/armada-388-clearfog.dts
> index 71ce201c903e..35207aa1f4ec 100644
> --- a/arch/arm/boot/dts/armada-388-clearfog.dts
> +++ b/arch/arm/boot/dts/armada-388-clearfog.dts
> @@ -351,6 +351,8 @@
> };
>
> dsa@0 {
> + status = "okay";
> +
> compatible = "marvell,dsa";
> dsa,ethernet = <ð1>;
> dsa,mii-bus = <&mdio>;
> @@ -444,3 +446,66 @@
> status = "disabled";
> };
> };
> +
> +&mdio {
> + status = "okay";
> +
> + switch@4 {
> + compatible = "marvell,mv88e6085";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <4>;
> + pinctrl-0 = <&clearfog_dsa0_clk_pins &clearfog_dsa0_pins>;
> + pinctrl-names = "default";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan5";
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan4";
> + };
> +
> + port@2 {
> + reg = <2>;
> + label = "lan3";
> + };
> +
> + port@3 {
> + reg = <3>;
> + label = "lan2";
> + };
> +
> + port@4 {
> + reg = <4>;
> + label = "lan1";
> + };
> +
> + port@5 {
> + reg = <5>;
> + label = "cpu";
> + ethernet = <ð1>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@6 {
> + /* 88E1512 external phy */
> + reg = <6>;
> + label = "lan6";
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +};
> --
> 2.9.3
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* staging/fbtft: Backward Device Tree compatibility in a drm version
From: Noralf Trønnes @ 2017-01-04 17:14 UTC (permalink / raw)
To: devicetree@vger.kernel.org
Cc: OSUOSL Drivers, DRI Development, Greg Kroah-Hartman
Hi,
I'm working on a drivers/gpu/drm version of drivers/staging/fbtft which
are drivers for tiny, usually SPI connected, displays. Now I'm wondering
if I can be backwards compatible and support Device Trees written for the
fbtft drivers. The main obstacle as I understand it, is the init property
which are values to be written to the controller registers to support a
different panel with the same controller. It even has encoded values for
delays... My understanding is that this is not accepted.
It's in fact a display panel description in the form of register values
and delays.
Here is what a binding doc would look like for one of the fbtft drivers:
* Samsung S6D02A1 Framebuffer Driver
Required properties:
- compatible: Should be "samsung,s6d02a1".
The node for this driver must be a child node of a SPI controller, hence
all mandatory properties described in ../spi/spi-bus.txt must be specified.
Optional properties:
- dc-gpios: D/C pin used with the 4-wire 8-bit data serial interface mode
- reset-gpios: Reset pin
- led-gpios: Backlight control
- width: Panel width in pixels
- height: Panel height in pixels
- rotate: Panel rotation in degrees counter clockwise (0,90,180,270)
- bgr: Panel is wired as BGR565 instead of RGB565
- buswidth: Bit width of the bus, in the case of SPI: 8 (4-wire) or 9
(3-wire) bits.
- txbuflen: Size of transfer buffer. Used for little-big endian
conversion.
- debug: Control debug output to the kernel log
- init: Panel initialization sequence overriding the driver default.
Values OR'ed with:
0x1000000 - Write the following values to this register.
0x2000000 - Delay in milliseconds
Example:
mz61581: mz61581@0{
compatible = "samsung,s6d02a1";
reg = <0>;
spi-max-frequency = <128000000>;
spi-cpol;
spi-cpha;
width = <320>;
height = <480>;
rotate = <270>;
bgr;
buswidth = <8>;
txbuflen = <32768>;
reset-gpios = <&gpio 15 0>;
dc-gpios = <&gpio 25 0>;
led-gpios = <&gpio 18 0>;
init = <0x10000b0 00
0x1000011
0x20000ff
0x10000b3 0x02 0x00 0x00 0x00
0x10000c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43
0x10000c1 0x08 0x16 0x08 0x08
0x10000c4 0x11 0x07 0x03 0x03
0x10000c6 0x00
0x10000c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21
0x08 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00
0x1000035 0x00
0x1000036 0xa0
0x100003a 0x55
0x1000044 0x00 0x01
0x10000d0 0x07 0x07 0x1d 0x03
0x10000d1 0x03 0x30 0x10
0x10000d2 0x03 0x14 0x04
0x1000029
0x100002c>;
/* This is a workaround to make sure the init sequence slows down
and doesn't fail */
debug = <3>;
};
Noralf.
^ permalink raw reply
* RE: [PATCH v4 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Appana Durga Kedareswara Rao @ 2017-01-04 17:00 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland@arm.com, dan.j.williams@intel.com,
vinod.koul@intel.com, michal.simek@xilinx.com, Soren Brinkmann,
moritz.fischer@ettus.com, laurent.pinchart@ideasonboard.com,
luis@debethencourt.com, Jose.Abreu@synopsys.com,
dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20170104152613.hhdptwsyg73ev3dz@rob-hp-laptop>
Hi Rob,
Thanks for the review....
> On Wed, Jan 04, 2017 at 07:05:53PM +0530, Kedareswara rao Appana wrote:
> > When VDMA is configured for more than one frame in the h/w for example
> > h/w is configured for n number of frames and user Submits n number of
> > frames and triggered the DMA using issue_pending API.
> > In the current driver flow we are submitting one frame at a time but
> > we should submit all the n number of frames at one time as the h/w Is
> > configured for n number of frames.
>
> Please fix run-on sentences, capitalization, and word wrapping.
Sure will fix in the next version....
[Snip]
-- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > @@ -66,6 +66,8 @@ Optional child node properties:
> > Optional child node properties for VDMA:
> > - xlnx,genlock-mode: Tells Genlock synchronization is
> > enabled/disabled in hardware.
> > +- xlnx,fstore-config: Tells Whether Frame Store Configuration is
> > + enabled/disabled in hardware.
>
> What's the default (when not present)? That should be the most common case.
> Looks like the code treats this as bool, but that's not clear here. The name is not
> clear what it is doing. Enabling or disabling the feature?
Default value is zero...
When this property is present it tells hardware is configured for frame store configuration.
Will fix the explanation part in the next version like below.
xlnx,fstore-config: Tells hardware is configured for frame store configuration.
Is the above explanation clear???
Regards,
Kedar.
^ permalink raw reply
* [PATCH] scripts/dtc: Update to upstream version 0931cea3ba20
From: Rob Herring @ 2017-01-04 16:57 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA; +Cc: Frank Rowand, Mark Rutland
Sync to upstream dtc commit 0931cea3ba20 ("dtc: fdtdump: check fdt if
not in scanning mode"). In particular, this pulls in dtc overlay
support.
This adds the following commits from upstream:
f88865469b65 dtc: Fix memory leak in character literal parsing
00fbb8696b66 Rename boot_info
1ef86ad2c24f dtc: Clean up /dts-v1/ and /plugin/ handling in grammar
e3c769aa9c16 dtc: Don't always generate __symbols__ for plugins
c96cb3c0169e tests: Don't use -@ on plugin de/recompile tests
66381538ce24 tests: Remove "suppression of fixups" tests
ba765b273f0f tests: Clarify dtc overlay tests
6ea8cd944fcd tests: More thorough tests of libfdt overlay application without dtc
7d8ef6e1db97 tests: Correct fdt handling of overlays without fixups and base trees without symbols
b4dc0ed8b127 tests: Fix double expansion bugs in test code
3ea879dc0c8f tests: Split overlay tests into those with do/don't exercise dtc plugin generation
47b4d66a2f11 tests: Test auto-alias generation on base tree, not overlay
72e1ad811523 tests: Make overlay/plugin tests unconditional
e7b3c3b5951b tests: Add overlay tests
9637e3f772a9 tests: Add check_path test
20f29d8d41f6 dtc: Plugin and fixup support
a2c92cac53f8 dtc: Document the dynamic plugin internals
8f70ac39801d checks: Pass boot_info instead of root node
ea10f953878f libfdt: add missing errors to fdt_strerror()
daa75e8fa594 libfdt: fix fdt_stringlist_search()
e28eff5b787a libfdt: fix fdt_stringlist_count()
ae97c7722840 tests: overlay: Rename the device tree blobs to be more explicit
96162d2bd9cb tests: overlay: Add test suffix to the compiled blobs
5ce8634733b7 libfdt: Add fdt_overlay_apply to the exported symbols
804a9db90ad2 fdt: strerr: Remove spurious BADOVERLAY
e8c3a1a493fa tests: overlay: Move back the bad fixup tests
7a72d89d3f81 libfdt: overlay: Fix symbols and fixups nodes condition
cabbaa972cdd libfdt: overlay: Report a bad overlay for mismatching local fixups
deb0a5c1aeaa libfdt: Add BADPHANDLE error string
7b7a6be9ba15 libfdt: Don't use 'index' as a local variable name
aea8860d831e tests: Add tests cases for the overlay code
0cdd06c5135b libfdt: Add overlay application function
39240cc865cf libfdt: Extend the reach of FDT_ERR_BADPHANDLE
4aa3a6f5e6d9 libfdt: Add new errors for the overlay code
6d1832c9e64b dtc: Remove "home page" link
45fd440a9561 Fix some typing errors in libfdt.h and livetree.c
a59be4939c13 Merge tag 'v1.4.2'
a34bb721caca dtc: Fix assorted problems in the testcases for the -a option
874f40588d3e Implement the -a option to pad dtb aligned
ec02b34c05be dtc: Makefile improvements for release uploading
1ed45d40a137 dtc: Bump version to 1.4.2
36fd7331fb11 libfdt: simplify fdt_del_mem_rsv()
d877364e4a0f libfdt: Add fdt_setprop_inplace_namelen_partial
3e9037aaad44 libfdt: Add fdt_getprop_namelen_w
84e0e1346c68 libfdt: Add max phandle retrieval function
d29126c90acb libfdt: Add iterator over properties
902d0f0953d0 libfdt: Add a subnodes iterator macro
c539075ba8ba fdtput.c: Fix memory leak.
f79ddb83e185 fdtget.c: Fix memory leak
1074ee54b63f convert-dtsv0-lexer.l: fix memory leak
e24d39a024e6 fdtdump.c: make sure size_t argument to memchr is always unsigned.
44a59713cf05 Remove unused srcpos_dump() function
cb9241ae3453 DTC: Fix memory leak on flatname.
1ee0ae24ea09 Simplify check field and macro names
9d97527a8621 Remove property check functions
2e709d158e11 Remove tree check functions
c4cb12e193e3 Alter grammar to allow multiple /dts-v1/ tags
d71d25d76012 Use xasprintf() in srcpos
9dc404958e9c util: Add xasprintf portable asprintf variant
beef80b8b55f Correct a missing space in a fdt_header cast
68d43cec1253 Correct line lengths in libfdt.h
b0dbceafd49a Correct space-after-tab in libfdt.h
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
scripts/dtc/checks.c | 349 ++++++++--------
scripts/dtc/dtc-lexer.l | 21 +-
scripts/dtc/dtc-lexer.lex.c_shipped | 650 +++++++++++++++---------------
scripts/dtc/dtc-parser.tab.c_shipped | 752 ++++++++++++++++++-----------------
scripts/dtc/dtc-parser.tab.h_shipped | 54 +--
scripts/dtc/dtc-parser.y | 34 +-
scripts/dtc/dtc.c | 69 +++-
scripts/dtc/dtc.h | 39 +-
scripts/dtc/flattree.c | 41 +-
scripts/dtc/fstree.c | 5 +-
scripts/dtc/libfdt/Makefile.libfdt | 2 +-
scripts/dtc/libfdt/fdt_ro.c | 30 +-
scripts/dtc/libfdt/fdt_rw.c | 6 +-
scripts/dtc/libfdt/fdt_strerror.c | 6 +
scripts/dtc/libfdt/fdt_wip.c | 29 +-
scripts/dtc/libfdt/libfdt.h | 210 ++++++++--
scripts/dtc/libfdt/libfdt_env.h | 1 +
scripts/dtc/livetree.c | 299 +++++++++++++-
scripts/dtc/srcpos.c | 35 +-
scripts/dtc/srcpos.h | 1 -
scripts/dtc/treesource.c | 14 +-
scripts/dtc/util.c | 30 ++
scripts/dtc/util.h | 1 +
scripts/dtc/version_gen.h | 2 +-
24 files changed, 1661 insertions(+), 1019 deletions(-)
Just sending diffstat to the list. Full patch is here[1].
Rob
[1] https://git.kernel.org/cgit/linux/kernel/git/robh/linux.git/commit/?h=dt/next&id=6f05afcbb031722ec1eff77dde188ff2edf8940e
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver
From: Nishanth Menon @ 2017-01-04 16:51 UTC (permalink / raw)
To: Tony Lindgren
Cc: Linus Walleij, Gary Bisson, Grygorii Strashko, Mark Rutland,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Lokesh Vutla
In-Reply-To: <20170104160511.GF25222-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
On 01/04/2017 10:05 AM, Tony Lindgren wrote:
> * Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> [170104 07:39]:
>> On 01/02/2017 04:12 PM, Tony Lindgren wrote:
>> [...]
>> Minor one:
>>> + * Copyright (C) 2015 Texas Instruments, Inc.
>> Could you make this:
>> Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
>
> Sure, it's your patch :) Will repost.
Thanks.
I just finished testing it as well and well it still does the job it
used to do. thanks for taking this forward.
I did get a few kernel-doc warnings, which i need to look closer at.
will comment back today if I see something of use for us to fix up.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/5] ARM: dts: armada388: rework clearfog's .dtsi references
From: Gregory CLEMENT @ 2017-01-04 16:31 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Rob Herring, Sebastian Hesselbarth
In-Reply-To: <E1cO4UH-0008SR-GW-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
Hi Russell,
On lun., janv. 02 2017, Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> This patch series, based upon the previous series adding Clearfog Base
> support, reworks the clearfog .dtsi file to reference nodes by label
> rather than by path.
>
> Not everything is moved - just those which had labels at the time the
> patches were created.
All the series is applied on mvebu/dt, however I would like to have a
commit log for the first patch before pushing it to arm-soc.
Thanks,
Gregory
>
> arch/arm/boot/dts/armada-388-clearfog-base.dts | 15 +
> arch/arm/boot/dts/armada-388-clearfog.dtsi | 353 ++++++++++-----------
> .../arm/boot/dts/armada-38x-solidrun-microsom.dtsi | 113 ++++---
> 3 files changed, 245 insertions(+), 236 deletions(-)
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/5] ARM: dts: armada388-clearfog: add phy reset gpio-hog
From: Gregory CLEMENT @ 2017-01-04 16:26 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper,
Sebastian Hesselbarth, Rob Herring, Mark Rutland,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <E1cO4Vz-00005t-8n-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
Hi Russell,
On lun., janv. 02 2017, Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
It would be nice to have some word here about this patch. Especially why
we need it now. I guess it is for being less dependent on the
initialization done by the bootloader but maybe you have other reasons.
Thanks,
Gregory
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> ---
> arch/arm/boot/dts/armada-388-clearfog-base.dts | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm/boot/dts/armada-388-clearfog-base.dts b/arch/arm/boot/dts/armada-388-clearfog-base.dts
> index f86e1876fb38..da788ea40717 100644
> --- a/arch/arm/boot/dts/armada-388-clearfog-base.dts
> +++ b/arch/arm/boot/dts/armada-388-clearfog-base.dts
> @@ -74,7 +74,17 @@
> phy = <&phy1>;
> };
>
> +&gpio0 {
> + phy1_reset {
> + gpio-hog;
> + gpios = <19 GPIO_ACTIVE_LOW>;
> + output-low;
> + line-name = "phy1-reset";
> + };
> +};
> +
> &mdio {
> + pinctrl-0 = <&mdio_pins µsom_phy_clk_pins &clearfog_phy_pins>;
> phy1: ethernet-phy@1 {
> /*
> * Annoyingly, the marvell phy driver configures the LED
> @@ -87,6 +97,11 @@
> };
>
> &pinctrl {
> + /* phy1 reset */
> + clearfog_phy_pins: clearfog-phy-pins {
> + marvell,pins = "mpp19";
> + marvell,function = "gpio";
> + };
> rear_button_pins: rear-button-pins {
> marvell,pins = "mpp44";
> marvell,function = "gpio";
> --
> 2.7.4
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V5 3/3] cfg80211: support ieee80211-freq-limit DT property
From: Rafał Miłecki @ 2017-01-04 16:13 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rafał Miłecki
In-Reply-To: <1483536406.7312.3.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
On 4 January 2017 at 14:26, Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
>
>> V4: Move code to of.c
>> Use one helper called at init time (no runtime hooks)
>> Modify orig_flags
>
>
>> +/**
>> + * wiphy_read_of_freq_limits - read frequency limits from device
>> tree
>> + *
>> + * @wiphy: the wireless device to get extra limits for
>> + *
>> + * Some devices may have extra limitations specified in DT. This may
>> be useful
>> + * for chipsets that normally support more bands but are limited due
>> to board
>> + * design (e.g. by antennas or extermal power amplifier).
>> + *
>> + * This function reads info from DT and uses it to *modify* channels
>> (disable
>> + * unavailable ones). It's usually a *bad* idea to use it in drivers
>> with
>> + * shared channel data as DT limitations are device specific.
>> + *
>> + * As this function access device node it has to be called after
>> set_wiphy_dev.
>> + * It also modifies channels so they have to be set first.
>> + */
>
> It should also be called before wiphy_register(), I think? And I
> suppose you should add a comment about not being able to use shared
> channels.
>
>> + pr_debug("Disabling freq %d MHz as
>> it's out of OF limits\n",
>> + chan->center_freq);
>> + chan->orig_flags |=
>> IEEE80211_CHAN_DISABLED;
>>
> But just setting orig_flags also won't work, since it'd be overwritten
> again by wiphy_register(), no?
I told you I successfully tested it, didn't I? Well, I quickly checked
wiphy_register and couldn't understand how it was possible it worked
for me...
OK, so after some debugging I understood why I got this working. It's
the way brcmfmac handles channels.
At the beginning all channels are disabled: see __wl_2ghz_channels &
__wl_5ghz_channels. They have IEEE80211_CHAN_DISABLED set in "flags"
for every channel.
In early phase brcmfmac calls wiphy_read_of_freq_limits which sets
IEEE80211_CHAN_DISABLED in "orig_flags" for unavailable channels.
Then brcmf_construct_chaninfo kicks in. Normally it removes
IEEE80211_CHAN_DISABLED from "flags" for most of channels, but it
doesn't happen anymore due to my change:
if (channel->orig_flags & IEEE80211_CHAN_DISABLED)
continue;
Then brcmfmac calls wiphy_apply_custom_regulatory which sets some bits
like IEEE80211_CHAN_NO_80MHZ and IEEE80211_CHAN_NO_160MHZ in "flags".
Finally wiphy_register is called which copies "flags" to
"original_flags". As brcmfmac /respected/ IEEE80211_CHAN_DISABLED set
in orig_flags, it also left IEEE80211_CHAN_DISABLED in flags. This way
I got IEEE80211_CHAN_DISABLED in orig_flags after overwriting that
field inside wiphy_register.
That's quite crazy, right?
I guess you're right after all, I should set IEEE80211_CHAN_DISABLED
in "flags" field, let wiphy_register copy that to "orig_flags" and
sanitize brcmfmac.
--
Rafał
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver
From: Tony Lindgren @ 2017-01-04 16:05 UTC (permalink / raw)
To: Nishanth Menon
Cc: Linus Walleij, Gary Bisson, Grygorii Strashko, Mark Rutland,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Lokesh Vutla
In-Reply-To: <63a8a1ec-343a-8c96-a0d2-21d81f7ad10e-l0cyMroinI0@public.gmane.org>
* Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> [170104 07:39]:
> On 01/02/2017 04:12 PM, Tony Lindgren wrote:
> [...]
> Minor one:
> > + * Copyright (C) 2015 Texas Instruments, Inc.
> Could you make this:
> Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
Sure, it's your patch :) Will repost.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: mbox-name vs. mbox-names (was: Re: [PATCH v4 1/5] mailbox: dt: Supply bindings for ST's Mailbox IP)
From: Lee Jones @ 2017-01-04 16:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, kernel, Maxime Coquelin, Jassi Brar,
devicetree@vger.kernel.org
In-Reply-To: <20161024084107.GD14477@dell>
On Mon, 24 Oct 2016, Lee Jones wrote:
> On Fri, 21 Oct 2016, Geert Uytterhoeven wrote:
>
> > On Fri, Oct 16, 2015 at 9:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > > .../devicetree/bindings/mailbox/sti-mailbox.txt | 51 ++++++++++++++++++++++
> > > 1 file changed, 51 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > > new file mode 100644
> > > index 0000000..b61eec9
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > > @@ -0,0 +1,51 @@
> > > +ST Microelectronics Mailbox Driver
> > > +
> > > +Each ST Mailbox IP currently consists of 4 instances of 32 channels. Messages
> > > +are passed between Application and Remote processors using shared memory.
> > > +
> > > +Controller
> > > +----------
> > > +
> > > +Required properties:
> > > +- compatible : Should be "st,stih407-mailbox"
> > > +- reg : Offset and length of the device's register set
> > > +- mbox-name : Name of the mailbox
> >
> > All other mailbox drivers use "mbox-names". Oops, it's in v4.9-rc1...
> >
> > Can we still fix that?
>
> So long as all the fixes; changes to the driver and DT are merged in a
> single kernel release, we can change it.
Scrap that, change of plan. Actually current code is correct.
mbox-name does not have the same functionality as mbox-names.
mbox-names is a client-side property used to request channels, where
as mbox-name is used to provide a better user-readable name other than
mailbox{0..x}.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] mailbox: sti: Fix mbox-names copy and paste error
From: Lee Jones @ 2017-01-04 16:01 UTC (permalink / raw)
To: Rob Herring
Cc: patrice.chotard-qxv4g6HH51o,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-F5mvAk5X5gdBDgjK7y7TUQ, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170104145732.7yiqbi7rf7m6lem4@rob-hp-laptop>
On Wed, 04 Jan 2017, Rob Herring wrote:
> On Wed, Jan 04, 2017 at 12:05:27PM +0000, Lee Jones wrote:
> > Due to an over-sight, mbox-names has become mbox-name in some instances.
> >
> > Let's put it right.
> >
> > Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > Documentation/devicetree/bindings/mailbox/sti-mailbox.txt | 4 ++--
> > arch/arm/boot/dts/stih407-family.dtsi | 8 ++++----
> > drivers/mailbox/mailbox-sti.c | 2 +-
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > index 351f612..648d176 100644
> > --- a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > +++ b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> > @@ -9,7 +9,7 @@ Controller
> > Required properties:
> > - compatible : Should be "st,stih407-mailbox"
> > - reg : Offset and length of the device's register set
> > -- mbox-name : Name of the mailbox
> > +- mbox-names : Name of the mailbox
>
> It's worse than this. mbox-names is for the mailbox client side.
Ah yes, of course. False alarm.
Let's just leave it as it is. ;)
I'll reply to the reporter directly.
> This should just be dropped. Plus, *-names is pointless when there
> is only 1 element.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/9] ARM: dts: armada388: Introduce Clearfog Base DT
From: Gregory CLEMENT @ 2017-01-04 15:48 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Jason Cooper,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
Rob Herring, Sebastian Hesselbarth
In-Reply-To: <E1cO43Q-0007yJ-ON-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
Hi Russell,
On lun., janv. 02 2017, Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> This patch series, based upon the previously submitted fix for the SPI
> flash, reworks the Clearfog DT files to add support for the SolidRun
> Clearfog Base platform.
>
> The conventional model is now known as the "Clearfog Pro" module, which
> has the DSA switch and two PCIe sockets. The base model is a smaller
> board without the DSA switch, replacing it with a second copper gigabit
> port, and only one PCIe socket.
>
> We retain the original DT file (named armada-388-clearfog.dtb) for
> compatibility with existing installations - not only the filename,
> but also the board name exposed in userspace.
All the series applied on mvebu/dt.
I amended the last two patches because the license text was mangled as
spotted by Alexandre Belloni, so I fixed it while applying the patches.
Thanks,
Gregory
>
> arch/arm/boot/dts/Makefile | 2 +
> arch/arm/boot/dts/armada-388-clearfog-base.dts | 94 ++++++
> arch/arm/boot/dts/armada-388-clearfog-pro.dts | 55 ++++
> arch/arm/boot/dts/armada-388-clearfog.dts | 364 ++++-----------------
> arch/arm/boot/dts/armada-388-clearfog.dtsi | 310 ++++++++++++++++++
> .../arm/boot/dts/armada-38x-solidrun-microsom.dtsi | 21 ++
> 6 files changed, 548 insertions(+), 298 deletions(-)
> create mode 100644 arch/arm/boot/dts/armada-388-clearfog-base.dts
> create mode 100644 arch/arm/boot/dts/armada-388-clearfog-pro.dts
> create mode 100644 arch/arm/boot/dts/armada-388-clearfog.dtsi
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver
From: Nishanth Menon @ 2017-01-04 15:38 UTC (permalink / raw)
To: Tony Lindgren, Linus Walleij
Cc: Gary Bisson, Grygorii Strashko, Mark Rutland, Rob Herring,
devicetree, linux-gpio, linux-omap, Lokesh Vutla
In-Reply-To: <20170102221228.GH9325@atomide.com>
On 01/02/2017 04:12 PM, Tony Lindgren wrote:
[...]
Minor one:
> + * Copyright (C) 2015 Texas Instruments, Inc.
Could you make this:
Copyright (C) 2015-2017 Texas Instruments Incorporated -
http://www.ti.com/
--
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
From: Fabio Estevam @ 2017-01-04 15:26 UTC (permalink / raw)
To: Steve Longerbeam
Cc: Mark Rutland, devel, Steve Longerbeam, Philipp Zabel,
devicetree@vger.kernel.org, Greg Kroah-Hartman,
Russell King - ARM Linux, linux-kernel, robh+dt@kernel.org,
Sascha Hauer, Fabio Estevam, mchehab, Shawn Guo,
linux-arm-kernel@lists.infradead.org, linux-media
In-Reply-To: <1483477049-19056-6-git-send-email-steve_longerbeam@mentor.com>
On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> + camera: ov5642@3c {
> + compatible = "ovti,ov5642";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5642>;
> + clocks = <&clks IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + reg = <0x3c>;
> + xclk = <24000000>;
> + DOVDD-supply = <&vgen4_reg>; /* 1.8v */
> + AVDD-supply = <&vgen5_reg>; /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
Please use vgen3 so that by default we have the valid AVDD-supply for
revC boards which is more recent and more the users have access to.
> + mipi_camera: ov5640@3c {
> + compatible = "ovti,ov5640_mipi";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5640>;
> + reg = <0x3c>;
> + clocks = <&clks IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + xclk = <24000000>;
> + DOVDD-supply = <&vgen4_reg>; /* 1.8v */
> + AVDD-supply = <&vgen5_reg>; /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
Same here.
> + pinctrl_ov5640: ov5640grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x80000000
> + MX6QDL_PAD_SD1_CLK__GPIO1_IO20 0x80000000
Please avoid all the 0x80000000 IOMUX settings and replace them by
their real values.
^ permalink raw reply
* Re: [PATCH v4 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Rob Herring @ 2017-01-04 15:26 UTC (permalink / raw)
To: Kedareswara rao Appana
Cc: mark.rutland-5wv7dgnIgG8, dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
appanad-gjFFaj9aHVfQT0dZR+AlfA,
moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
luis-HiykPkW1eAzzDCI4PIEvbQC/G2K4zDHf,
Jose.Abreu-HKixBCOQz3hWk0Htik3J/w,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483536954-27203-3-git-send-email-appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
On Wed, Jan 04, 2017 at 07:05:53PM +0530, Kedareswara rao Appana wrote:
> When VDMA is configured for more than one frame in the h/w
> for example h/w is configured for n number of frames and user
> Submits n number of frames and triggered the DMA using issue_pending API.
> In the current driver flow we are submitting one frame at a time
> but we should submit all the n number of frames at one time as the h/w
> Is configured for n number of frames.
Please fix run-on sentences, capitalization, and word wrapping.
>
> This patch fixes this issue.
>
> Reviewed-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v4:
> ---> Add Check for framestore configuration on Transmit case as well
> as suggested by Jose Abreu.
> ---> Modified the dev_dbg checks to dev_warn checks as suggested
> by Jose Abreu.
> Changes for v3:
> ---> Added Checks for frame store configuration. If frame store
> Configuration is not present at the h/w level and user
> Submits less frames added debug prints in the driver as relevant.
> Changes for v2:
> ---> Fixed race conditions in the driver as suggested by Jose Abreu
> ---> Fixed unnecessray if else checks in the vdma_start_transfer
> as suggested by Laurent Pinchart.
>
> .../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
> drivers/dma/xilinx/xilinx_dma.c | 79 +++++++++++++++-------
> 2 files changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> index a2b8bfa..1f65e09 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> @@ -66,6 +66,8 @@ Optional child node properties:
> Optional child node properties for VDMA:
> - xlnx,genlock-mode: Tells Genlock synchronization is
> enabled/disabled in hardware.
> +- xlnx,fstore-config: Tells Whether Frame Store Configuration is
> + enabled/disabled in hardware.
What's the default (when not present)? That should be the most
common case. Looks like the code treats this as bool, but that's
not clear here. The name is not clear what it is doing. Enabling or
disabling the feature?
> Optional child node properties for AXI DMA:
> -dma-channels: Number of dma channels in child node.
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V2 3/5] Documetation: binding: modify the exynos5440 pcie binding
From: Rob Herring @ 2017-01-04 15:18 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
mark.rutland, kgene, krzk, kishon, jingoohan1, vivek.gautam,
pankaj.dubey, alim.akhtar, cpgs
In-Reply-To: <20170104123435.30740-4-jh80.chung@samsung.com>
On Wed, Jan 04, 2017 at 09:34:33PM +0900, Jaehoon Chung wrote:
> According to using PHY framework, updates the exynos5440-pcie binding.
> For maintaining backward compatibility, leaves the current dt-binding.
> (It should be deprecated.)
>
> Recommends to use the Phy Framework and "config" property to follow
> the designware-pcie binding.
> If you use the old way, can see "mssing *config* reg space" message.
> Because the getting configuration space address from range is old way.
>
> NOTE: When use the "config" property, first name of 'reg-names' must be
> set to "elbi". Otherwise driver can't maintain the backward capability.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Describes more commit message
> - Fixes the typos
> - Adds the new example for using PHY framework
> - Deprecated the old dt-binding description
> - Removes 'phy-names'
>
> .../bindings/pci/samsung,exynos5440-pcie.txt | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding
From: Rob Herring @ 2017-01-04 15:17 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, mark.rutland-5wv7dgnIgG8,
kgene-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
kishon-l0cyMroinI0, jingoohan1-Re5JQEeQqe8AvxtiuMwx3w,
vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ,
pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ,
alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ, cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20170104123435.30740-2-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, Jan 04, 2017 at 09:34:31PM +0900, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Remove the child node.
> - Add 2nd address to the parent reg prop.
>
> Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver
From: Vladimir Zapolskiy @ 2017-01-04 15:05 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-16-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/staging/media/imx/Makefile | 1 +
> drivers/staging/media/imx/imx-mipi-csi2.c | 509 ++++++++++++++++++++++++++++++
> 2 files changed, 510 insertions(+)
> create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
>
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 0000000..84df16e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,509 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/list.h>
> +#include <linux/irq.h>
> +#include <linux/of_device.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-async.h>
Please sort the list of headers alphabetically.
> +#include <asm/mach/irq.h>
Why do you need to include this header?
> +#include <video/imx-ipu-v3.h>
> +#include "imx-media.h"
> +
[snip]
> +static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> + int i, ret = 0;
> +
> + if (!csi2->src_sd)
> + return -EPIPE;
> + for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
> + if (csi2->sink_sd[i])
> + break;
> + }
> + if (i >= CSI2_NUM_SRC_PADS)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !csi2->stream_on) {
> + clk_prepare_enable(csi2->pix_clk);
It can complicate the design for you, but in general clk_prepare_enable()
can return an error.
> + ret = imxcsi2_dphy_wait(csi2);
> + if (ret)
> + clk_disable_unprepare(csi2->pix_clk);
> + } else if (!enable && csi2->stream_on) {
> + clk_disable_unprepare(csi2->pix_clk);
> + }
> +
> + if (!ret)
> + csi2->stream_on = enable;
> + return ret;
> +}
> +
[snip]
> +
> +static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
> +{
> + struct device_node *node = csi2->dev->of_node;
> + struct device_node *epnode;
> + struct v4l2_of_endpoint ep;
> + int ret = 0;
> +
> + epnode = of_graph_get_next_endpoint(node, NULL);
> + if (!epnode) {
> + v4l2_err(&csi2->sd, "failed to get endpoint node\n");
> + return -EINVAL;
> + }
> +
> + v4l2_of_parse_endpoint(epnode, &ep);
Do of_node_put(epnode) here and remove 'out' goto label.
> + if (ep.bus_type != V4L2_MBUS_CSI2) {
> + v4l2_err(&csi2->sd, "invalid bus type, must be MIPI CSI2\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + csi2->bus = ep.bus.mipi_csi2;
> +
> + v4l2_info(&csi2->sd, "data lanes: %d\n", csi2->bus.num_data_lanes);
> + v4l2_info(&csi2->sd, "flags: 0x%08x\n", csi2->bus.flags);
> +out:
> + of_node_put(epnode);
> + return ret;
> +}
> +
[snip]
> +static const struct of_device_id imxcsi2_dt_ids[] = {
> + { .compatible = "fsl,imx-mipi-csi2", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
> +
> +static struct platform_driver imxcsi2_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,
Please drop .owner assignment.
> + .of_match_table = imxcsi2_dt_ids,
> + },
> + .probe = imxcsi2_probe,
> + .remove = imxcsi2_remove,
> +};
> +
> +module_platform_driver(imxcsi2_driver);
> +
> +MODULE_DESCRIPTION("i.MX5/6 MIPI CSI-2 Receiver driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
> +
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH] mailbox: sti: Fix mbox-names copy and paste error
From: Rob Herring @ 2017-01-04 14:57 UTC (permalink / raw)
To: Lee Jones
Cc: patrice.chotard, jassisinghbrar, linux-arm-kernel, linux-kernel,
kernel, devicetree
In-Reply-To: <20170104120527.24806-1-lee.jones@linaro.org>
On Wed, Jan 04, 2017 at 12:05:27PM +0000, Lee Jones wrote:
> Due to an over-sight, mbox-names has become mbox-name in some instances.
>
> Let's put it right.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> Documentation/devicetree/bindings/mailbox/sti-mailbox.txt | 4 ++--
> arch/arm/boot/dts/stih407-family.dtsi | 8 ++++----
> drivers/mailbox/mailbox-sti.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> index 351f612..648d176 100644
> --- a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> +++ b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> @@ -9,7 +9,7 @@ Controller
> Required properties:
> - compatible : Should be "st,stih407-mailbox"
> - reg : Offset and length of the device's register set
> -- mbox-name : Name of the mailbox
> +- mbox-names : Name of the mailbox
It's worse than this. mbox-names is for the mailbox client side. This
should just be dropped. Plus, *-names is pointless when there is only 1
element.
Rob
^ permalink raw reply
* Re: [PATCH v2 14/19] media: imx: Add Camera Interface subdev driver
From: Vladimir Zapolskiy @ 2017-01-04 14:55 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-15-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is the camera interface driver that provides the v4l2
> user interface. Frames can be received from various sources:
>
> - directly from SMFC for capturing unconverted images directly from
> camera sensors.
>
> - from the IC pre-process encode task.
>
> - from the IC pre-process viewfinder task.
>
> - from the IC post-process task.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/staging/media/imx/Makefile | 2 +-
> drivers/staging/media/imx/imx-camif.c | 1010 +++++++++++++++++++++++++++++++++
> 2 files changed, 1011 insertions(+), 1 deletion(-)
> create mode 100644 drivers/staging/media/imx/imx-camif.c
>
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index d2a962c..fe9e992 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -8,4 +8,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
> -
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o imx-csi.o imx-smfc.o
as an option.
> diff --git a/drivers/staging/media/imx/imx-camif.c b/drivers/staging/media/imx/imx-camif.c
> new file mode 100644
> index 0000000..3cf167e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-camif.c
> @@ -0,0 +1,1010 @@
> +/*
> + * Video Camera Capture Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/of_platform.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
Please sort the list of headers alphabetically.
> +#include <video/imx-ipu-v3.h>
> +#include <media/imx.h>
> +#include "imx-media.h"
> +
> +#define DEVICE_NAME "imx-media-camif"
I would propose to drop this macro.
> +
> +#define CAMIF_NUM_PADS 2
> +
> +#define CAMIF_DQ_TIMEOUT 5000
Add a comment about time unit?
> +
> +struct camif_priv;
> +
This is a leftover apparently.
> +struct camif_priv {
> + struct device *dev;
> + struct video_device vfd;
> + struct media_pipeline mp;
> + struct imx_media_dev *md;
[snip]
> +static int camif_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct camif_priv *priv;
> + struct video_device *vfd;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> + priv->dev = &pdev->dev;
> +
> + pdata = priv->dev->platform_data;
> +
> + mutex_init(&priv->mutex);
> + spin_lock_init(&priv->q_lock);
> +
> + v4l2_subdev_init(&priv->sd, &camif_subdev_ops);
> + v4l2_set_subdevdata(&priv->sd, priv);
> + priv->sd.internal_ops = &camif_internal_ops;
> + priv->sd.entity.ops = &camif_entity_ops;
> + priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->sd.dev = &pdev->dev;
> + priv->sd.owner = THIS_MODULE;
> + /* get our group id and camif id */
> + priv->sd.grp_id = pdata->grp_id;
> + priv->id = (pdata->grp_id >> IMX_MEDIA_GRP_ID_CAMIF_BIT) - 1;
> + strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
> + snprintf(camif_videodev.name, sizeof(camif_videodev.name),
> + "%s devnode", pdata->sd_name);
> +
> + priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> +
> + vfd = &priv->vfd;
> + *vfd = camif_videodev;
> + vfd->lock = &priv->mutex;
> + vfd->queue = &priv->buffer_queue;
> +
> + video_set_drvdata(vfd, priv);
> +
> + v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
> +
> + ret = v4l2_async_register_subdev(&priv->sd);
> + if (ret)
> + goto free_ctrls;
> +
> + return 0;
> +free_ctrls:
> + v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
> + return ret;
A shorter version:
if (ret)
v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
return ret;
> +}
> +
> +static int camif_remove(struct platform_device *pdev)
> +{
> + struct camif_priv *priv =
> + (struct camif_priv *)platform_get_drvdata(pdev);
> +
> + v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
> + v4l2_async_unregister_subdev(&priv->sd);
> + media_entity_cleanup(&priv->sd.entity);
> + v4l2_device_unregister_subdev(&priv->sd);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id camif_ids[] = {
> + { .name = DEVICE_NAME },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, camif_ids);
> +
> +static struct platform_driver imx_camif_driver = {
> + .probe = camif_probe,
> + .remove = camif_remove,
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,
Please drop the owner assignment.
> + },
> +};
> +
> +module_platform_driver(imx_camif_driver);
> +
> +MODULE_DESCRIPTION("i.MX camera interface subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v6 1/5] dt-bindings: zte: documents zx2967 power domain driver bindings
From: Rob Herring @ 2017-01-04 14:54 UTC (permalink / raw)
To: Baoyou Xie
Cc: jun.nie, mark.rutland, krzk, arnd, ulf.hansson, amitdanielk,
claudiu.manoil, yangbo.lu, pankaj.dubey, geert+renesas,
laurent.pinchart, linux-arm-kernel, devicetree, linux-kernel,
shawnguo, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1483530494-14177-1-git-send-email-baoyou.xie@linaro.org>
On Wed, Jan 04, 2017 at 07:48:10PM +0800, Baoyou Xie wrote:
> This patch documents devicetree tree bindings for the ZTE zx2967
> power domain driver.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt b/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
> new file mode 100644
> index 0000000..1476318
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
> @@ -0,0 +1,17 @@
> +* ZTE 2967 SoC Power Domains
> +
> +2967 processors include support for multiple power domains which are used
> +to gate power to one or more peripherals on the processor.
> +
> +Required Properties:
> +- compatible: should be one of the following.
> + * zte,zx296718-pcu - for zx296718 board's power domain.
board? I'd expect this is for an SoC?
> +- reg: physical base address of the controller and length of memory mapped
> + region.
#power-domain-cells needed?
> +
> +Example:
> +
> + pcu_domain: pcu@0x00117000 {
pcu@117000
> + compatible = "zte,zx296718-pcu";
> + reg = <0x00117000 0x1000>;
> + };
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2017-01-04 14:53 UTC (permalink / raw)
To: Kalle Valo
Cc: Rob Herring,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rafał Miłecki
In-Reply-To: <874m1ej3oz.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On 4 January 2017 at 15:32, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> This new file should be used for properties handled at higher level and
>>>> so usable with all drivers.
>>>
>>> Why is this needed? Where would this data normally come from?
>>
>> Vendors limit choice of channels at their web user interface level. I
>> want to do better and report proper channels directly at kernel level
>> instead of masking them in every possible configuration tool.
>
> Why do vendors limit the channels? Is it because of a hardware
> limitation (antenna does not support, or not calibrated, for a certain
> band etc) or something else?
Please review & comment on the latest version, currently V5:
https://patchwork.kernel.org/patch/9495795/
"This can be used to specify extra hardware limitations caused by e.g.
used antennas or power amplifiers."
--
Rafał
^ permalink raw reply
* Re: [PATCH] dt-bindings: qman: Remove pool channel node
From: Rob Herring @ 2017-01-04 14:52 UTC (permalink / raw)
To: Madalin Bucur
Cc: mark.rutland-5wv7dgnIgG8, oss-fOR+EgIDQEHk1uMJSBkQmQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1483527829-8369-1-git-send-email-madalin.bucur-3arQi8VN3Tc@public.gmane.org>
On Wed, Jan 04, 2017 at 01:03:49PM +0200, Madalin Bucur wrote:
> From: Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
>
> No device tree has these, nor does any driver look for them.
>
> Signed-off-by: Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
> ---
> .../devicetree/bindings/soc/fsl/qman-portals.txt | 20 --------------------
> 1 file changed, 20 deletions(-)
Applied, thanks.
Rob
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt b/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> index 47e46cc..5a34f3a 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> @@ -5,7 +5,6 @@ Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
> CONTENTS
>
> - QMan Portal
> - - QMan Pool Channel
> - Example
>
> QMan Portal Node
> @@ -82,25 +81,6 @@ These subnodes should have the following properties:
> Definition: The phandle to the particular hardware device that this
> portal is connected to.
>
> -DPAA QMan Pool Channel Nodes
> -
> -Pool Channels are defined with the following properties.
> -
> -PROPERTIES
> -
> -- compatible
> - Usage: Required
> - Value type: <stringlist>
> - Definition: Must include "fsl,qman-pool-channel"
> - May include "fsl,<SoC>-qman-pool-channel"
> -
> -- fsl,qman-channel-id
> - Usage: Required
> - Value type: <u32>
> - Definition: The hardware index of the channel. This can also be
> - determined by dividing any of the channel's 8 work queue
> - IDs by 8
> -
> EXAMPLE
>
> The example below shows a (P4080) QMan portals container/bus node with two portals
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/4] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cyrille Pitchen @ 2017-01-04 14:52 UTC (permalink / raw)
To: Cédric Le Goater, Cyrille Pitchen,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Mark Rutland, Boris Brezillon, devicetree-u79uwXL29TY76Z2rM5mHXA,
Richard Weinberger, Marek Vasut, Rob Herring, Joel Stanley,
Brian Norris, David Woodhouse
In-Reply-To: <645db8c4-7f3c-f8bf-ddd9-3f513ce2ed14-Bxea+6Xhats@public.gmane.org>
Hi Cedric,
Le 21/12/2016 à 17:47, Cédric Le Goater a écrit :
> Hello Cyrille,
>
>>>> all aspeed_smc_[read|write]_[reg|user]() functions call
>>>> aspeed_smc_[start|stop]_user(), so this driver always selects the "USER"
>>>> mode and configures the control register based on chip->ctrl_val[smc_base].
>>>
>>> yes.
>>>
>>> Maybe you think it is too early to prepare ground for the other
>>> mode ? Is that really confusing in the code ? Do you think I should
>>> remove it for the initial support in the driver and stick to 'User'
>>> mode.
>>>
>>
>> I think it is not a big deal, at least technically. This is more a
>> psychological aspect of the review: the bigger patches, the more scarier.
>> I mean it requires more time and courage to dig into the source code hence
>> to understand what the driver actually does.
>> Sometime, it's better to split a big patch into many incremental and
>> smaller patches so it's more easy for reviewers to understand each part as
>> an independent feature. It also reveals the driver evolution during the
>> development process hence it helps to understand where it goes and what are
>> the next improvements to come.
>
> I fully agree. It is just that we have been sitting on the code for more
> than a year, using it in production and we should send to mainline
> much sooner. that was a mistake of us ...
>
>> Anyway, since the review is done now, on my side I won't ask you to remove
>> or split the support of the 'Command' mode in a separated patch.
>> I let you do as you want, if it help you to introduce some part of the
>> support of this 'Command' mode now even if not completed yet, no problem on
>> my side :)
>>
>> I was just giving you some pieces of advice for the next time if you want
>> to speed up the review of another patch introducing new features.
>>
>> However, I will just ask you one more version handling the dummy cycles
>> properly as it would help us for the global maintenance of the spi-nor
>> subsystem. This is the only mandatory modification I ask you, after that I
>> think it would be ok for me and since Marek has already reviewed your
>> driver, it would be ready to be merged into the spi-nor tree.
>
> Sending a v5 which should address your comments.
>
> I have removed the label property and will start a new thread in the
> topic. Any hints on which binding we could add this label prop ?
>
Here I will provide just few thoughts about this new DT property. I don't
pretend this is what should be done. I still think other mtd maintainers
should be involved to discuss this topic.
First the DT property name "label" sounds good to me: it is consistent with
"label" DT property used to name mtd partitions. However, I don't think it
should be documented in jedec,spi-nor.txt but *maybe* in partition.txt as
the purpose of this new DT property seems very close to the "label"
property of partition nodes: let's think about some hard-disk device
(/dev/sda) and its partition devices (/dev/sdaX).
Besides, the concept of this memory label is not limited to SPI NOR but
could also apply to NAND memories or any other MTD handled memories.
Hence the DT property might be handled by drivers/mtd/ofpart.c instead of
being handled by spi-nor.c or by each SPI NOR memory controller driver.
Finally, I guess we should take time to discuss and all agree what should
be done precisely before introducing a new DT property because one general
rule with DTB files is that users should be able to update their kernel
image (zImage, uImage, ...) without changing their DTB: device trees should
be backward compatible. Hence if we make a wrong choice today, we are
likely to have to live with it and keep supporting that bad choice.
Anyway, maybe you could describe a little bit your use case; what you
intend to do with this label from you userspace application.
Best regards,
Cyrille
>> Anyway, thanks for taking time to explain us how your driver work :)
>
> Thanks for the review !
>
> C.
>
>
>> Best regards,
>>
>> Cyrille
>>
>>
>>
>>>>> +static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>>>>> + struct device_node *np, struct resource *r)
>>>>> +{
>>>>> + const struct aspeed_smc_info *info = controller->info;
>>>>> + struct device *dev = controller->dev;
>>>>> + struct device_node *child;
>>>>> + unsigned int cs;
>>>>> + int ret = -ENODEV;
>>>>> +
>>>>> + for_each_available_child_of_node(np, child) {
>>>>> + struct aspeed_smc_chip *chip;
>>>>> + struct spi_nor *nor;
>>>>> + struct mtd_info *mtd;
>>>>> +
>>>>> + /* This driver does not support NAND or NOR flash devices. */
>>>>> + if (!of_device_is_compatible(child, "jedec,spi-nor"))
>>>>> + continue;
>>>>> +
>>>>> + ret = of_property_read_u32(child, "reg", &cs);
>>>>> + if (ret) {
>>>>> + dev_err(dev, "Couldn't not read chip select.\n");
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (cs >= info->nce) {
>>>>> + dev_err(dev, "Chip select %d out of range.\n",
>>>>> + cs);
>>>>> + ret = -ERANGE;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (controller->chips[cs]) {
>>>>> + dev_err(dev, "Chip select %d already in use by %s\n",
>>>>> + cs, dev_name(controller->chips[cs]->nor.dev));
>>>>> + ret = -EBUSY;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
>>>>> + if (!chip) {
>>>>> + ret = -ENOMEM;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + chip->controller = controller;
>>>>> + chip->ctl = controller->regs + info->ctl0 + cs * 4;
>>>>> + chip->cs = cs;
>>>>> +
>>>>> + nor = &chip->nor;
>>>>> + mtd = &nor->mtd;
>>>>> +
>>>>> + nor->dev = dev;
>>>>> + nor->priv = chip;
>>>>> + spi_nor_set_flash_node(nor, child);
>>>>> + nor->read = aspeed_smc_read_user;
>>>>> + nor->write = aspeed_smc_write_user;
>>>>> + nor->read_reg = aspeed_smc_read_reg;
>>>>> + nor->write_reg = aspeed_smc_write_reg;
>>>>> + nor->prepare = aspeed_smc_prep;
>>>>> + nor->unprepare = aspeed_smc_unprep;
>>>>> +
>>>>> + mtd->name = of_get_property(child, "label", NULL);
>>>>
>>>> This new "label" DT property should be removed from this patch and send
>>>> in a dedicated patch to be discussed separately. However I agree with
>>>> Marek: it looks generic so maybe it could be managed directly from
>>>> mtd_device_register() since setting such as name could also be done when
>>>> using a NAND flash. Anyway, I don't see the link between this name and
>>>> spi-nor. Hence I don't think the DT property should be documented in
>>>> jedec,spi-nor.txt.
>>>
>>> OK. I will remove it in the next version.
>>>
>>>> Be patient because I expect such a topic to be discussed quite a long
>>>> time before we all agree, even if this is "just" a new DT property ;)
>>>
>>> yeah. I expected that also :) But it is quite pratical to give user
>>> space a hint on the flash nature. Systems can have up to 4 different
>>> ones. So there is need for it IMO.
>>>
>>> How should I proceed then ? Shall I start a discussion with a single
>>> patch changing mtd_device_register() ? but I need to know which binding
>>> would be the more consensual for such a prop.
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>>>
>>>> Best regards,
>>>>
>>>> Cyrille
>>>>
>>>>
>>>>> +
>>>>> + ret = aspeed_smc_chip_setup_init(chip, r);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + /*
>>>>> + * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL
>>>>> + * attach when board support is present as determined
>>>>> + * by of property.
>>>>> + */
>>>>> + ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + ret = aspeed_smc_chip_setup_finish(chip);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + ret = mtd_device_register(mtd, NULL, 0);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + controller->chips[cs] = chip;
>>>>> + }
>>>>> +
>>>>> + if (ret)
>>>>> + aspeed_smc_unregister(controller);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int aspeed_smc_probe(struct platform_device *pdev)
>>>>> +{
>>>>> + struct device_node *np = pdev->dev.of_node;
>>>>> + struct device *dev = &pdev->dev;
>>>>> + struct aspeed_smc_controller *controller;
>>>>> + const struct of_device_id *match;
>>>>> + const struct aspeed_smc_info *info;
>>>>> + struct resource *res;
>>>>> + int ret;
>>>>> +
>>>>> + match = of_match_device(aspeed_smc_matches, &pdev->dev);
>>>>> + if (!match || !match->data)
>>>>> + return -ENODEV;
>>>>> + info = match->data;
>>>>> +
>>>>> + controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
>>>>> + info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
>>>>> + if (!controller)
>>>>> + return -ENOMEM;
>>>>> + controller->info = info;
>>>>> + controller->dev = dev;
>>>>> +
>>>>> + mutex_init(&controller->mutex);
>>>>> + platform_set_drvdata(pdev, controller);
>>>>> +
>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> + controller->regs = devm_ioremap_resource(dev, res);
>>>>> + if (IS_ERR(controller->regs)) {
>>>>> + dev_err(dev, "Cannot remap controller address.\n");
>>>>> + return PTR_ERR(controller->regs);
>>>>> + }
>>>>> +
>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>>> + controller->ahb_base = devm_ioremap_resource(dev, res);
>>>>> + if (IS_ERR(controller->ahb_base)) {
>>>>> + dev_err(dev, "Cannot remap controller address.\n");
>>>>> + return PTR_ERR(controller->ahb_base);
>>>>> + }
>>>>> +
>>>>> + ret = aspeed_smc_setup_flash(controller, np, res);
>>>>> + if (ret)
>>>>> + dev_err(dev, "Aspeed SMC probe failed %d\n", ret);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static struct platform_driver aspeed_smc_driver = {
>>>>> + .probe = aspeed_smc_probe,
>>>>> + .remove = aspeed_smc_remove,
>>>>> + .driver = {
>>>>> + .name = DEVICE_NAME,
>>>>> + .of_match_table = aspeed_smc_matches,
>>>>> + }
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(aspeed_smc_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
>>>>> +MODULE_AUTHOR("Cedric Le Goater <clg-Bxea+6Xhats@public.gmane.org>");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>>
>>>>
>>>
>>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox