* [PATCHv4 0/2] Nokia H4+ support
From: Sebastian Reichel @ 2017-04-13 0:26 UTC (permalink / raw)
To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
Johan Hedberg, Rob Herring
Cc: Pavel Machek, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi,
Here is the fourth revision of the nokia bluetooth patchset. Compared
to the previous one the following things have changed:
* Drop patch 1-8 (applied by Marcel) and update Cc/To for patchset
* Order includes in the driver alphabetically
* Explicitly include <linux/of.h> in the nokia bluetooth driver
-- Sebastian
Sebastian Reichel (2):
dt-bindings: net: bluetooth: Add nokia-bluetooth
Bluetooth: add nokia driver
.../devicetree/bindings/net/nokia-bluetooth.txt | 51 ++
drivers/bluetooth/Kconfig | 12 +
drivers/bluetooth/Makefile | 2 +
drivers/bluetooth/hci_nokia.c | 820 +++++++++++++++++++++
4 files changed, 885 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
create mode 100644 drivers/bluetooth/hci_nokia.c
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Sebastian Reichel @ 2017-04-13 0:26 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Greg Kroah-Hartman, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <C16140C4-0264-411B-9058-0B9013AAC82F-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]
Hi Marcel,
On Wed, Apr 12, 2017 at 10:19:21PM +0200, Marcel Holtmann wrote:
> Hi Sebastian,
>
> >>>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >>>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >>>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
> >>>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
> >>>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >>>>>>>> for the bluetooth subsystem.
> >>>>>>>
> >>>>>>> Greg doesn't read cover letters generally and since the serdev patches
> >>>>>>> are Cc rather than To him, he's probably not planning to pick them up.
> >>>>>>
> >>>>>> I wonder actually if we should merge all of these via bluetooth-next
> >>>>>> tree with proper Ack from Greg. However it would be good to also get
> >>>>>> buy in from Dave for merging this ultimately through net-next.
> >>>>>
> >>>>> I don't really care where it goes. I can take the whole thing in my
> >>>>> tty/serial tree now if no one objects and I get an ack from the relevant
> >>>>> maintainers {hint...}
> >>>>
> >>>> I think it is better if it goes thru BT tree. I have another driver
> >>>> converted that is dependent on this series. There's a couple other
> >>>> serdev changes on the list too, but this shouldn't depend on them.
> >>>
> >>> Is this waiting for something, or could it be queued to
> >>> bluetooth-next then? It would be nice to finally have
> >>> this in 4.12 :)
> >>
> >> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
> >
> > Sorry thought this was coming through mine:
> > Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> >
> > Merge away!
>
> so I have applied patches 1-8 to bluetooth-next tree.
>
> The last 2 I left out since they do cause build issues on non-DT
> platforms. We need to be able to build the driver on all platforms
> so that sanity compile checks happen all the time.
>
> CC drivers/bluetooth/hci_nokia.o
> drivers/bluetooth/hci_nokia.c:802:34: error: array type has incomplete element type ‘struct of_device_id’
> static const struct of_device_id nokia_bluetooth_of_match[] = {
> ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/bluetooth/hci_nokia.c:803:4: error: field name not in record or union initializer
> { .compatible = "nokia,h4p-bluetooth", },
> ^
> drivers/bluetooth/hci_nokia.c:803:4: note: (near initialization for ‘nokia_bluetooth_of_match’)
> drivers/bluetooth/hci_nokia.c:815:21: error: implicit declaration of function ‘of_match_ptr’ [-Werror=implicit-function-declaration]
> .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> ^~~~~~~~~~~~
> drivers/bluetooth/hci_nokia.c:802:34: warning: ‘nokia_bluetooth_of_match’ defined but not used [-Wunused-variable]
> static const struct of_device_id nokia_bluetooth_of_match[] = {
> ^~~~~~~~~~~~~~~~~~~~~~~~
Building without CONFIG_OF should work already. Note, that its
actually enabled in your build, since nokia_bluetooth_of_match
is guarded by "#ifdef CONFIG_OF". The actual problem is, that
<linux/of.h> is not included in your build. Looks like it was
implicitly included in my configurations, so I didn't notice.
I will send PATCHv4 with the added include and includes sorted
alphabetically.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* RE: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
From: Vikas MANOCHA @ 2017-04-13 0:03 UTC (permalink / raw)
To: Alexandre TORGUE, Patrice CHOTARD
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
Rob Herring, Russell King
In-Reply-To: <e6592982-9c24-bfde-469b-770218bf88be-qxv4g6HH51o@public.gmane.org>
Hi Alex,
> -----Original Message-----
> From: Alexandre TORGUE
> Sent: Tuesday, April 11, 2017 12:51 AM
> To: Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>; Patrice CHOTARD <patrice.chotard-qxv4g6HH51o@public.gmane.org>
> Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; moderated list:ARM PORT
> <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>; open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; Maxime
> Coquelin <mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Subject: Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
>
> Hi Vikas
>
> On 04/10/2017 08:40 PM, Vikas Manocha wrote:
> > Thanks Alex,
> >
> > On 04/10/2017 12:23 AM, Alexandre Torgue wrote:
> >> Hi
> >>
> >> On 04/08/2017 03:12 AM, Vikas Manocha wrote:
> >>> Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the
> >>> major spces of the two boards:
> >>>
> >>> stm32f769I discovery board:
> >>> - Cortex-M7 core @216MHz
> >>> - 2MB mcu internal flash
> >>> - 512KB internal sram
> >>> - 16MB sdram memory
> >>> - 64MB qspi flash memory
> >>> - 4 inch wvga LCD-TFT Display
> >>>
> >>> stm32f746 discovery board:
> >>> - Cortex-M7 core @216MHz
> >>> - 1MB mcu internal flash
> >>> - 320KB internal sram
> >>> - 8MB sdram memory
> >>> - 16MB qspi flash memory
> >>> - 4.3 inch 480x272 LCD-TFT display
> >>>
> >>> Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> ---
> >>> arch/arm/boot/dts/Makefile | 2 +
> >>> arch/arm/boot/dts/stm32f746-disco.dts | 101 ++++++++++++++++++++++++++++++++++
> >>> arch/arm/boot/dts/stm32f746.dtsi | 2 +-
> >>> arch/arm/boot/dts/stm32f769-disco.dts | 101
> >>> ++++++++++++++++++++++++++++++++++
> >>> 4 files changed, 205 insertions(+), 1 deletion(-) create mode
> >>> 100644 arch/arm/boot/dts/stm32f746-disco.dts
> >>> create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
> >>>
> >>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >>> index 0118084..a119f74 100644
> >>> --- a/arch/arm/boot/dts/Makefile
> >>> +++ b/arch/arm/boot/dts/Makefile
> >>> @@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
> >>> dtb-$(CONFIG_ARCH_STM32)+= \
> >>> stm32f429-disco.dtb \
> >>> stm32f469-disco.dtb \
> >>> + stm32f746-disco.dtb \
> >>> + stm32f769-disco.dtb \
> >>> stm32429i-eval.dtb \
> >>> stm32746g-eval.dtb
> >>> dtb-$(CONFIG_MACH_SUN4I) += \
> >>> diff --git a/arch/arm/boot/dts/stm32f746-disco.dts
> >>> b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> new file mode 100644
> >>> index 0000000..c0e313f
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + * a) This file is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU General Public License as
> >>> + * published by the Free Software Foundation; either version 2 of the
> >>> + * License, or (at your option) any later version.
> >>> + *
> >>> + * This file 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.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + * b) Permission is hereby granted, free of charge, to any person
> >>> + * obtaining a copy of this software and associated documentation
> >>> + * files (the "Software"), to deal in the Software without
> >>> + * restriction, including without limitation the rights to use,
> >>> + * copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + * sell copies of the Software, and to permit persons to whom the
> >>> + * Software is furnished to do so, subject to the following
> >>> + * conditions:
> >>> + *
> >>> + * The above copyright notice and this permission notice shall be
> >>> + * included in all copies or substantial portions of the Software.
> >>> + *
> >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + * OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> + model = "STMicroelectronics STM32F746-DISCO board";
> >>> + compatible = "st,stm32f746-disco", "st,stm32f746";
> >>> +
> >>> + chosen {
> >>> + bootargs = "root=/dev/ram";
> >>> + stdout-path = "serial0:115200n8";
> >>> + };
> >>> +
> >>> + memory {
> >>> + reg = <0xC0000000 0x800000>;
> >>> + };
> >>> +
> >>> + aliases {
> >>> + serial0 = &usart1;
> >>> + };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> + clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >>
> >> Pin muxing is not defined in board file. Please move it into SOC dtsi file.
Ok, I will move pin muxing in the stm32f746.dtsi file as per current implementation & send V2.
> >
> > Pin muxing used is different for different boards. e.g. usart1_rx pad is PA10 for stm32f769-disco board while it is PB7 for stm32f746-
> disco board.
> > The other possibilities for same pad (usart1_rx) is PB15. To make situation bit more complex, it is only available in f769 device.
> >
> > Putting in SOC dtsi file means having lot of combinations for different pins in separate groups.
> > e.g. only for one instance of one ip (usart1), following groups might be required at one point of time:
> >
> > usart1_pa10_pa9 {..}
> > usart1_pa10_pb14 {..}
> > usart1_pa10_pb6 {..}
> >
> > usart1_pb7_pa9 {..}
> > usart1_pb7_pb14 {..}
> > usart1_pb7_pb6 {..}
> >
> > usart1_pb15_pa9 {..}
> > usart1_pb15_pb14 {..}
> > usart1_pb15_pb6 {..}
> >
> > In case of boards based on stm32f746 device, all the above mentioned groups with pb14 & pb15 will not be available.
> > One solution (to avoid using not available groups) could be to have separate dtsi (or separate pinmux.dtsi) for different devices of
> same family like one for stm32f746 & other for stm32f769. Still it does not resolve the need to have lot of groups combinations for
> each instance of every peripheral in dtsi as mentioned above.
>
> Yes, it is what I want to have. I did the job on for STM32F4 (F429 / F469). You could have a look on ARM Linux patchwork:
> https://patchwork.kernel.org/patch/9669433/
>
> To sum-up the implementation:
>
> -Pinmuxing is defined in separate files which will be included in board dts file.
>
> -We have a common pinmuxing file + a dedicated pinmuxing file per SOC.
>
> Example for STMF469-disco:
> stm32f4-pinctrl.dtsi --> stm32f469-pinctrl.dtsi --> stm32f469-disco.dts
>
> stm32f4-pinctrl.dtsi contains common pinmuxing bindings between STM32F4 stm32f469-pinctrl.dtsi contains dedicated pinmuxing
> bindings for
> STM32F469 (ex: QSPI pins, gpio-ranges ...)
>
> This implementation is under review.
Sounds good.
Cheers,
Vikas
>
>
> > It seems cleaner solution would be pin muxing in board dts file. Please let me know if there is some drawback of this approach. One
> point which i can think of is : duplication of pinmux groups in different board dts files.
>
> Pinmuxing in board file is currently not my choice. In the board file we only select the group to use.
>
> Regards
> Alex
>
>
> >
> > Cheers,
> > Vikas
> >
> >>> + usart1_pins: usart1@0 {
> >>> + pins1 {
> >>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> + bias-disable;
> >>> + drive-push-pull;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + pins2 {
> >>> + pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
> >>> + bias-disable;
> >>> + };
> >>> + };
> >>> +
> >>> + qspi_pins: qspi@0 {
> >>> + pins {
> >>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> + <STM32F746_PD11_FUNC_QUADSPI_BK1_IO0>,
> >>> + <STM32F746_PD12_FUNC_QUADSPI_BK1_IO1>,
> >>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> + pinctrl-0 = <&usart1_pins>;
> >>> + pinctrl-names = "default";
> >>> + status = "okay";
> >>> +};
> >>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi
> >>> b/arch/arm/boot/dts/stm32f746.dtsi
> >>> index f321ffe..826700f 100644
> >>> --- a/arch/arm/boot/dts/stm32f746.dtsi
> >>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> >>> @@ -178,7 +178,7 @@
> >>> interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
> >>> };
> >>>
> >>> - pin-controller {
> >>> + pinctrl: pin-controller {
> >>> #address-cells = <1>;
> >>> #size-cells = <1>;
> >>> compatible = "st,stm32f746-pinctrl"; diff --git
> >>> a/arch/arm/boot/dts/stm32f769-disco.dts
> >>> b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> new file mode 100644
> >>> index 0000000..5f8558e
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + * a) This file is free software; you can redistribute it and/or
> >>> + * modify it under the terms of the GNU General Public License as
> >>> + * published by the Free Software Foundation; either version 2 of the
> >>> + * License, or (at your option) any later version.
> >>> + *
> >>> + * This file 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.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + * b) Permission is hereby granted, free of charge, to any person
> >>> + * obtaining a copy of this software and associated documentation
> >>> + * files (the "Software"), to deal in the Software without
> >>> + * restriction, including without limitation the rights to use,
> >>> + * copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + * sell copies of the Software, and to permit persons to whom the
> >>> + * Software is furnished to do so, subject to the following
> >>> + * conditions:
> >>> + *
> >>> + * The above copyright notice and this permission notice shall be
> >>> + * included in all copies or substantial portions of the Software.
> >>> + *
> >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + * OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> + model = "STMicroelectronics STM32F769-DISCO board";
> >>> + compatible = "st,stm32f769-disco", "st,stm32f7";
> >>> +
> >>> + chosen {
> >>> + bootargs = "root=/dev/ram";
> >>> + stdout-path = "serial0:115200n8";
> >>> + };
> >>> +
> >>> + memory {
> >>> + reg = <0xC0000000 0x1000000>;
> >>> + };
> >>> +
> >>> + aliases {
> >>> + serial0 = &usart1;
> >>> + };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> + clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >> same.
> >>
> >>> + usart1_pins: usart1@0 {
> >>> + pins1 {
> >>> + pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> + bias-disable;
> >>> + drive-push-pull;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + pins2 {
> >>> + pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
> >>> + bias-disable;
> >>> + };
> >>> + };
> >>> +
> >>> + qspi_pins: qspi@0 {
> >>> + pins {
> >>> + pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> + <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> + <STM32F746_PC9_FUNC_QUADSPI_BK1_IO0>,
> >>> + <STM32F746_PC10_FUNC_QUADSPI_BK1_IO1>,
> >>> + <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> + <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> + slew-rate = <2>;
> >>> + };
> >>> + };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> + pinctrl-0 = <&usart1_pins>;
> >>> + pinctrl-names = "default";
> >>> + status = "okay";
> >>> +};
> >>>
> >> .
> >>
--
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: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Florian Fainelli @ 2017-04-12 23:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <20170412221016.GA29708-g2DYL2Zd6BY@public.gmane.org>
On 04/12/2017 03:10 PM, Andrew Lunn wrote:
>>>>> To give some more background and rational for this change.
>>>>>
>>>>> On a platform where we have a parent MDIO bus, backed by the
>>>>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
>>>>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
>>>>> assignment of of_node. This slave MII bus is created in order to
>>>>> intercept reads/writes to problematic addresses (e.g: that clashes with
>>>>> another piece of hardware).
>>>>>
>>>>> This means that the slave DSA MII bus inherits all child nodes from the
>>>>> originating master MII bus. This also means that when the slave MII bus
>>>>> is probed via of_mdiobus_register(), we probe the same devices twice:
>>>>> once through the master, another time through the slave.
>>>>
>>>> Ah, O.K. This makes more sense. On the hardware i have, we get three
>>>> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
>>>> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
>>>> bus. And i've never seen issues.
>>>>
>>>> So your real problem here is you have two mdio busses using the same
>>>> device tree properties. I would actually say that is just plain
>>>> broken.
>>>
>>> From a Device Tree/HW representation perspective, we do have the
>>> external BCM53125 switch physically attached to the 7445/7278
>>> SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
>>> representation is correct. There is also an integrated Gigabit PHY
>>> (bcm7xxx) which is attached to that bus.
>
> This is made harder by you talking about a board which does not appear
> to have its DT file in mainline. So i'm having to guess what it looks
> like.
The DT binding is in tree and provides an example of how the switch
looks like, below is the example, but I am also adding the MDIO bus and
the PHYs just so you can see how things wind up:
switch_top@f0b00000 {
compatible = "simple-bus";
#size-cells = <1>;
#address-cells = <1>;
ranges = <0 0xf0b00000 0x40804>;
ethernet_switch@0 {
compatible = "brcm,bcm7445-switch-v4.0";
#size-cells = <0>;
#address-cells = <1>;
reg = <0x0 0x40000
0x40000 0x110
0x40340 0x30
0x40380 0x30
0x40400 0x34
0x40600 0x208>;
reg-names = "core", "reg", intrl2_0", "intrl2_1",
"fcb, "acb";
interrupts = <0 0x18 0
0 0x19 0>;
brcm,num-gphy = <1>;
brcm,num-rgmii-ports = <2>;
brcm,fcb-pause-override;
brcm,acb-packets-inflight;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
label = "gphy";
reg = <0>;
phy-handle = <&phy5>;
};
sw0port1: port@1 {
label = "rgmii_1";
reg = <1>;
phy-mode = "rgmii";
fixed-link {
speed = <1000>;
full-duplex;
};
}
};
};
mdio@403c0 {
reg = <0x403c0 0x8 0x40300 0x18>;
#address-cells = <0x1>;
#size-cells = <0x0>;
compatible = "brcm,unimac-mdio";
reg-names = "mdio", "mdio_indir_rw";
switch: switch@0 {
broken-turn-around;
reg = <0x0>;
compatible = "brcm,bcm53125";
#address-cells = <1>;
#size-cells = <0>;
ports {
..
port@8 {
ethernet = <&sw0port1>;
};
...
};
};
phy5: ethernet-phy@5 {
reg = <0x5>;
compatible = "ethernet-phy-ieee802.3-c22";
};
};
};
>
> So what i think we are talking about is this bit of code:
>
> static int bcm_sf2_mdio_register(struct dsa_switch *ds)
> {
> struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
> struct device_node *dn;
> static int index;
> int err;
>
> /* Find our integrated MDIO bus node */
> dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
> priv->master_mii_bus = of_mdio_find_bus(dn);
> if (!priv->master_mii_bus)
> return -EPROBE_DEFER;
>
> get_device(&priv->master_mii_bus->dev);
> priv->master_mii_dn = dn;
>
> priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
> if (!priv->slave_mii_bus)
> return -ENOMEM;
>
> priv->slave_mii_bus->priv = priv;
> priv->slave_mii_bus->name = "sf2 slave mii";
> priv->slave_mii_bus->read = bcm_sf2_sw_mdio_read;
> priv->slave_mii_bus->write = bcm_sf2_sw_mdio_write;
> snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
> index++);
> priv->slave_mii_bus->dev.of_node = dn;
>
> If i get you right, your switch is hanging off the MDIO bus
> "brcm,unimac-mdio" you find the dn for. You then register another MDIO
> bus using the exact same node? How does that make any sense? Isn't it
> a physical separate MDIO bus?
First, the main switch is memory mapped into the 7445 SoC's register
space. This switch has an external MDIO bus which connects to an
integrated Gigabit PHY at MDIO address 5, but also to a BCM53125 switch
at address 30.
Because of a bug in the D0 revision of the 7445, programming the
BCM53125 switch through MDIO ends-up programming the 7445 memory mapped
switch as well because the integrated 7445 switch has its pseudo-PHY
snooping accesses to the MDIO bus! What was done to work-around this is
to create a slave MII bus through DSA, and divert the reads/writes
from/to the BCM53125 by instead using internal 7445 switch registers
which isolate its pseudo-PHY from the MDIO bus, thus no double
programming anymore.
Since the BCM53125 switch is a) physically attached to the mdio@403c0
node, and b) needs to have visibility in the OF world for DSA to probe
it, this is what I did here.
The slave MII bus is using the same node here because that's the
simplest way to make this bus inherit the devices of interest from the
parent bus.
> So it should have its own set of nodes
> in the device tree. This is how we do it for the Marvell switches. See
> Documentation/devicetree/binding/net/dsa/marvell.txt and
> arch/arm/boot/dts/vf610-zii-dev-rev-b.dts. That DT blob uses
> phy-handle to link the switch ports to the phys on the mdio bus.
>From a pure HW representation, this is not quite correct, because the
switch is physically attached to mdio@403c0, but since we are
pathologically broken, we need something different here...
--
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
* [PATCH] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-12 23:50 UTC (permalink / raw)
To: alexandre.torgue-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o
Cc: Vikas Manocha,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
Rob Herring, Russell King
Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
specs of the two boards:
stm32f769I discovery board:
- Cortex-M7 core @216MHz
- 2MB mcu internal flash
- 512KB internal sram
- 16MB sdram memory
- 64MB qspi flash memory
- 4 inch wvga LCD-TFT Display
stm32f746 discovery board:
- Cortex-M7 core @216MHz
- 1MB mcu internal flash
- 320KB internal sram
- 8MB sdram memory
- 16MB qspi flash memory
- 4.3 inch 480x272 LCD-TFT display
Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
---
Changed in v2:
- moved pin muxing from board dts file to dtsi file.
arch/arm/boot/dts/Makefile | 2 +
arch/arm/boot/dts/stm32f746-disco.dts | 74 +++++++++++++++++++++++++++++++++++
arch/arm/boot/dts/stm32f746.dtsi | 12 ++++++
arch/arm/boot/dts/stm32f769-disco.dts | 74 +++++++++++++++++++++++++++++++++++
4 files changed, 162 insertions(+)
create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..a119f74 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
dtb-$(CONFIG_ARCH_STM32)+= \
stm32f429-disco.dtb \
stm32f469-disco.dtb \
+ stm32f746-disco.dtb \
+ stm32f769-disco.dtb \
stm32429i-eval.dtb \
stm32746g-eval.dtb
dtb-$(CONFIG_MACH_SUN4I) += \
diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
new file mode 100644
index 0000000..55afae9
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f746-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file 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.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+ model = "STMicroelectronics STM32F746-DISCO board";
+ compatible = "st,stm32f746-disco", "st,stm32f746";
+
+ chosen {
+ bootargs = "root=/dev/ram";
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0xC0000000 0x800000>;
+ };
+
+ aliases {
+ serial0 = &usart1;
+ };
+
+};
+
+&clk_hse {
+ clock-frequency = <25000000>;
+};
+
+&usart1 {
+ pinctrl-0 = <&usart1_pins_pa9_pa10>;
+ pinctrl-names = "default";
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..1064fb1 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,18 @@
bias-disable;
};
};
+ usart1_pins_pa9_pa10: usart1@1 {
+ pins1 {
+ pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
+ bias-disable;
+ drive-push-pull;
+ slew-rate = <0>;
+ };
+ pins2 {
+ pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
+ bias-disable;
+ };
+ };
};
rcc: rcc@40023800 {
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
new file mode 100644
index 0000000..166728a
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file 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.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+ model = "STMicroelectronics STM32F769-DISCO board";
+ compatible = "st,stm32f769-disco", "st,stm32f7";
+
+ chosen {
+ bootargs = "root=/dev/ram";
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0xC0000000 0x1000000>;
+ };
+
+ aliases {
+ serial0 = &usart1;
+ };
+
+};
+
+&clk_hse {
+ clock-frequency = <25000000>;
+};
+
+&usart1 {
+ pinctrl-0 = <&usart1_pins_a>;
+ pinctrl-names = "default";
+ status = "okay";
+};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Andrew Lunn @ 2017-04-12 22:10 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <417fdaa1-7aba-e646-8a50-043322f1410d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> To give some more background and rational for this change.
> >>>
> >>> On a platform where we have a parent MDIO bus, backed by the
> >>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
> >>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
> >>> assignment of of_node. This slave MII bus is created in order to
> >>> intercept reads/writes to problematic addresses (e.g: that clashes with
> >>> another piece of hardware).
> >>>
> >>> This means that the slave DSA MII bus inherits all child nodes from the
> >>> originating master MII bus. This also means that when the slave MII bus
> >>> is probed via of_mdiobus_register(), we probe the same devices twice:
> >>> once through the master, another time through the slave.
> >>
> >> Ah, O.K. This makes more sense. On the hardware i have, we get three
> >> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
> >> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
> >> bus. And i've never seen issues.
> >>
> >> So your real problem here is you have two mdio busses using the same
> >> device tree properties. I would actually say that is just plain
> >> broken.
> >
> > From a Device Tree/HW representation perspective, we do have the
> > external BCM53125 switch physically attached to the 7445/7278
> > SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
> > representation is correct. There is also an integrated Gigabit PHY
> > (bcm7xxx) which is attached to that bus.
This is made harder by you talking about a board which does not appear
to have its DT file in mainline. So i'm having to guess what it looks
like.
So what i think we are talking about is this bit of code:
static int bcm_sf2_mdio_register(struct dsa_switch *ds)
{
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
struct device_node *dn;
static int index;
int err;
/* Find our integrated MDIO bus node */
dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
priv->master_mii_bus = of_mdio_find_bus(dn);
if (!priv->master_mii_bus)
return -EPROBE_DEFER;
get_device(&priv->master_mii_bus->dev);
priv->master_mii_dn = dn;
priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
if (!priv->slave_mii_bus)
return -ENOMEM;
priv->slave_mii_bus->priv = priv;
priv->slave_mii_bus->name = "sf2 slave mii";
priv->slave_mii_bus->read = bcm_sf2_sw_mdio_read;
priv->slave_mii_bus->write = bcm_sf2_sw_mdio_write;
snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
index++);
priv->slave_mii_bus->dev.of_node = dn;
If i get you right, your switch is hanging off the MDIO bus
"brcm,unimac-mdio" you find the dn for. You then register another MDIO
bus using the exact same node? How does that make any sense? Isn't it
a physical separate MDIO bus? So it should have its own set of nodes
in the device tree. This is how we do it for the Marvell switches. See
Documentation/devicetree/binding/net/dsa/marvell.txt and
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts. That DT blob uses
phy-handle to link the switch ports to the phys on the mdio bus.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Florian Fainelli @ 2017-04-12 21:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
Rob Herring, Frank Rowand,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <6de72d4c-5634-f3d6-5bfd-fcc0acda0b83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 04/11/2017 04:23 PM, Florian Fainelli wrote:
> On 04/11/2017 04:14 PM, Andrew Lunn wrote:
>>> To give some more background and rational for this change.
>>>
>>> On a platform where we have a parent MDIO bus, backed by the
>>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
>>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
>>> assignment of of_node. This slave MII bus is created in order to
>>> intercept reads/writes to problematic addresses (e.g: that clashes with
>>> another piece of hardware).
>>>
>>> This means that the slave DSA MII bus inherits all child nodes from the
>>> originating master MII bus. This also means that when the slave MII bus
>>> is probed via of_mdiobus_register(), we probe the same devices twice:
>>> once through the master, another time through the slave.
>>
>> Ah, O.K. This makes more sense. On the hardware i have, we get three
>> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
>> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
>> bus. And i've never seen issues.
>>
>> So your real problem here is you have two mdio busses using the same
>> device tree properties. I would actually say that is just plain
>> broken.
>
> From a Device Tree/HW representation perspective, we do have the
> external BCM53125 switch physically attached to the 7445/7278
> SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
> representation is correct. There is also an integrated Gigabit PHY
> (bcm7xxx) which is attached to that bus.
>
> From a SW perspective though, we want to talk to the integrated Gigabit
> PHY using mdio-bcm-unimac but talk to the external BCM53125 switch using
> the slave MII bus created by the bcm_sf2 driver in order to create an
> isolation. We need to inherit some of the parent (mdio-bcm-unimac) child
> DT nodes (such as the BCM53125), but not the GPHY. The easiest solution
> I found was to use this patch.
>
> Using mdiobus_register() instead of of_mdiobus_register() was
> considered, but then, the child BCM53125 has no more "visbility" into
> the OF world at all, and it matters, because this switch is also driven
> via a DSA switch driver and its Ethernet data-path is connected to one
> port of the bcm_sf2 switch..
>
> Thankfully the HW bug was fixed eventually ;)
In fact, all I need is to flag the internal Gigabit PHY for the slave
MII bus node with something that makes it appear as "disabled" which I
can presumably do with of_update_property() and putting a status =
"disabled" property in there. Let me do something like that and see how
big of a hack this becomes.
--
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 v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Cyrille Pitchen @ 2017-04-12 20:57 UTC (permalink / raw)
To: Guochun Mao, Cyrille Pitchen
Cc: Mark Rutland, devicetree, Richard Weinberger, Russell King,
linux-kernel, Rob Herring, linux-mtd, Matthias Brugger,
linux-mediatek, David Woodhouse, linux-arm-kernel
In-Reply-To: <1491381462-21893-2-git-send-email-guochun.mao@mediatek.com>
Hi Guochun,
Le 05/04/2017 à 10:37, Guochun Mao a écrit :
> When nor's size larger than 16MByte, nor's address width maybe
> set to 3 or 4, and controller should change address width according
> to nor's setting.
>
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
> drivers/mtd/spi-nor/mtk-quadspi.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> index e661877..b637770 100644
> --- a/drivers/mtd/spi-nor/mtk-quadspi.c
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -104,6 +104,8 @@
> #define MTK_NOR_MAX_RX_TX_SHIFT 6
> /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
> #define MTK_NOR_MAX_SHIFT 7
> +/* nor controller 4-byte address mode enable bit */
> +#define MTK_NOR_4B_ADDR_EN BIT(4)
>
> /* Helpers for accessing the program data / shift data registers */
> #define MTK_NOR_PRG_REG(n) (MTK_NOR_PRGDATA0_REG + 4 * (n))
> @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
> 10000);
> }
>
> +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
> +{
> + u8 val;
> + struct spi_nor *nor = &mt8173_nor->nor;
> +
> + val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
> +
> + switch (nor->addr_width) {
> + case 3:
> + val &= ~MTK_NOR_4B_ADDR_EN;
> + break;
> + case 4:
> + val |= MTK_NOR_4B_ADDR_EN;
> + break;
> + default:
> + dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
> + nor->addr_width);
> + break;
> + }
> +
> + writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
> +}
> +
> static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
> {
> int i;
>
> + mt8173_nor_set_addr_width(mt8173_nor);
> +
> for (i = 0; i < 3; i++) {
Should it be 'i < nor->addr_width' instead of 'i < 3' ?
Does it work when accessing data after 128Mbit ?
Best regards,
Cyrille
> writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
> addr >>= 8;
>
^ permalink raw reply
* Re: [PATCH v2 2/3] net: macb: Remove CONFIG_OF around DT match table
From: Rob Herring @ 2017-04-12 20:51 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel@vger.kernel.org, Andrew Lunn, Vivien Didelot,
Lee Jones, Nicolas Ferre, Frank Rowand,
open list:NETWORKING DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-3-f.fainelli@gmail.com>
On Tue, Apr 11, 2017 at 11:41 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> A subsequent patch is going to make of_match_node() an inline stub when
> CONFIG_OF is disabled, which will let the compiler eliminate unused variables.
> In order not to clutter the code more, remove the CONFIG_OF #ifdef such that
> macb_dt_ids and what it references are always defined.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 30606b11b128..01016e9525ee 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2811,7 +2811,6 @@ static int macb_init(struct platform_device *pdev)
> return 0;
> }
>
> -#if defined(CONFIG_OF)
> /* 1518 rounded up */
> #define AT91ETHER_MAX_RBUFF_SZ 0x600
> /* max number of receive buffers */
> @@ -3215,7 +3214,6 @@ static const struct of_device_id macb_dt_ids[] = {
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);
Did you actually check that the compiler can eliminate things when
modules are enabled? Because it's not going to be able to eliminate
MODULE_DEVICE_TABLE entry AFAICT.
Rob
^ permalink raw reply
* Re: [PATCH v3 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Cyrille Pitchen @ 2017-04-12 20:28 UTC (permalink / raw)
To: Ludovic Barre, Cyrille Pitchen, Marek Vasut
Cc: Boris Brezillon, Alexandre Torgue,
devicetree-u79uwXL29TY76Z2rM5mHXA, Richard Weinberger,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
David Woodhouse
In-Reply-To: <1492016785-24976-3-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>
Hi Ludovic,
Globally I will say this is pretty good and, IMHO, almost ready to be
merged into the github/spi-nor tree.
I have few comments, all but one are mainly recommendations hence not
blocking on my side. However one should be fixed. Even if not critical
and very unlikely to happen, I think it is more a potential bug than a
simple cosmetic issue.
So if you have time to prepare a new series, I will try if possible to
add your patch into the PR, which should be sent ideally before the end
of the week. I can't give you any guarantee since the timing is really
short now. Besides Marek still needs to add his Acked-by / Reviewed-by
once satisfied, otherwise your patch will have to wait for the next release.
If something I say is wrong or if you disagree with me, do not hesitate:
we can discuss and I can change my mind.
So my comments to follow:
Le 12/04/2017 à 19:06, Ludovic Barre a écrit :
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
>
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
> registers
> -read memory-mapped mode: the external Flash memory is mapped to the
> microcontroller address space and is seen by the system as if it was
> an internal memory
>
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> ---
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/stm32-quadspi.c | 693 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 701 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 7252087..bfdfb1e 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
> To compile this driver as a module, choose M here: the module
> will be called intel-spi-platform.
>
> +config SPI_STM32_QUADSPI
> + tristate "STM32 Quad SPI controller"
> + depends on ARCH_STM32
> + help
> + This enables support for the STM32 Quad SPI controller.
> + We only connect the NOR to this controller.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 72238a7..285aab8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> obj-$(CONFIG_SPI_INTEL_SPI) += intel-spi.o
> obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM) += intel-spi-platform.o
> +obj-$(CONFIG_SPI_STM32_QUADSPI) += stm32-quadspi.o
> \ No newline at end of file
> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
> new file mode 100644
> index 0000000..9e90dee
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c
> @@ -0,0 +1,693 @@
> +/*
> + * stm32_quadspi.c
> + *
> + * Copyright (C) 2017, Ludovic Barre
> + *
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +#include <linux/clk.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#define QUADSPI_CR 0x00
> +#define CR_EN BIT(0)
> +#define CR_ABORT BIT(1)
> +#define CR_DMAEN BIT(2)
> +#define CR_TCEN BIT(3)
> +#define CR_SSHIFT BIT(4)
> +#define CR_DFM BIT(6)
> +#define CR_FSEL BIT(7)
> +#define CR_FTHRES_SHIFT 8
> +#define CR_FTHRES_MASK GENMASK(12, 8)
> +#define CR_FTHRES(n) (((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
> +#define CR_TEIE BIT(16)
> +#define CR_TCIE BIT(17)
> +#define CR_FTIE BIT(18)
> +#define CR_SMIE BIT(19)
> +#define CR_TOIE BIT(20)
> +#define CR_PRESC_SHIFT 24
> +#define CR_PRESC_MASK GENMASK(31, 24)
> +#define CR_PRESC(n) (((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
> +
> +#define QUADSPI_DCR 0x04
> +#define DCR_CSHT_SHIFT 8
> +#define DCR_CSHT_MASK GENMASK(10, 8)
> +#define DCR_CSHT(n) (((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
> +#define DCR_FSIZE_SHIFT 16
> +#define DCR_FSIZE_MASK GENMASK(20, 16)
> +#define DCR_FSIZE(n) (((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
> +
> +#define QUADSPI_SR 0x08
> +#define SR_TEF BIT(0)
> +#define SR_TCF BIT(1)
> +#define SR_FTF BIT(2)
> +#define SR_SMF BIT(3)
> +#define SR_TOF BIT(4)
> +#define SR_BUSY BIT(5)
> +#define SR_FLEVEL_SHIFT 8
> +#define SR_FLEVEL_MASK GENMASK(13, 8)
> +
> +#define QUADSPI_FCR 0x0c
> +#define FCR_CTCF BIT(1)
> +
> +#define QUADSPI_DLR 0x10
> +
> +#define QUADSPI_CCR 0x14
> +#define CCR_INST_SHIFT 0
> +#define CCR_INST_MASK GENMASK(7, 0)
> +#define CCR_INST(n) (((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
> +#define CCR_IMODE_NONE (0 << 8)
> +#define CCR_IMODE_1 (1 << 8)
> +#define CCR_IMODE_2 (2 << 8)
> +#define CCR_IMODE_4 (3 << 8)
> +#define CCR_ADMODE_NONE (0 << 10)
> +#define CCR_ADMODE_1 (1 << 10)
> +#define CCR_ADMODE_2 (2 << 10)
> +#define CCR_ADMODE_4 (3 << 10)
For bitfields, it would have been better to use (1u << 8) that is
unsigned int instead of signed int. It might avoid some warnings or even
bugs depending on how you use those macros, especially if you were using
BIT(31). The GENMASK() macro generates an unsigned int on its side.
However, I guess you have tested your driver so if you have noticed any
issue, I will say this is not blocking for me :)
You don't use BIT(31) and I don't see any use of the >> operator so I
think it should be safe. My comment is just a recommendation for the
next time!
To be fair, you can point out the atmel_qspi.c driver and I did the same
mistake but like you, I didn't use the >> operator with the bitmask I
have defined :p
> +#define CCR_ADSIZE_SHIFT 12
> +#define CCR_ADSIZE_MASK GENMASK(13, 12)
> +#define CCR_ADSIZE(n) (((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
> +#define CCR_ABMODE_NONE (0 << 14)
> +#define CCR_ABMODE_1 (1 << 14)
> +#define CCR_ABMODE_2 (2 << 14)
> +#define CCR_ABMODE_4 (3 << 14)
> +#define CCR_ABSIZE_8 (0 << 16)
> +#define CCR_ABSIZE_16 (1 << 16)
> +#define CCR_ABSIZE_24 (2 << 16)
> +#define CCR_ABSIZE_32 (3 << 16)
> +#define CCR_DCYC_SHIFT 18
> +#define CCR_DCYC_MASK GENMASK(22, 18)
> +#define CCR_DCYC(n) (((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
> +#define CCR_DMODE_NONE (0 << 24)
> +#define CCR_DMODE_1 (1 << 24)
> +#define CCR_DMODE_2 (2 << 24)
> +#define CCR_DMODE_4 (3 << 24)
> +#define CCR_FMODE_INDW (0 << 26)
> +#define CCR_FMODE_INDR (1 << 26)
> +#define CCR_FMODE_APM (2 << 26)
> +#define CCR_FMODE_MM (3 << 26)
> +
> +#define QUADSPI_AR 0x18
> +#define QUADSPI_ABR 0x1c
> +#define QUADSPI_DR 0x20
> +#define QUADSPI_PSMKR 0x24
> +#define QUADSPI_PSMAR 0x28
> +#define QUADSPI_PIR 0x2c
> +#define QUADSPI_LPTR 0x30
> +#define LPTR_DFT_TIMEOUT 0x10
> +
> +#define FSIZE_VAL(size) (__fls(size) - 1)
> +
> +#define STM32_MAX_MMAP_SZ SZ_256M
> +#define STM32_MAX_NORCHIP 2
> +
> +#define STM32_QSPI_FIFO_TIMEOUT_US 30000
> +#define STM32_QSPI_BUSY_TIMEOUT_US 100000
> +
> +struct stm32_qspi_flash {
> + struct spi_nor nor;
> + u32 cs;
> + u32 fsize;
> + u32 presc;
> + u32 read_mode;
> + struct stm32_qspi *qspi;
> +};
> +
> +struct stm32_qspi {
> + struct device *dev;
> + void __iomem *io_base;
> + void __iomem *mm_base;
> + resource_size_t mm_size;
> + u32 nor_num;
> + struct clk *clk;
> + u32 clk_rate;
> + struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
> + struct completion cmd_completion;
> +
> + /*
> + * to protect device configuration, could be different between
> + * 2 flash access (bk1, bk2)
> + */
> + struct mutex lock;
> +};
> +
> +struct stm32_qspi_cmd {
> + u8 addr_width;
> + u8 dummy;
> + bool tx_data;
> + u8 opcode;
> + u32 framemode;
> + u32 qspimode;
> + u32 addr;
> + size_t len;
> + void *buf;
> +};
> +
> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
> +{
> + u32 cr;
> + int err = 0;
> +
> + if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
> + return 0;
> +
> + reinit_completion(&qspi->cmd_completion);
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
> +
> + if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
> + msecs_to_jiffies(1000)))
> + err = -ETIMEDOUT;
> +
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> + return err;
> +}
> +
> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
> +{
> + u32 sr;
> +
> + return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
> + !(sr & SR_BUSY), 10,
> + STM32_QSPI_BUSY_TIMEOUT_US);
> +}
> +
> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
> + struct stm32_qspi_cmd *cmd, bool read)
> +{
> + u32 dmode = CCR_DMODE_1;
> +
> + cmd->framemode = CCR_IMODE_1;
> +
> + if (read) {
> + switch (nor->flash_read) {
> + case SPI_NOR_NORMAL:
> + case SPI_NOR_FAST:
> + dmode = CCR_DMODE_1;
> + break;
> + case SPI_NOR_DUAL:
> + dmode = CCR_DMODE_2;
> + break;
> + case SPI_NOR_QUAD:
> + dmode = CCR_DMODE_4;
> + break;
> + }
> + }
> +
> + cmd->framemode |= cmd->tx_data ? dmode : 0;
> + cmd->framemode |= cmd->addr_width ? CCR_ADMODE_1 : 0;
> +}
> +
> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
> +{
> + *val = readb_relaxed(addr);
> +}
> +
> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
> +{
> + writeb_relaxed(*val, addr);
> +}
> +
> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + void (*tx_fifo)(u8 *, void __iomem *);
> + u32 len = cmd->len, sr;
> + u8 *buf = cmd->buf;
> + int ret;
> +
> + if (cmd->qspimode == CCR_FMODE_INDW)
> + tx_fifo = stm32_qspi_write_fifo;
> + else
> + tx_fifo = stm32_qspi_read_fifo;
> +
> + while (len--) {
> + ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
> + sr, (sr & SR_FTF), 10,
> + STM32_QSPI_FIFO_TIMEOUT_US);
> + if (ret) {
> + dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
> + break;
> + }
> + tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
> + }
> +
> + return ret;
> +}
> +
> +static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
> + return 0;
> +}
> +
> +static int stm32_qspi_tx(struct stm32_qspi *qspi,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + if (!cmd->tx_data)
> + return 0;
> +
> + if (cmd->qspimode == CCR_FMODE_MM)
> + return stm32_qspi_tx_mm(qspi, cmd);
> +
> + return stm32_qspi_tx_poll(qspi, cmd);
> +}
> +
> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
> + const struct stm32_qspi_cmd *cmd)
> +{
> + struct stm32_qspi *qspi = flash->qspi;
> + u32 ccr, dcr, cr;
> + int err;
> +
> + err = stm32_qspi_wait_nobusy(qspi);
> + if (err)
> + goto abort;
> +
> + dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
> + dcr |= DCR_FSIZE(flash->fsize);
> + writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
> +
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + cr &= ~CR_PRESC_MASK & ~CR_FSEL;
> + cr |= CR_PRESC(flash->presc);
> + cr |= flash->cs ? CR_FSEL : 0;
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> + if (cmd->tx_data)
> + writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
> +
> + ccr = cmd->framemode | cmd->qspimode;
> +
> + if (cmd->dummy)
> + ccr |= CCR_DCYC(cmd->dummy);
> +
> + if (cmd->addr_width)
> + ccr |= CCR_ADSIZE(cmd->addr_width - 1);
> +
> + ccr |= CCR_INST(cmd->opcode);
> + writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
> +
> + if (cmd->addr_width && cmd->qspimode != CCR_FMODE_MM)
> + writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
> +
> + err = stm32_qspi_tx(qspi, cmd);
> + if (err)
> + goto abort;
> +
> + if (cmd->qspimode != CCR_FMODE_MM) {
> + err = stm32_qspi_wait_cmd(qspi);
> + if (err)
> + goto abort;
> + writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
> + }
> +
> + return err;
> +
> +abort:
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> + dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
> + return err;
> +}
> +
> +static int stm32_qspi_read_reg(struct spi_nor *nor,
> + u8 opcode, u8 *buf, int len)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "read_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = opcode;
> + cmd.tx_data = true;
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = CCR_FMODE_INDR;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "write_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = opcode;
> + cmd.tx_data = !!(buf && len > 0);
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
> + u_char *buf)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> + struct stm32_qspi_cmd cmd;
> + int err;
> +
> + dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
> + nor->read_opcode, buf, (u32)from, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->read_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)from;
> + cmd.tx_data = true;
> + cmd.dummy = nor->read_dummy;
> + cmd.len = len;
> + cmd.buf = buf;
> + cmd.qspimode = flash->read_mode;
> +
> + stm32_qspi_set_framemode(nor, &cmd, true);
> + err = stm32_qspi_send(flash, &cmd);
> +
> + return err ? err : len;
> +}
> +
> +static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
> + const u_char *buf)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> + int err;
> +
> + dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
> + nor->program_opcode, buf, (u32)to, len);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->program_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)to;
> + cmd.tx_data = true;
> + cmd.len = len;
> + cmd.buf = (void *)buf;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> + err = stm32_qspi_send(flash, &cmd);
> +
> + return err ? err : len;
> +}
> +
> +static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct device *dev = flash->qspi->dev;
> + struct stm32_qspi_cmd cmd;
> +
> + dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
> +
> + memset(&cmd, 0, sizeof(cmd));
> + cmd.opcode = nor->erase_opcode;
> + cmd.addr_width = nor->addr_width;
> + cmd.addr = (u32)offs;
> + cmd.qspimode = CCR_FMODE_INDW;
> +
> + stm32_qspi_set_framemode(nor, &cmd, false);
> +
> + return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
> +{
> + struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
> + u32 cr, sr, fcr = 0;
> +
> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> + sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
> +
> + if ((cr & CR_TCIE) && (sr & SR_TCF)) {
> + /* tx complete */
> + fcr |= FCR_CTCF;
> + complete(&qspi->cmd_completion);
> + } else {
> + dev_info_ratelimited(qspi->dev, "spurious interrupt\n");
> + }
> +
> + writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> +
> + mutex_lock(&qspi->lock);
> + return 0;
> +}
> +
> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct stm32_qspi_flash *flash = nor->priv;
> + struct stm32_qspi *qspi = flash->qspi;
> +
> + mutex_unlock(&qspi->lock);
> +}
> +
> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
> + struct device_node *np)
> +{
> + u32 width, flash_read, presc, cs_num, max_rate = 0;
> + struct stm32_qspi_flash *flash;
> + struct mtd_info *mtd;
> + int ret;
> +
> + of_property_read_u32(np, "reg", &cs_num);
> + if (cs_num >= STM32_MAX_NORCHIP)
> + return -EINVAL;
> +
> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
> + if (!max_rate)
> + return -EINVAL;
> +
> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
> +
> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
> + width = 1;
> +
> + if (width == 4)
> + flash_read = SPI_NOR_QUAD;
> + else if (width == 2)
> + flash_read = SPI_NOR_DUAL;
> + else if (width == 1)
> + flash_read = SPI_NOR_NORMAL;
> + else
> + return -EINVAL;
> +
> + flash = &qspi->flash[cs_num];
> + flash->qspi = qspi;
> + flash->cs = cs_num;
> + flash->presc = presc;
> +
> + flash->nor.dev = qspi->dev;
> + spi_nor_set_flash_node(&flash->nor, np);
> + flash->nor.priv = flash;
> + mtd = &flash->nor.mtd;
> + mtd->priv = &flash->nor;
This line is not needed: spi_nor_scan() already initializes mtd->priv
with the nor pointer. Not blocking anyway.
> +
> + flash->nor.read = stm32_qspi_read;
> + flash->nor.write = stm32_qspi_write;
> + flash->nor.erase = stm32_qspi_erase;
> + flash->nor.read_reg = stm32_qspi_read_reg;
> + flash->nor.write_reg = stm32_qspi_write_reg;
> + flash->nor.prepare = stm32_qspi_prep;
> + flash->nor.unprepare = stm32_qspi_unprep;
A shared comment: your implementations of all those handlers seem good
to me. I didn't notice any particular mistake or issue :)
> +
> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> + | CR_EN, qspi->io_base + QUADSPI_CR);
> +
> + /*
> + * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> + * which define the size of nor flash.
> + * if fsize is NULL, the controller can't sent spi-nor command.
> + * set a temporary value just to discover the nor flash with
> + * "spi_nor_scan". After, the right value (mtd->size) can be set.
> + */
> + flash->fsize = FSIZE_VAL(SZ_1K);
> +
> + ret = spi_nor_scan(&flash->nor, NULL, flash_read);
> + if (ret) {
> + dev_err(qspi->dev, "device scan failed\n");
At this point, (flash->qspi != NULL)
Hence in the control flow, once you have returned from
stm32_qspi__flash_setup() back into stm32_qspi_probe(), you handle the
reported error calling stm32_qspi_mtd_free().
This latest function test flash->qspi, which is not NULL, before calling
mtd_device_unregister(). However you did not register flash->nor.mtd at all.
Though not critical, can you fix this anyway, please?
I think, it's more a potential bug than a costmetic issue.
> + return ret;
> + }
> +
> + flash->fsize = FSIZE_VAL(mtd->size);
> +
> + flash->read_mode = CCR_FMODE_MM;
> + if (mtd->size > qspi->mm_size)
> + flash->read_mode = CCR_FMODE_INDR;
> +
> + writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret) {
> + dev_err(qspi->dev, "mtd device parse failed\n");
Almost the same mistake here: you shouldn't call mtd_device_unregister()
later from stm32_qspi_mtd_free() if mtd_devire_register() fails here.
> + return ret;
> + }
> +
> + dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> + flash->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> + return 0;
> +}
> +
> +static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
> +{
> + int i;
> +
> + for (i = 0; i < STM32_MAX_NORCHIP; i++) {
> + if (qspi->flash[i].qspi)
> + mtd_device_unregister(&qspi->flash[i].nor.mtd);
> + }
IMHO, I guess braces {} in the for() loop are not needed here. So if
checkpatch doesn't complain it's ok for me, you can leave it as is if
you want. Otherwise, please fix!
> +}
> +
> +static int stm32_qspi_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *flash_np;
> + struct reset_control *rstc;
> + struct stm32_qspi *qspi;
> + struct resource *res;
> + int ret, irq;
> +
> + qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
> + if (!qspi)
> + return -ENOMEM;
> +
> + qspi->nor_num = of_get_child_count(dev->of_node);
> + if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
> + return -ENODEV;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
> + qspi->io_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(qspi->io_base))
> + return PTR_ERR(qspi->io_base);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
> + qspi->mm_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(qspi->mm_base))
> + return PTR_ERR(qspi->mm_base);
> +
> + qspi->mm_size = resource_size(res);
> +
> + irq = platform_get_irq(pdev, 0);
> + ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
> + dev_name(dev), qspi);
> + if (ret) {
> + dev_err(dev, "failed to request irq\n");
> + return ret;
> + }
> +
> + init_completion(&qspi->cmd_completion);
> +
> + qspi->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(qspi->clk))
> + return PTR_ERR(qspi->clk);
> +
> + qspi->clk_rate = clk_get_rate(qspi->clk);
> + if (!qspi->clk_rate)
> + return -EINVAL;
> +
> + ret = clk_prepare_enable(qspi->clk);
> + if (ret) {
> + dev_err(dev, "can not enable the clock\n");
> + return ret;
> + }
> +
> + rstc = devm_reset_control_get(dev, NULL);
> + if (!IS_ERR(rstc)) {
> + reset_control_assert(rstc);
> + udelay(2);
> + reset_control_deassert(rstc);
> + }
> +
> + qspi->dev = dev;
> + platform_set_drvdata(pdev, qspi);
> + mutex_init(&qspi->lock);
> +
> + for_each_available_child_of_node(dev->of_node, flash_np) {
> + ret = stm32_qspi_flash_setup(qspi, flash_np);
> + if (ret) {
> + dev_err(dev, "unable to setup flash chip\n");
> + goto err_flash;
> + }
> + }
> +
> + return 0;
> +
> +err_flash:
> + mutex_destroy(&qspi->lock);
> + stm32_qspi_mtd_free(qspi);
> +
> + clk_disable_unprepare(qspi->clk);
> + return ret;
> +}
> +
> +static int stm32_qspi_remove(struct platform_device *pdev)
> +{
> + struct stm32_qspi *qspi = platform_get_drvdata(pdev);
> +
> + /* disable qspi */
> + writel_relaxed(0, qspi->io_base + QUADSPI_CR);
> +
> + stm32_qspi_mtd_free(qspi);
> + mutex_destroy(&qspi->lock);
> +
> + clk_disable_unprepare(qspi->clk);
> + return 0;
> +}
> +
> +static const struct of_device_id stm32_qspi_match[] = {
> + {.compatible = "st,stm32f469-qspi"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, stm32_qspi_match);
> +
> +static struct platform_driver stm32_qspi_driver = {
> + .probe = stm32_qspi_probe,
> + .remove = stm32_qspi_remove,
> + .driver = {
> + .name = "stm32-quadspi",
> + .of_match_table = stm32_qspi_match,
> + },
> +};
> +module_platform_driver(stm32_qspi_driver);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_AUTHOR("Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
> +MODULE_LICENSE("GPL v2");
>
Best regards,
Cyrille
--
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/4] bluetooth: hci_uart: add LL protocol serdev driver support
From: Marcel Holtmann @ 2017-04-12 20:20 UTC (permalink / raw)
To: Rob Herring
Cc: linux-bluetooth, linux-arm-kernel, Gustavo F. Padovan,
Johan Hedberg, Mark Rutland, Wei Xu, Eyal Reizer, Satish Patel,
netdev, devicetree
In-Reply-To: <20170407143516.9945-1-robh@kernel.org>
Hi Rob,
> Turns out that the LL protocol and the TI-ST are the same thing AFAICT.
> The TI-ST adds firmware loading, GPIO control, and shared access for
> NFC, FM radio, etc. For now, we're only implementing what is needed for
> BT. This mirrors other drivers like BCM and Intel, but uses the new
> serdev bus.
>
> The firmware loading is greatly simplified by using existing
> infrastructure to send commands. It may be a bit slower than the
> original code using synchronous functions, but the real bottleneck is
> likely doing firmware load at 115.2kbps.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org
> ---
> v2:
> - Use IS_ENABLED() to fix module build
>
> drivers/bluetooth/hci_ll.c | 261 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 260 insertions(+), 1 deletion(-)
can you re-send any missing patch on top of today's bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Marcel Holtmann @ 2017-04-12 20:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Rob Herring
In-Reply-To: <20170411140637.GA4388-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Hi Sebastian,
>>>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>>>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>>>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
>>>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
>>>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
>>>>>>>> for the bluetooth subsystem.
>>>>>>>
>>>>>>> Greg doesn't read cover letters generally and since the serdev patches
>>>>>>> are Cc rather than To him, he's probably not planning to pick them up.
>>>>>>
>>>>>> I wonder actually if we should merge all of these via bluetooth-next
>>>>>> tree with proper Ack from Greg. However it would be good to also get
>>>>>> buy in from Dave for merging this ultimately through net-next.
>>>>>
>>>>> I don't really care where it goes. I can take the whole thing in my
>>>>> tty/serial tree now if no one objects and I get an ack from the relevant
>>>>> maintainers {hint...}
>>>>
>>>> I think it is better if it goes thru BT tree. I have another driver
>>>> converted that is dependent on this series. There's a couple other
>>>> serdev changes on the list too, but this shouldn't depend on them.
>>>
>>> Is this waiting for something, or could it be queued to
>>> bluetooth-next then? It would be nice to finally have
>>> this in 4.12 :)
>>
>> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
>
> Sorry thought this was coming through mine:
> Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
>
> Merge away!
so I have applied patches 1-8 to bluetooth-next tree.
The last 2 I left out since they do cause build issues on non-DT platforms. We need to be able to build the driver on all platforms so that sanity compile checks happen all the time.
CC drivers/bluetooth/hci_nokia.o
drivers/bluetooth/hci_nokia.c:802:34: error: array type has incomplete element type ‘struct of_device_id’
static const struct of_device_id nokia_bluetooth_of_match[] = {
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:803:4: error: field name not in record or union initializer
{ .compatible = "nokia,h4p-bluetooth", },
^
drivers/bluetooth/hci_nokia.c:803:4: note: (near initialization for ‘nokia_bluetooth_of_match’)
drivers/bluetooth/hci_nokia.c:815:21: error: implicit declaration of function ‘of_match_ptr’ [-Werror=implicit-function-declaration]
.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
^~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:802:34: warning: ‘nokia_bluetooth_of_match’ defined but not used [-Wunused-variable]
static const struct of_device_id nokia_bluetooth_of_match[] = {
^~~~~~~~~~~~~~~~~~~~~~~~
Regards
Marcel
--
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 04/10] drm/stm: Add STM32 LTDC driver
From: Benjamin Gaignard @ 2017-04-12 18:58 UTC (permalink / raw)
To: Eric Anholt
Cc: Mark Rutland, devicetree, Alexandre TORGUE, Arnd Bergmann,
linux-arm-kernel, Russell King, Rob Herring, Philippe Cornu,
Yannick Fertre, kernel, dri-devel@lists.freedesktop.org,
Maxime Coquelin, Mickael Reulier, Vincent Abriou, Fabien Dessenne,
Gabriel FERNANDEZ, Benjamin Gaignard
In-Reply-To: <87wpaqeks2.fsf@eliezer.anholt.net>
2017-04-11 22:45 GMT+02:00 Eric Anholt <eric@anholt.net>:
> Yannick Fertre <yannick.fertre@st.com> writes:
>
>> This controller provides output signals to interface directly a variety
>> of LCD and TFT panels. These output signals are: RGB signals
>> (up to 24bpp), vertical & horizontal synchronisations, data enable and
>> the pixel clock.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
>> ---
>> drivers/gpu/drm/Kconfig | 3 +-
>> drivers/gpu/drm/Makefile | 1 +
>> drivers/gpu/drm/stm/Kconfig | 16 +
>> drivers/gpu/drm/stm/Makefile | 7 +
>> drivers/gpu/drm/stm/drv.c | 221 ++++++++
>> drivers/gpu/drm/stm/ltdc.c | 1210 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/stm/ltdc.h | 40 ++
>> 7 files changed, 1497 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/stm/Kconfig
>> create mode 100644 drivers/gpu/drm/stm/Makefile
>> create mode 100644 drivers/gpu/drm/stm/drv.c
>> create mode 100644 drivers/gpu/drm/stm/ltdc.c
>> create mode 100644 drivers/gpu/drm/stm/ltdc.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 78d7fc0..dd5762a 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -203,7 +203,6 @@ config DRM_VGEM
>> as used by Mesa's software renderer for enhanced performance.
>> If M is selected the module will be called vgem.
>>
>> -
>
> Stray whitespace change.
>
> With this removed, the driver is:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
>
> Apologies for the delay in the second review offered. The remainder of
> my comments are little cleanups, all of which I think are optional and
> fine to do after the code lands.
>
> You should probably update MAINTAINERS for your new driver. If you'd
> like to maintain this driver in the drm-misc small drivers collection
> (https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html),
> send a follow-up patch to the list to add the MAINTAINERS entry, and I
> can get that and patches 1-4 merged. Once you have a few more patches
> in, we can add you to the drm-misc committers crew so you can merge
> directly after getting review.
Since we are moving sti driver to drm-misc it makes sense to move all
STMicroelectronic SoC display drivers to it.
>
> I'll also take this moment to plug something: Please feel welcome to
> review other people's driver patches on the list. You've built
> something nice here, and probably learned a lot of lessons along the way
> that you could share with others. (I just found out about
> of_reset_control in reviewing your code, and I wish I had known about it
> back when I was landing vc4!)
>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> new file mode 100644
>> index 0000000..922f021
>> --- /dev/null
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>
>> +static void ltdc_crtc_disable(struct drm_crtc *crtc)
>> +{
>> + struct ltdc_device *ldev = crtc_to_ltdc(crtc);
>> + struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + if (!crtc->enabled) {
>> + DRM_DEBUG_DRIVER("already disabled\n");
>> + return;
>> + }
>
> I think this crtc->enabled is a given for the disable() being called.
>
>> +
>> + drm_crtc_vblank_off(crtc);
>> +
>> + /* disable LTDC */
>> + reg_clear(ldev->regs, LTDC_GCR, GCR_LTDCEN);
>> +
>> + /* disable IRQ */
>> + reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> + /* immediately commit disable of layers before switching off LTDC */
>> + reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
>> +
>> + if (event) {
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irq(&crtc->dev->event_lock);
>> + if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irq(&crtc->dev->event_lock);
>> + }
>
> I believe that we're guaranteed that crtc->state->event is NULL in the
> disable call, since your atomic_flush() already armed or sent the event
> and NULLed out the pointer.
>
>> +struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev)
>> +{
>> + struct drm_connector *connector;
>> + int err;
>> +
>> + connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
>> + if (!connector) {
>> + DRM_ERROR("Failed to allocate connector\n");
>> + return NULL;
>> + }
>> +
>> + connector->polled = DRM_CONNECTOR_POLL_HPD;
>> +
>> + err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
>> + DRM_MODE_CONNECTOR_LVDS);
>
> I think DRM_MODE_CONNECTOR_DPI (and _ENCODER_DPI) are slightly more
> accurate descriptions, if I'm interpreting your pinmux setup right.
> It's cosmetic, though.
>
>> +static struct drm_panel *ltdc_get_panel(struct drm_device *ddev)
>> +{
>> + struct device *dev = ddev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *entity, *port = NULL;
>> + struct drm_panel *panel = NULL;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + /*
>> + * Parse ltdc node to get remote port and find RGB panel / HDMI slave
>> + * If a dsi or a bridge (hdmi, lvds...) is connected to ltdc,
>> + * a remote port & RGB panel will not be found.
>> + */
>> + for_each_endpoint_of_node(np, entity) {
>> + if (!of_device_is_available(entity))
>> + continue;
>> +
>> + port = of_graph_get_remote_port_parent(entity);
>> + if (port) {
>> + panel = of_drm_find_panel(port);
>> + of_node_put(port);
>> + if (panel) {
>> + DRM_DEBUG_DRIVER("remote panel %s\n",
>> + port->full_name);
>> + } else {
>> + DRM_DEBUG_DRIVER("panel missing\n");
>> + of_node_put(entity);
>> + }
>> + }
>> + }
>
> Future work: You may find the new drm_of_find_panel_or_bridge() useful
> to drop this loop.
>
>> +
>> + return panel;
>> +}
>> +
>> +int ltdc_load(struct drm_device *ddev)
>> +{
>> + struct platform_device *pdev = to_platform_device(ddev->dev);
>> + struct ltdc_device *ldev = ddev->dev_private;
>> + struct device *dev = ddev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct drm_encoder *encoder;
>> + struct drm_connector *connector = NULL;
>> + struct drm_crtc *crtc;
>> + struct reset_control *rstc;
>> + struct resource res;
>> + int irq, ret, i;
>> +
>> + DRM_DEBUG_DRIVER("\n");
>> +
>> + ldev->panel = ltdc_get_panel(ddev);
>> + if (!ldev->panel)
>> + return -EPROBE_DEFER;
>> +
>> + rstc = of_reset_control_get(np, NULL);
>> +
>> + mutex_init(&ldev->err_lock);
>> +
>> + ldev->pixel_clk = devm_clk_get(dev, "lcd");
>> + if (IS_ERR(ldev->pixel_clk)) {
>> + DRM_ERROR("Unable to get lcd clock\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (clk_prepare_enable(ldev->pixel_clk)) {
>> + DRM_ERROR("Unable to prepare pixel clock\n");
>> + return -ENODEV;
>> + }
>
> Future work: You may want to move the pixel clock enable into the CRTC's
> .enable() and disable in .disable(). It sounded in previous versions
> like the HW uses that clock for all register accesses, so you'd need to
> protect a couple of other places, but that should save power when the
> device is off, right?
>
>> +
>> + if (of_address_to_resource(np, 0, &res)) {
>> + DRM_ERROR("Unable to get resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + ldev->regs = devm_ioremap_resource(dev, &res);
>> + if (IS_ERR(ldev->regs)) {
>> + DRM_ERROR("Unable to get ltdc registers\n");
>> + return PTR_ERR(ldev->regs);
>> + }
>> +
>> + for (i = 0; i < MAX_IRQ; i++) {
>> + irq = platform_get_irq(pdev, i);
>> + if (irq < 0)
>> + continue;
>> +
>> + ret = devm_request_threaded_irq(dev, irq, ltdc_irq,
>> + ltdc_irq_thread, IRQF_ONESHOT,
>> + dev_name(dev), ddev);
>> + if (ret) {
>> + DRM_ERROR("Failed to register LTDC interrupt\n");
>> + return ret;
>> + }
>> + }
>> +
>> + if (!IS_ERR(rstc))
>> + reset_control_deassert(rstc);
>> +
>> + /* Disable interrupts */
>> + reg_clear(ldev->regs, LTDC_IER,
>> + IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> + ret = ltdc_get_caps(ddev);
>> + if (ret) {
>> + DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
>> + ldev->caps.hw_version);
>> + return ret;
>> + }
>> +
>> + DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
>> +
>> + if (ltdc_create_encoders(ddev)) {
>> + DRM_ERROR("Failed to create encoders\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (ldev->panel) {
>> + encoder = ltdc_rgb_encoder_find(ddev);
>> + if (!encoder) {
>> + DRM_ERROR("Failed to find RGB encoder\n");
>> + ret = -EINVAL;
>> + goto err;
>> + }
>
> Given that ltdc_create_encoders() only does work if ldev->panel, its
> body could probably be moved in here and then ltdc_rgb_encoder_find
> could be dropped.
>
>> +
>> + connector = ltdc_rgb_connector_create(ddev);
>> + if (!connector) {
>> + DRM_ERROR("Failed to create RGB connector\n");
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + ret = drm_mode_connector_attach_encoder(connector, encoder);
>> + if (ret) {
>> + DRM_ERROR("Failed to attach connector to encoder\n");
>> + goto err;
>> + }
>> +
>> + drm_panel_attach(ldev->panel, connector);
>> + }
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 3/3] of: Make of_match_node() an inline stub for CONFIG_OF=n
From: kbuild test robot @ 2017-04-12 18:58 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
andrew-g2DYL2Zd6BY,
vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Florian Fainelli,
Lee Jones, Nicolas Ferre, Rob Herring, Frank Rowand,
open list:NETWORKING DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-4-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4084 bytes --]
Hi Florian,
[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.11-rc6 next-20170412]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/of-Make-of_match_node-an-inline-stub-for-CONFIG_OF-n/20170412-160916
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arm-s3c2410_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-s3c2410.c: In function 's3c24xx_get_device_quirks':
>> drivers/i2c/busses/i2c-s3c2410.c:174:25: error: 's3c24xx_i2c_match' undeclared (first use in this function)
match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-s3c2410.c:174:25: note: each undeclared identifier is reported only once for each function it appears in
--
drivers/watchdog/s3c2410_wdt.c: In function 's3c2410_get_wdt_drv_data':
>> drivers/watchdog/s3c2410_wdt.c:515:25: error: 's3c2410_wdt_match' undeclared (first use in this function)
match = of_match_node(s3c2410_wdt_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/watchdog/s3c2410_wdt.c:515:25: note: each undeclared identifier is reported only once for each function it appears in
vim +/s3c24xx_i2c_match +174 drivers/i2c/busses/i2c-s3c2410.c
faf93ff6 Giridhar Maruthy 2013-01-24 158 .data = (void *)(QUIRK_S3C2440 | QUIRK_NO_GPIO) },
117053f7 Vasanth Ananthan 2013-11-11 159 { .compatible = "samsung,exynos5-sata-phy-i2c",
117053f7 Vasanth Ananthan 2013-11-11 160 .data = (void *)(QUIRK_S3C2440 | QUIRK_POLL | QUIRK_NO_GPIO) },
27452498 Karol Lewandowski 2012-04-23 161 {},
27452498 Karol Lewandowski 2012-04-23 162 };
27452498 Karol Lewandowski 2012-04-23 163 MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
27452498 Karol Lewandowski 2012-04-23 164 #endif
^1da177e Linus Torvalds 2005-04-16 165
ec7c34a4 Krzysztof Kozlowski 2016-04-21 166 /*
27452498 Karol Lewandowski 2012-04-23 167 * Get controller type either from device tree or platform device variant.
^1da177e Linus Torvalds 2005-04-16 168 */
5f1b1155 Pankaj Dubey 2014-01-15 169 static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev)
^1da177e Linus Torvalds 2005-04-16 170 {
27452498 Karol Lewandowski 2012-04-23 171 if (pdev->dev.of_node) {
27452498 Karol Lewandowski 2012-04-23 172 const struct of_device_id *match;
0915833b Krzysztof Kozlowski 2016-04-21 173
b900ba4c Karol Lewandowski 2012-05-30 @174 match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
5f1b1155 Pankaj Dubey 2014-01-15 175 return (kernel_ulong_t)match->data;
27452498 Karol Lewandowski 2012-04-23 176 }
5a5f5080 Thomas Abraham 2011-09-13 177
27452498 Karol Lewandowski 2012-04-23 178 return platform_get_device_id(pdev)->driver_data;
^1da177e Linus Torvalds 2005-04-16 179 }
^1da177e Linus Torvalds 2005-04-16 180
ec7c34a4 Krzysztof Kozlowski 2016-04-21 181 /*
ec7c34a4 Krzysztof Kozlowski 2016-04-21 182 * Complete the message and wake up the caller, using the given return code,
:::::: The code at line 174 was first introduced by commit
:::::: b900ba4c1513a8c9a2fab8dca4cc6f50b17d6861 i2c: s3c2410: Fix pointer type passed to of_match_node()
:::::: TO: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
:::::: CC: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22646 bytes --]
^ permalink raw reply
* [PATCH v5 8/8] ARM: ux500: Add vendor prefix to tps61052 node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Aaro Koskinen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Tony Lindgren, Lee Jones, Javier Martinez Canillas, Mark Rutland,
Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170412172800.23035-1-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
The tps61052 device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/ste-hrefprev60.dtsi b/arch/arm/boot/dts/ste-hrefprev60.dtsi
index 5882a2606ac3..3f14b4df69b4 100644
--- a/arch/arm/boot/dts/ste-hrefprev60.dtsi
+++ b/arch/arm/boot/dts/ste-hrefprev60.dtsi
@@ -30,7 +30,7 @@
i2c@80004000 {
tps61052@33 {
- compatible = "tps61052";
+ compatible = "ti,tps61052";
reg = <0x33>;
};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 7/8] mfd: tps6105x: Add OF device ID table
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
Changes in v4: None
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/tps6105x.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c
index baa12ea666fb..187848c93779 100644
--- a/drivers/mfd/tps6105x.c
+++ b/drivers/mfd/tps6105x.c
@@ -173,9 +173,17 @@ static const struct i2c_device_id tps6105x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, tps6105x_id);
+static const struct of_device_id tps6105x_of_match[] = {
+ { .compatible = "ti,tps61050" },
+ { .compatible = "ti,tps61052" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tps6105x_of_match);
+
static struct i2c_driver tps6105x_driver = {
.driver = {
.name = "tps6105x",
+ .of_match_table = tps6105x_of_match,
},
.probe = tps6105x_probe,
.remove = tps6105x_remove,
--
2.9.3
^ permalink raw reply related
* [PATCH v5 6/8] dt-bindings: mfd: Add TI tps6105x chip bindings
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
There are Device Tree source files defining a device node for the
tps61050/61052 I2C chip but there isn't a binding document for it.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
new file mode 100644
index 000000000000..93602c7a19c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
@@ -0,0 +1,17 @@
+* Device tree bindings for TI TPS61050/61052 Boost Converters
+
+The TP61050/TPS61052 is a high-power "white LED driver". The
+device provides LED, GPIO and regulator functionalities.
+
+Required properties:
+- compatible: "ti,tps61050" or "ti,tps61052"
+- reg: Specifies the I2C slave address
+
+Example:
+
+i2c0 {
+ tps61052@33 {
+ compatible = "ti,tps61052";
+ reg = <0x33>;
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 5/8] i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Wolfram Sang, linux-i2c, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The example contains a device node for a retu device, but
its compatible string doesn't have a vendor prefix.
While being there, drop the -mfd suffix since isn't correct.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Avoid using MFD in Device Tree (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
index 8ce9cd2855b5..c143948b2a37 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
@@ -20,8 +20,8 @@ i2c@0 {
#address-cells = <1>;
#size-cells = <0>;
- retu-mfd: retu@1 {
- compatible = "retu-mfd";
+ retu: retu@1 {
+ compatible = "nokia,retu";
reg = <0x1>;
};
};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 4/8] ARM: dts: n8x0: Add vendor prefix to retu node
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Mark Rutland,
linux-omap, Russell King, linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
The retu device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 1b06430c8013..91886231e5a8 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -16,7 +16,7 @@
#address-cells = <1>;
#size-cells = <0>;
retu: retu@1 {
- compatible = "retu";
+ compatible = "nokia,retu";
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
reg = <0x1>;
--
2.9.3
^ permalink raw reply related
* [PATCH v5 3/8] mfd: retu: Add OF device ID table
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Aaro Koskinen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Tony Lindgren, Lee Jones, Javier Martinez Canillas
In-Reply-To: <20170412172800.23035-1-javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>'s Acked-by tag.
- Add Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>'s Acked-by tag.
Changes in v4:
- Drop -mfd suffix in compatible string (Lee Jones).
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/retu-mfd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index 53e1d386d2c0..e7d27b7861c1 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -308,9 +308,17 @@ static const struct i2c_device_id retu_id[] = {
};
MODULE_DEVICE_TABLE(i2c, retu_id);
+static const struct of_device_id retu_of_match[] = {
+ { .compatible = "nokia,retu" },
+ { .compatible = "nokia,tahvo" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, retu_of_match);
+
static struct i2c_driver retu_driver = {
.driver = {
.name = "retu-mfd",
+ .of_match_table = retu_of_match,
},
.probe = retu_probe,
.remove = retu_remove,
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v5 2/8] mfd: retu: Drop -mfd suffix from I2C device ID name
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Mark Rutland,
linux-omap, Russell King, linux-arm-kernel
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
It's not correct to encode the subsystem in the I2C device name, so
drop the -mfd suffix. To maintain bisect-ability, change driver and
platform code / DTS users in the same patch.
Suggested-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
---
Changes in v5:
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4: None
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 4 ++--
arch/arm/mach-omap1/board-nokia770.c | 4 ++--
drivers/mfd/retu-mfd.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 7e5ffc583c90..1b06430c8013 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -15,8 +15,8 @@
>;
#address-cells = <1>;
#size-cells = <0>;
- retu_mfd: retu@1 {
- compatible = "retu-mfd";
+ retu: retu@1 {
+ compatible = "retu";
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
reg = <0x1>;
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c
index ee8d9f553db4..06243c0b12d2 100644
--- a/arch/arm/mach-omap1/board-nokia770.c
+++ b/arch/arm/mach-omap1/board-nokia770.c
@@ -233,10 +233,10 @@ static struct platform_device nokia770_cbus_device = {
static struct i2c_board_info nokia770_i2c_board_info_2[] __initdata = {
{
- I2C_BOARD_INFO("retu-mfd", 0x01),
+ I2C_BOARD_INFO("retu", 0x01),
},
{
- I2C_BOARD_INFO("tahvo-mfd", 0x02),
+ I2C_BOARD_INFO("tahvo", 0x02),
},
};
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index d4c114abeb75..53e1d386d2c0 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -302,8 +302,8 @@ static int retu_remove(struct i2c_client *i2c)
}
static const struct i2c_device_id retu_id[] = {
- { "retu-mfd", 0 },
- { "tahvo-mfd", 0 },
+ { "retu", 0 },
+ { "tahvo", 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, retu_id);
--
2.9.3
^ permalink raw reply related
* [PATCH v5 1/8] dt-bindings: mfd: Add retu/tahvo ASIC chips bindings
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Mark Rutland
In-Reply-To: <20170412172800.23035-1-javier@osg.samsung.com>
There are Device Tree source files defining a device node for the
retu/tahvo I2C chip, but there isn't a DT binding document for it.
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Acked-by: Tony Lindgren <tony@atomide.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v5:
- Add missing properties for interrupts to DT binding doc (Rob Herring).
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/retu.txt | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
diff --git a/Documentation/devicetree/bindings/mfd/retu.txt b/Documentation/devicetree/bindings/mfd/retu.txt
new file mode 100644
index 000000000000..e1ea3a36a038
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/retu.txt
@@ -0,0 +1,23 @@
+* Device tree bindings for Nokia Retu and Tahvo multi-function device
+
+Retu and Tahvo are a multi-function devices found on Nokia Internet
+Tablets (770, N800 and N810). The Retu chip provides watchdog timer
+and power button control functionalities while Tahvo chip provides
+USB transceiver functionality.
+
+Required properties:
+- compatible: "nokia,retu" or "nokia,tahvo"
+- reg: Specifies the I2C slave address of the ASIC chip
+- interrupts: The interrupt line the device is connected to
+- interrupt-parent: The parent interrupt controller
+
+Example:
+
+i2c0 {
+ retu: retu@1 {
+ compatible = "nokia,retu";
+ interrupt-parent = <&gpio4>;
+ interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+ reg = <0x1>;
+ };
+};
--
2.9.3
^ permalink raw reply related
* [PATCH v5 0/8] mfd: Add OF device table to I2C drivers that are missing it
From: Javier Martinez Canillas @ 2017-04-12 17:27 UTC (permalink / raw)
To: linux-kernel
Cc: Aaro Koskinen, devicetree, Rob Herring, Tony Lindgren, Lee Jones,
Javier Martinez Canillas, Benoît Cousson, Wolfram Sang,
Mark Rutland, linux-omap, Russell King, linux-arm-kernel,
linux-i2c
Hello,
This series add OF device ID tables to mfd I2C drivers whose devices are
either used in Device Tree source files or are listed in binding docs as
a compatible string.
That's done because the plan is to change the I2C core to report proper OF
modaliases instead of always reporting a MODALIAS=i2c:<foo> regardless if
a device was registered via DT or using the legacy platform data mechanism.
So these patches will make sure that mfd I2C drivers modules will continue
to be autoloaded once the I2C core is changed to report proper OF modalias.
Users didn't have a vendor prefix in the used compatible strings, but since
there wasn't a DT binding document for these drivers, it can be said that
were working for mere luck and so this series fixes the users and add a DT
binding doc for the drivers.
Most patches can be applied independently, with the exception of patches
2 to 4 that should be applied in the same tree to keep bisect-ability. I
suggest these to go through the MFD subsystem tree.
Best regards,
Javier
Changes in v5:
- Add missing properties for interrupts to DT binding doc (Rob Herring).
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Aaro Koskinen's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
- Add Lee Jones <lee.jones@linaro.org>'s Acked-by tag.
- Add Rob Herring's Acked-by tag.
- Add Acked-by: Tony Lindgren <tony@atomide.com>'s Acked-by tag.
Changes in v4:
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Drop -mfd suffix in compatible string (Lee Jones).
- Avoid using MFD in Device Tree (Lee Jones).
- Use "dt-bindings: mfd:" prefix in subject line (Rob Herring).
- Add information about what functions the device serve (Lee Jones).
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
Javier Martinez Canillas (8):
dt-bindings: mfd: Add retu/tahvo ASIC chips bindings
mfd: retu: Drop -mfd suffix from I2C device ID name
mfd: retu: Add OF device ID table
ARM: dts: n8x0: Add vendor prefix to retu node
i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
dt-bindings: mfd: Add TI tps6105x chip bindings
mfd: tps6105x: Add OF device ID table
ARM: ux500: Add vendor prefix to tps61052 node
.../devicetree/bindings/i2c/i2c-cbus-gpio.txt | 4 ++--
Documentation/devicetree/bindings/mfd/retu.txt | 23 ++++++++++++++++++++++
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 ++++++++++++++++
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 4 ++--
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
arch/arm/mach-omap1/board-nokia770.c | 4 ++--
drivers/mfd/retu-mfd.c | 12 +++++++++--
drivers/mfd/tps6105x.c | 8 ++++++++
8 files changed, 65 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
--
2.9.3
^ permalink raw reply
* Re: [PATCH v5 1/4] gpio: mvebu: Add limited PWM support
From: Thierry Reding @ 2017-04-12 17:21 UTC (permalink / raw)
To: Ralph Sennhauser
Cc: Andrew Lunn, Linus Walleij, Alexandre Courbot, Rob Herring,
Mark Rutland, Jason Cooper, Gregory Clement,
Sebastian Hesselbarth, Russell King, linux-pwm, linux-gpio,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170409180931.4884-2-ralph.sennhauser@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2739 bytes --]
On Sun, Apr 09, 2017 at 08:09:27PM +0200, Ralph Sennhauser wrote:
> From: Andrew Lunn <andrew@lunn.ch>
>
> Armada 370/XP devices can 'blink' GPIO lines with a configurable on
> and off period. This can be modelled as a PWM.
>
> However, there are only two sets of PWM configuration registers for
> all the GPIO lines. This driver simply allows a single GPIO line per
> GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
> EBUSY.
>
> Due to the interleaving of registers it is not simple to separate the
> PWM driver from the GPIO driver. Thus the GPIO driver has been
> extended with a PWM driver.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> URL: https://patchwork.ozlabs.org/patch/427287/
> URL: https://patchwork.ozlabs.org/patch/427295/
> [Ralph Sennhauser:
> * Port forward
> * Merge PWM portion into gpio-mvebu.c
> * Switch to atomic PWM API
> * Add new compatible string marvell,armada-370-xp-gpio
> * Update and merge documentation patch
> * Update MAINTAINERS]
> Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
> Tested-by: Andrew Lunn <andrew@lunn.ch>
> ---
> .../devicetree/bindings/gpio/gpio-mvebu.txt | 32 ++
> MAINTAINERS | 2 +
> drivers/gpio/gpio-mvebu.c | 324 ++++++++++++++++++++-
> 3 files changed, 346 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> index a6f3bec..fe49e9d 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
> @@ -38,6 +38,24 @@ Required properties:
> - #gpio-cells: Should be two. The first cell is the pin number. The
> second cell is reserved for flags, unused at the moment.
>
> +Optional properties:
> +
> +In order to use the gpio lines in PWM mode, some additional optional
> +properties are required. Only Armada 370 and XP support these properties.
> +
> +- compatible: Must contain "marvell,armada-370-xp-gpio"
> +
> +- reg: an additional register set is needed, for the GPIO Blink
> + Counter on/off registers.
> +
> +- reg-names: Must contain an entry "pwm" corresponding to the
> + additional register range needed for pwm operation.
> +
> +- #pwm-cells: Should be two. The first cell is the GPIO line number. The
> + second cell is the period in nanoseconds.
> +
> +- clocks: Must be a phandle to the clock for the gpio controller.
One other thing: there's a mix of pwm/PWM and gpio/GPIO in this hunk. In
prose, always use the all-uppercase variants because they are
abbreviations.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH V3 2/4] pwm: tegra: Increase precision in pwm rate calculation
From: Thierry Reding @ 2017-04-12 17:19 UTC (permalink / raw)
To: Laxman Dewangan
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
mark.rutland-5wv7dgnIgG8, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1491557642-15940-3-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]
On Fri, Apr 07, 2017 at 03:04:00PM +0530, Laxman Dewangan wrote:
> The rate of the PWM calculated as follows:
> hz = NSEC_PER_SEC / period_ns;
> rate = (rate + (hz / 2)) / hz;
>
> This has the precision loss in lower PWM rate.
>
> Change this to have more precision as:
> hz = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC * 100, period_ns);
> rate = DIV_ROUND_CLOSEST(rate * 100, hz)
>
> Example:
> 1. period_ns = 16672000, PWM clock rate is 200KHz.
> Based on old formula
> hz = NSEC_PER_SEC / period_ns
> = 1000000000ul/16672000
> = 59 (59.98)
> rate = (200K + 59/2)/59 = 3390
>
> Based on new method:
> hz = 5998
> rate = DIV_ROUND_CLOSE(200000*100, 5998) = 3334
>
> If we measure the PWM signal rate, we will get more accurate period
> with rate value of 3334 instead of 3390.
>
> 2. period_ns = 16803898, PWM clock rate is 200KHz.
> Based on old formula:
> hz = 59, rate = 3390
> Based on new formula:
> hz = 5951, rate = 3360
>
> The PWM signal rate of 3360 is more near to requested period than 3333.
>
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> ---
> Changes from v1:
> - None
>
> Changes from V2:
> - Fix the commit message with exact formula used.
> ---
> drivers/pwm/pwm-tegra.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 0a688da..21518be 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -76,6 +76,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
> unsigned long long c = duty_ns;
> unsigned long rate, hz;
> + unsigned long long ns100 = NSEC_PER_SEC;
> u32 val = 0;
> int err;
>
> @@ -94,9 +95,11 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> * cycles at the PWM clock rate will take period_ns nanoseconds.
> */
> rate = clk_get_rate(pc->clk) >> PWM_DUTY_WIDTH;
> - hz = NSEC_PER_SEC / period_ns;
>
> - rate = (rate + (hz / 2)) / hz;
> + /* Consider precision in PWM_SCALE_WIDTH rate calculation */
> + ns100 *= 100;
> + hz = DIV_ROUND_CLOSEST_ULL(ns100, period_ns);
I think hz could overflow for small enough values of period_ns. I've
sent a patch that makes hz unsigned long long. While at it, the patch
also removes the ns100 variable which isn't really necessary here.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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