* Re: [PATCHv2 2/3] ARM: drm: Intel FPGA VIP Frame Buffer II drm driver
From: Jani Nikula @ 2017-04-25 7:17 UTC (permalink / raw)
To: daniel.vetter-ral2JQCrhuEAvxtiuMwx3w,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
tien.hock.loh-ral2JQCrhuEAvxtiuMwx3w, Ong-CC+yJ3UmIYqDUpFQwHEjaQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <1493086006-4392-3-git-send-email-hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Tue, 25 Apr 2017, hean.loong.ong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> +++ b/drivers/gpu/drm/ivip/Makefile
> @@ -0,0 +1,9 @@
> +#
> +# Makefile for the drm device driver. This driver provides support for the
> +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> +
> +ccflags-y := -Iinclude/drm
Just a drive-by observation, there are patches on the list removing such
ccflags from existing drivers. You shouldn't need this. Just make sure
all your #includes begin with <drm/.
BR,
Jani.
> +
> +obj-$(CONFIG_DRM_IVIP) += ivip.o
> +ivip-objs := intel_vip_of.o intel_vip_core.o \
> +intel_vip_conn.o
--
Jani Nikula, Intel Open Source Technology Center
--
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] PM / Domains: Fix DT example
From: Viresh Kumar @ 2017-04-25 7:18 UTC (permalink / raw)
To: Rafael Wysocki, Kevin Hilman, Ulf Hansson
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
Viresh Kumar, devicetree-u79uwXL29TY76Z2rM5mHXA
The power-domain provider's #power-domain-cells field is set to 0 and
yet the children is using an index to point the power domain. Fix it by
removing the index field.
Fixes: 70bb510e4279 ("dt/bindings / PM/Domains: Update binding for PM domain idle states")
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/power/power_domain.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 940707d095cc..14bd9e945ff6 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -81,7 +81,7 @@ domain provided by the 'parent' power controller.
child: power-controller@12341000 {
compatible = "foo,power-controller";
reg = <0x12341000 0x1000>;
- power-domains = <&parent 0>;
+ power-domains = <&parent>;
#power-domain-cells = <0>;
domain-idle-states = <&DOMAIN_PWR_DN>;
};
--
2.12.0.432.g71c3a4f4ba37
--
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: [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Jiada Wang @ 2017-04-25 7:56 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam, linux-spi,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAMuHMdVQS494-BAc-W-XOOLK8Xow85n+Cgih0FG+t4QxCFxhMA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Geert
On 04/24/2017 06:10 AM, Geert Uytterhoeven wrote:
> Hi Jiada,
>
> On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang<jiada_wang-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> wrote:
>> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> wrote:
>>>> On 04/13/2017 12:47 PM, Geert Uytterhoeven wrote:
>>>>> On Thu, Apr 13, 2017 at 2:59 PM, Mark Brown<broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>>>> On Thu, Apr 13, 2017 at 05:13:59AM -0700, jiada_wang-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org wrote:
>>>>>>> From: Jiada Wang<jiada_wang-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
>>>>>>>
>>>>>>> v1:
>>>>>>> add Slave mode support in SPI core
>>>>>>> spidev create slave device when SPI controller work in slave mode
>>>>>>> spi-imx support to work in slave mode
>>>>>> Adding Geert who also had a series doing this in progress that was
>>>>>> getting very near to being merged.
>>>>> Thank you!
>>>>>
>>>>> Actually my plan is to fix the last remaining issues and resubmit for
>>>>> v4.13.
>>>> I noticed your patch set for SPI slave support,
>>>> (I am sure you can find out some of the change
>>>> in this patch set is based on your work).
>>>> we have similar requirement to add slave mode support to ecspi IP on imx6
>>>> Soc.
>>>>
>>>> Our use case is to use spidev as an interface to communicate with
>>>> external
>>>> SPI master devices.
>>>> meanwhile the SPI bus controller can also act as master device to send
>>>> data
>>>> to other
>>>> SPI slave devices on the board.
>>> That sounds a bit hackish to me. SPI was never meant to be a multi-master
>>> bus.
>>> While it can be done, you will need external synchronization (signals) to
>>> avoid conflicts between the SPI masters.
>> It doesn't need to be a multi-master bus,
>> for example A is master device for slave device B.
>> while B has its own slave device C
>> for each SPI connection A<=> B, and B<=> C, there is only one master
>> device.
>>
>> and I think from use case point of view, it's very normal,
>> one CPU upon receives command from external SPI master device,
>> it writes data to its own slave device (EEPROM) connected to it.
> So "A<=> B" and "B<=> C" are two distinct SPI buses?
> Or do they share some signals?
>
> Your comment seems to suggest otherwise:
the use case of
"A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
MOSI lines,
but there is no SS line between A and C. so for each SPI slave device,
there is only one
master device.
so I think the question becomes whether the above mentioned hardware
setup is valid or not.
Thanks,
Jiada
>>>> I found in your implementation, SPI bus controller is limited to either work in master mode or
>>>> slave mode, is there any reasoning to not configure SPI mode based on SPI devices use case?
> If they are distinct, it should work. Then B has two SPI controllers: one slave
> controller controlled by A, and one master controller to control C.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803
From: Lee Jones @ 2017-04-25 7:57 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
Liam Girdwood, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170424160103.9447-8-icenowy-h8G6r0blFSE@public.gmane.org>
On Tue, 25 Apr 2017, Icenowy Zheng wrote:
> As axp20x-regulator now supports AXP803, add a cell for it.
>
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> ---
> Changes in v4:
> - Added a trailing comma for new cell, for easier further cell addition.
> Changes in v3:
> - Make the new cell one-liner.
>
> drivers/mfd/axp20x.c | 3 ++-
> drivers/regulator/axp20x-regulator.c | 6 +++---
These 2 changes are orthogonal, thus there is no reason to send them
bundled into a single patch. Doing so complicates things greatly.
Please resubmit the two changes separately, so that they may be
absorbed by our respective subsystems.
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 1dc6235778eb..917b6ddc4f15 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -848,7 +848,8 @@ static struct mfd_cell axp803_cells[] = {
> .name = "axp20x-pek",
> .num_resources = ARRAY_SIZE(axp803_pek_resources),
> .resources = axp803_pek_resources,
> - }
> + },
> + { .name = "axp20x-regulator" },
> };
>
> static struct mfd_cell axp806_cells[] = {
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 9356ec8a9a1f..e2608fe770b9 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -311,13 +311,13 @@ static const struct regulator_desc axp803_regulators[] = {
> AXP803_FLDO1_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
> AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
> AXP803_FLDO2_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(3)),
> - AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
> + AXP_DESC_IO(AXP803, LDO_IO0, "ldo-io0", "ips", 700, 3300, 100,
> AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
> - AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
> + AXP_DESC_IO(AXP803, LDO_IO1, "ldo-io1", "ips", 700, 3300, 100,
> AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
> - AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
> + AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc-ldo", "ips", 3000),
> };
>
> static const struct regulator_linear_range axp806_dcdca_ranges[] = {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-25 8:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, linux-i2c, devicetree,
Linux Kernel Mailing List, OpenBMC Maillist, Ryan Chen
In-Reply-To: <1493086864.25766.266.camel@kernel.crashing.org>
Adding Ryan to thread.
>> +static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> + struct platform_device *pdev)
>> +{
>
> Minor nit ... I'm really not fan of those underscores.
>
> We use __ functions in some cases in the kernel for low level
> helpers, usually when it's a low level variant of an existing
> function or an "unlocked" variant, but I don't think generalizing
> it to pretty much everything in the driver is worthwhile here.
>
> If you want to be explicit about locking, I would suggest you
> use a comment in front of the function explaining if it
> expects to be called with the lock held.
>
> We tend to only do that when *both* functions exist and one is
> implemented in term of the other.
Okay, I guess that makes sense. Sorry, I thought the "unlocked"
variant might refer to a function that you have to pay close attention
to the context in which it is called; with as many functions as I have
that require the lock to be held, I would like there to be some way to
say the function is "unsafe," but I guess if there is no convention to
do that, then there is no convention to do that.
^ permalink raw reply
* Re: [PATCH] PM / Domains: Fix DT example
From: Ulf Hansson @ 2017-04-25 8:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Kevin Hilman, linaro-kernel,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Vincent Guittot, devicetree@vger.kernel.org
In-Reply-To: <533b52e0ea175bf6bb893370c7f8c0309aae235a.1493104411.git.viresh.kumar@linaro.org>
On 25 April 2017 at 09:18, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> The power-domain provider's #power-domain-cells field is set to 0 and
> yet the children is using an index to point the power domain. Fix it by
> removing the index field.
>
> Fixes: 70bb510e4279 ("dt/bindings / PM/Domains: Update binding for PM domain idle states")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Documentation/devicetree/bindings/power/power_domain.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index 940707d095cc..14bd9e945ff6 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -81,7 +81,7 @@ domain provided by the 'parent' power controller.
> child: power-controller@12341000 {
> compatible = "foo,power-controller";
> reg = <0x12341000 0x1000>;
> - power-domains = <&parent 0>;
> + power-domains = <&parent>;
> #power-domain-cells = <0>;
> domain-idle-states = <&DOMAIN_PWR_DN>;
> };
> --
> 2.12.0.432.g71c3a4f4ba37
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Uwe Kleine-König @ 2017-04-25 8:07 UTC (permalink / raw)
To: Jiada Wang
Cc: Mark Rutland, devicetree@vger.kernel.org, Mark Brown,
linux-kernel@vger.kernel.org, linux-spi, Rob Herring,
Geert Uytterhoeven, Sascha Hauer, Fabio Estevam, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <58FF0127.7070703@mentor.com>
Hello Jiada,
On Tue, Apr 25, 2017 at 12:56:23AM -0700, Jiada Wang wrote:
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.
So you need a mutex to make A not use the bus while B communicates to C.
Otherwise you have two drivers on MOSI (A and B) and MISO (B and C).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH RFC 0/5] *** SPI Slave mode support ***
From: Geert Uytterhoeven @ 2017-04-25 8:09 UTC (permalink / raw)
To: Jiada Wang
Cc: Mark Rutland, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Rob Herring, linux-spi, Mark Brown,
Sascha Hauer, Fabio Estevam, Shawn Guo,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <58FF0127.7070703@mentor.com>
Hi Jiada,
On Tue, Apr 25, 2017 at 9:56 AM, Jiada Wang <jiada_wang@mentor.com> wrote:
> On 04/24/2017 06:10 AM, Geert Uytterhoeven wrote:
>> On Mon, Apr 24, 2017 at 2:48 PM, Jiada Wang<jiada_wang@mentor.com> wrote:
>>> On 04/24/2017 03:55 AM, Geert Uytterhoeven wrote:
>>>> On Fri, Apr 14, 2017 at 7:39 AM, Jiada Wang<jiada_wang@mentor.com>
>>>> wrote:
>>>>> Our use case is to use spidev as an interface to communicate with
>>>>> external
>>>>> SPI master devices.
>>>>> meanwhile the SPI bus controller can also act as master device to send
>>>>> data
>>>>> to other
>>>>> SPI slave devices on the board.
>>>>
>>>> That sounds a bit hackish to me. SPI was never meant to be a
>>>> multi-master
>>>> bus.
>>>> While it can be done, you will need external synchronization (signals)
>>>> to
>>>> avoid conflicts between the SPI masters.
>>>
>>> It doesn't need to be a multi-master bus,
>>> for example A is master device for slave device B.
>>> while B has its own slave device C
>>> for each SPI connection A<=> B, and B<=> C, there is only one master
>>> device.
>>>
>>> and I think from use case point of view, it's very normal,
>>> one CPU upon receives command from external SPI master device,
>>> it writes data to its own slave device (EEPROM) connected to it.
>>
>> So "A<=> B" and "B<=> C" are two distinct SPI buses?
>> Or do they share some signals?
>>
>> Your comment seems to suggest otherwise:
>
> the use case of
> "A (master) <=> B (slave)", "B (master) <=> C(slave)", do share MISO and
> MOSI lines,
> but there is no SS line between A and C. so for each SPI slave device, there
> is only one
> master device.
Do you share CLK, too? Then you need a slave select from B to C.
If you use a separate clock, the slave select from B to C can be optional.
> so I think the question becomes whether the above mentioned hardware setup
> is valid or not.
It's a non-conventional SPI bus setup, but it can work, provided you have
some form of synchronization between A and B.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/5 v3] usb: host: add DT bindings for faraday fotg2
From: Linus Walleij @ 2017-04-25 8:12 UTC (permalink / raw)
To: Hans Ulli Kroll
Cc: openwrt-devel, devicetree@vger.kernel.org, Paulius Zaleckas,
Greg Kroah-Hartman, linux-usb@vger.kernel.org, Janos Laube,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <alpine.LNX.2.00.1704241846300.15211@T420s>
On Mon, Apr 24, 2017 at 6:53 PM, Hans Ulli Kroll
<ulli.kroll@googlemail.com> wrote:
> Got NAK'ed from Rob on some ealier round due missing "device mode" on this
> IP. I've blatantly overrided this to a host only driver.
>
> These are the needed changes in DT to support both modes
> Note the -dr at the end of fotg210, to reflect this in an dual role device
OK I understood the discussion such that the compatible should
simply be ""faraday,fotg210" as that is the name of the hardware
IP block. This is the name of the hardware name in the Faraday
page:
http://www.faraday-tech.com/html/Product/IPProduct/InterfaceIP/USB2_0.htm
Any other string implies how it is used, so that was what I understood
as the reason to reject it with the "-hcd" (host controller device) suffix.
> +- dr_mode : indicates the working mode for "fotg210-dr" compatible
> + controllers. Can be "host", "peripheral". Default to
> + "host" if not defined for backward compatibility.
This seems right, so it is part of the generic bindings, correct?
> usb@68000000 {
> - compatible = "cortina,gemini-usb", "faraday,fotg210";
> + compatible = "cortina,gemini-usb", "faraday,fotg210-dr";
But this would be wrong, because the compatible should only
indicate what kind of hardware it is, not how it is going to be used
(whether as host only, slave only or dual-role (OTG).
I hope I didn't get anything wrong here :/
Yours,
Linus Walleij
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply
* [PATCH 0/2] Add max7360 mfd driver and DT documentation
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Valentin Sitdikov
This series add initial support of mfd core driver for max7360 chip
Andrei Dranitca (1):
mfd: max7360: Add mfd core device driver
Valentin Sitdikov (1):
Add DT bindings documentation for the max7360 mfd driver
Documentation/devicetree/bindings/mfd/max7360.txt | 72 ++++
drivers/mfd/Kconfig | 16 +
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 393 ++++++++++++++++++++++
include/linux/mfd/max7360.h | 130 +++++++
5 files changed, 612 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
create mode 100644 drivers/mfd/max7360.c
create mode 100644 include/linux/mfd/max7360.h
--
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
* [PATCH 1/2] Add DT bindings documentation for the max7360 mfd driver
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Valentin Sitdikov, Andrei Dranitca
In-Reply-To: <20170425081557.13941-1-valentin_sitdikov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Valentin Sitdikov <valentin_sitdikov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
Documentation/devicetree/bindings/mfd/max7360.txt | 72 +++++++++++++++++++++++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max7360.txt
diff --git a/Documentation/devicetree/bindings/mfd/max7360.txt b/Documentation/devicetree/bindings/mfd/max7360.txt
new file mode 100644
index 0000000..359073a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max7360.txt
@@ -0,0 +1,72 @@
+* Maxim MAX7360 multi-function device
+
+The Maxim MAX7360 is a multifunction device which includes
+64 key switches, eight LED drivers/GPIOs, PWM intensity control,
+and rotary switch control.
+
+Required properties:
+- compatible: Should be the following: "maxim,max7360"
+- reg: Specifies the i2c slave address of the max7360 block. It can be 0x38, 0x3a, 0x3c or 0x3e IIUC.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+ the interrupts from MAX7360 are routed to.
+- interrupt-names: might be "int-shared" or list of "inti" and "intk"
+- interrupt-controller: Identifies the device as an interrupt controller.
+- #interrupt-cells : Number of cells to encode an interrupt source, shall be 1.
+
+Examples:
+
+Without subnodes:
+ max7360@38 {
+ compatible = "maxim,max7360";
+ reg = <0x38>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "int-shared";
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ };
+
+With subnodes:
+ max7360@38 {
+ compatible = "maxim,max7360";
+ reg = <0x38>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "int-shared";
+ interrupt-controller;
+ #interrupt-cells = <0x1>;
+
+ max7360_gpio: max7360_gpio@0 {
+ compatible = "maxim,max7360-gpio";
+ gpio-controller;
+ #gpio-cells = <0x2>;
+ interrupt-controller;
+ #interrupt-cells = <0x2>;
+ interrupts = <0>;
+ };
+
+ max7360_keypad {
+ compatible = "maxim,max7360-keypad";
+ maxim,debounce_reg = /bits/ 8 <0xef>;
+ maxim,ports_reg = /bits/ 8 <0xae>;
+ linux,keymap = < MATRIX_KEY(0, 0, KEY_F5)
+ MATRIX_KEY(1, 0, KEY_F4) >;
+ keypad,num-rows = <2>;
+ keypad,num-columns = <1>;
+ interrupts = <1>;
+ };
+
+ max7360_pwm: max7360_pwm {
+ compatible = "maxim,max7360-pwm";
+ #pwm-cells = <0x2>;
+ };
+
+ max7360_rotary_encoder {
+ compatible = "maxim,max7360-rotary";
+ interrupts = <2>;
+ };
+
+ };
--
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 2/2] mfd: max7360: Add mfd core device driver
From: Valentin Sitdikov @ 2017-04-25 8:15 UTC (permalink / raw)
To: lee.jones, robh+dt, mark.rutland, devicetree, linux-kernel
Cc: Andrei Dranitca, Valentin Sitdikov
In-Reply-To: <20170425081557.13941-1-valentin_sitdikov@mentor.com>
From: Andrei Dranitca <Andrei_Dranitca@mentor.com>
This patch adds core/irq driver to support MAX7360 i2c chip
which contains keypad, gpio, pwm, gpo and rotary encoder submodules.
Signed-off-by: Valentin Sitdikov <valentin_sitdikov@mentor.com>
Signed-off-by: Andrei Dranitca <Andrei_Dranitca@mentor.com>
---
drivers/mfd/Kconfig | 16 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/max7360.c | 393 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/max7360.h | 130 +++++++++++++++
4 files changed, 540 insertions(+)
create mode 100644 drivers/mfd/max7360.c
create mode 100644 include/linux/mfd/max7360.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 55ecdfb..2227c65 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -688,6 +688,22 @@ config MFD_MAX8998
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_MAX7360
+ tristate "Maxim Semiconductor MAX7360 support"
+ depends on I2C && OF
+ select MFD_CORE
+ select REGMAP_I2C
+ select IRQ_DOMAIN
+ help
+ Say yes here to add support for Maxim Semiconductor MAX7360.
+ This provides microprocessors with management of up to 64 key switches,
+ with an additional eight LED drivers/GPIOs that feature constant-current,
+ PWM intensity control, and rotary switch control options.
+
+ This driver provides common support for accessing the device,
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
config MFD_MT6397
tristate "MediaTek MT6397 PMIC Support"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 31ce076..f01f3a1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o
obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX7360) += max7360.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77686) += max77686.o
obj-$(CONFIG_MFD_MAX77693) += max77693.o
diff --git a/drivers/mfd/max7360.c b/drivers/mfd/max7360.c
new file mode 100644
index 0000000..b4c2fdd
--- /dev/null
+++ b/drivers/mfd/max7360.c
@@ -0,0 +1,393 @@
+/*
+ * Copyright (C) 2017 Mentor Graphics
+ *
+ * Author: Valentin Sitdikov <Valentin.Sitdikov@mentor.com>
+ * Author: Andrei Dranitca <Andrei_Dranitca@mentor.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max7360.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin)
+{
+ struct i2c_client *client = max7360->i2c;
+ int ret = 0;
+
+ spin_lock(&max7360->lock);
+ if (max7360->gpio_pins & BIT(pin)) {
+ dev_err(&client->dev, "pin %d already requested, mask %x",
+ pin, max7360->gpio_pins);
+ spin_unlock(&max7360->lock);
+ return -EBUSY;
+ }
+ max7360->gpio_pins |= BIT(pin);
+ dev_dbg(&client->dev, "pin %d requested successfully", pin);
+ spin_unlock(&max7360->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(max7360_request_pin);
+
+void max7360_free_pin(struct max7360 *max7360, u8 pin)
+{
+ spin_lock(&max7360->lock);
+ max7360->gpio_pins &= ~BIT(pin);
+ spin_unlock(&max7360->lock);
+}
+EXPORT_SYMBOL_GPL(max7360_free_pin);
+
+static const struct mfd_cell max7360_devices[] = {
+ {
+ .name = "max7360-gpio",
+ .of_compatible = "maxim,max7360-gpio",
+ },
+ {
+ .name = "max7360-keypad",
+ .of_compatible = "maxim,max7360-keypad",
+ },
+ {
+ .name = "max7360-pwm",
+ .of_compatible = "maxim,max7360-pwm",
+ },
+ {
+ .name = "max7360-rotary",
+ .of_compatible = "maxim,max7360-rotary",
+ },
+};
+
+static irqreturn_t max7360_irq(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqi(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_GPIO);
+ handle_nested_irq(virq);
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_ROTARY);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t max7360_irqk(int irq, void *data)
+{
+ struct max7360 *max7360 = data;
+ int virq;
+
+ virq = irq_find_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ handle_nested_irq(virq);
+
+ return IRQ_HANDLED;
+}
+
+static int max7360_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hwirq)
+{
+ struct max7360 *max7360 = d->host_data;
+
+ irq_set_chip_data(virq, max7360);
+ irq_set_chip_and_handler(virq, &dummy_irq_chip,
+ handle_edge_irq);
+ irq_set_nested_thread(virq, 1);
+ irq_set_noprobe(virq);
+
+ return 0;
+}
+
+static void max7360_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+ irq_set_chip_and_handler(virq, NULL, NULL);
+ irq_set_chip_data(virq, NULL);
+}
+
+static const struct irq_domain_ops max7360_irq_ops = {
+ .map = max7360_irq_map,
+ .unmap = max7360_irq_unmap,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+static int max7360_irq_init(struct max7360 *max7360, struct device_node *np)
+{
+ int ret;
+
+ max7360->inti = of_irq_get_byname(np, "inti");
+ max7360->intk = of_irq_get_byname(np, "intk");
+
+ if (max7360->inti < 0 || max7360->intk < 0) {
+ max7360->shared_irq = of_irq_get_byname(np, "int-shared");
+
+ if (max7360->shared_irq < 0) {
+ dev_err(max7360->dev, "failed to find IRQ in dts\n");
+ return -EINVAL;
+ }
+
+ ret = request_threaded_irq(max7360->shared_irq, NULL,
+ max7360_irq,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to request IRQ: %d\n",
+ ret);
+ return ret;
+ }
+ } else {
+ max7360->shared_irq = 0;
+ ret = request_threaded_irq(max7360->inti, NULL, max7360_irqi,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to request inti IRQ: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = request_threaded_irq(max7360->intk, NULL, max7360_irqk,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "max7360", max7360);
+ if (ret) {
+ free_irq(max7360->inti, max7360);
+ dev_err(max7360->dev, "failed to request intk IRQ: %d\n",
+ ret);
+ return ret;
+ }
+ }
+
+ max7360->domain = irq_domain_add_simple(np, MAX7360_NR_INTERNAL_IRQS,
+ 0, &max7360_irq_ops, max7360);
+
+ if (!max7360->domain) {
+ if (max7360->shared_irq)
+ free_irq(max7360->shared_irq, max7360);
+ else {
+ free_irq(max7360->inti, max7360);
+ free_irq(max7360->intk, max7360);
+ }
+ dev_err(max7360->dev, "Failed to create irqdomain\n");
+ return -ENODEV;
+ }
+
+ irq_create_mapping(max7360->domain, MAX7360_INT_GPIO);
+ irq_create_mapping(max7360->domain, MAX7360_INT_KEYPAD);
+ irq_create_mapping(max7360->domain, MAX7360_INT_ROTARY);
+
+ return 0;
+}
+
+void max7360_fall_deepsleep(struct max7360 *max7360)
+{
+ max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_8192);
+}
+EXPORT_SYMBOL_GPL(max7360_fall_deepsleep);
+
+void max7360_take_catnap(struct max7360 *max7360)
+{
+ max7360_write_reg(max7360, MAX7360_REG_SLEEP, MAX7360_AUTOSLEEP_256);
+}
+EXPORT_SYMBOL_GPL(max7360_take_catnap);
+
+static int max7360_chip_init(struct max7360 *max7360)
+{
+ max7360->gpio_pins = MAX7360_MAX_GPIO;
+ max7360->gpo_count = 0;
+ max7360->col_count = MAX7360_COL_GPO_PINS;
+ return 0;
+}
+
+static int max7360_device_init(struct max7360 *max7360)
+{
+ int ret = 0;
+
+ ret = mfd_add_devices(max7360->dev, -1, max7360_devices,
+ ARRAY_SIZE(max7360_devices), NULL,
+ 0, max7360->domain);
+ if (ret)
+ dev_err(max7360->dev, "failed to add child devices\n");
+
+ return ret;
+}
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count)
+{
+ if (count > MAX7360_MAX_GPO)
+ return -EINVAL;
+ if (max7360->col_count + count > MAX7360_COL_GPO_PINS) {
+ dev_err(max7360->dev,
+ "trying to request %d pins as gpo while %d pins already used as COL\n",
+ count, max7360->col_count);
+ return -EINVAL;
+ }
+ max7360->gpo_count = count;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_gpo_pin_count);
+
+int max7360_request_col_count(struct max7360 *max7360, u8 count)
+{
+ if (max7360->gpo_count + count > MAX7360_COL_GPO_PINS) {
+ dev_err(max7360->dev,
+ "trying to request %d pins as COL while %d pins already used as gpo\n",
+ count, max7360->gpo_count);
+ return -EINVAL;
+ }
+ max7360->col_count = count;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(max7360_request_col_count);
+
+static const struct regmap_range max7360_volatile_ranges[] = {
+ {
+ .range_min = MAX7360_REG_KEYFIFO,
+ .range_max = MAX7360_REG_KEYFIFO,
+ }, {
+ .range_min = 0x48,
+ .range_max = 0x4a,
+ },
+};
+
+static const struct regmap_access_table max7360_volatile_table = {
+ .yes_ranges = max7360_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(max7360_volatile_ranges),
+};
+
+static const struct regmap_config max7360_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0xff,
+ .volatile_table = &max7360_volatile_table,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int max7360_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct device_node *np = i2c->dev.of_node;
+ struct max7360 *max7360;
+
+ int ret;
+
+ max7360 = devm_kzalloc(&i2c->dev, sizeof(struct max7360),
+ GFP_KERNEL);
+ if (!max7360)
+ return -ENOMEM;
+
+ spin_lock_init(&max7360->lock);
+
+ max7360->dev = &i2c->dev;
+ max7360->i2c = i2c;
+
+ i2c_set_clientdata(i2c, max7360);
+
+ max7360->regmap = devm_regmap_init_i2c(i2c, &max7360_regmap_config);
+ ret = max7360_chip_init(max7360);
+ if (ret)
+ return ret;
+
+ ret = max7360_irq_init(max7360, np);
+ if (ret)
+ return ret;
+
+ ret = max7360_device_init(max7360);
+ if (ret) {
+ dev_err(max7360->dev, "failed to add child devices\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int max7360_remove(struct i2c_client *client)
+{
+ struct max7360 *max7360 = i2c_get_clientdata(client);
+
+ mfd_remove_devices(max7360->dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int max7360_suspend(struct device *dev)
+{
+ return 0;
+}
+
+static int max7360_resume(struct device *dev)
+{
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(max7360_dev_pm_ops, max7360_suspend, max7360_resume);
+
+static const struct of_device_id max7360_match[] = {
+ { .compatible = "maxim,max7360" },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, max7360_match);
+
+static const struct i2c_device_id max7360_id[] = {
+ { "max7360", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max7360_id);
+
+static struct i2c_driver max7360_driver = {
+ .driver = {
+ .name = "max7360",
+ .pm = &max7360_dev_pm_ops,
+ .of_match_table = max7360_match,
+ },
+ .probe = max7360_probe,
+ .remove = max7360_remove,
+ .id_table = max7360_id,
+};
+
+static int __init max7360_init(void)
+{
+ return i2c_add_driver(&max7360_driver);
+}
+subsys_initcall(max7360_init);
+
+static void __exit max7360_exit(void)
+{
+ i2c_del_driver(&max7360_driver);
+}
+module_exit(max7360_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MAX7360 MFD core driver");
diff --git a/include/linux/mfd/max7360.h b/include/linux/mfd/max7360.h
new file mode 100644
index 0000000..d139ddd
--- /dev/null
+++ b/include/linux/mfd/max7360.h
@@ -0,0 +1,130 @@
+#ifndef __LINUX_MFD_MAX7360_H
+#define __LINUX_MFD_MAX7360_H
+#include <linux/regmap.h>
+
+#define MAX7360_MAX_KEY_ROWS 8
+#define MAX7360_MAX_KEY_COLS 8
+#define MAX7360_MAX_KEY_NUM (MAX7360_MAX_KEY_ROWS * MAX7360_MAX_KEY_COLS)
+#define MAX7360_ROW_SHIFT 3
+
+#define MAX7360_MAX_GPIO 8
+#define MAX7360_MAX_GPO 6
+#define MAX7360_COL_GPO_PINS 8
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_KEYFIFO 0x00
+#define MAX7360_REG_CONFIG 0x01
+#define MAX7360_REG_DEBOUNCE 0x02
+#define MAX7360_REG_INTERRUPT 0x03
+#define MAX7360_REG_PORTS 0x04
+#define MAX7360_REG_KEYREP 0x05
+#define MAX7360_REG_SLEEP 0x06
+
+/*
+ * MAX7360 registers
+ */
+#define MAX7360_REG_GPIOCFG 0x40
+#define MAX7360_REG_GPIOCTRL 0x41
+#define MAX7360_REG_GPIODEB 0x42
+#define MAX7360_REG_GPIOCURR 0x43
+#define MAX7360_REG_GPIOOUTM 0x44
+#define MAX7360_REG_PWMCOM 0x45
+#define MAX7360_REG_RTRCFG 0x46
+#define MAX7360_REG_GPIOIN 0x49
+#define MAX7360_REG_RTR_CNT 0x4A
+#define MAX7360_REG_PWMBASE 0x50
+#define MAX7360_REG_PWMCFG 0x58
+
+
+#define MAX7360_REG_PORTCFGBASE 0x58
+
+/*
+ * Configuration register bits
+ */
+#define MAX7360_CFG_SLEEP (1 << 7)
+#define MAX7360_CFG_INTERRUPT (1 << 5)
+#define MAX7360_CFG_KEY_RELEASE (1 << 3)
+#define MAX7360_CFG_WAKEUP (1 << 1)
+#define MAX7360_CFG_TIMEOUT (1 << 0)
+
+/*
+ * Autosleep register values (ms)
+ */
+#define MAX7360_AUTOSLEEP_8192 0x01
+#define MAX7360_AUTOSLEEP_4096 0x02
+#define MAX7360_AUTOSLEEP_2048 0x03
+#define MAX7360_AUTOSLEEP_1024 0x04
+#define MAX7360_AUTOSLEEP_512 0x05
+#define MAX7360_AUTOSLEEP_256 0x06
+
+#define MAX7360_INT_INTI 0
+#define MAX7360_INT_INTK 1
+
+#define MAX7360_INT_GPIO 0
+#define MAX7360_INT_KEYPAD 1
+#define MAX7360_INT_ROTARY 2
+
+#define MAX7360_NR_INTERNAL_IRQS 3
+
+struct max7360 {
+ spinlock_t lock; /* lock access to the structure */
+ struct device *dev;
+ struct i2c_client *i2c;
+ struct irq_domain *domain;
+ struct regmap *regmap;
+
+ int irq_base;
+ int num_gpio;
+ int shared_irq;
+ int inti;
+ int intk;
+ u8 gpio_pins;
+ u8 col_count;
+ u8 gpo_count;
+};
+
+static inline int max7360_read_reg(struct max7360 *max7360, int reg)
+{
+ unsigned int ival;
+ int ret;
+
+ ret = regmap_read(max7360->regmap, reg, &ival);
+ if (!ret)
+ return ival;
+ return 0;
+}
+
+static inline int max7360_write_reg(struct max7360 *max7360, u8 reg, u8 val)
+{
+ return regmap_write(max7360->regmap, reg, val);
+}
+
+static inline int max7360_set_bits(struct max7360 *max7360, u8 reg,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, bit_mask);
+}
+
+static inline int max7360_clr_bits(struct max7360 *max7360, u8 reg,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, 0);
+}
+
+static inline int max7360_update(struct max7360 *max7360, u8 reg, u8 val,
+ unsigned int bit_mask)
+{
+ return regmap_update_bits(max7360->regmap, reg, bit_mask, val);
+}
+
+int max7360_request_pin(struct max7360 *max7360, u8 pin);
+void max7360_free_pin(struct max7360 *max7360, u8 pin);
+
+void max7360_take_catnap(struct max7360 *max7360);
+void max7360_fall_deepsleep(struct max7360 *max7360);
+
+int max7360_request_gpo_pin_count(struct max7360 *max7360, u8 count);
+int max7360_request_col_count(struct max7360 *max7360, u8 count);
+
+#endif
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v8 2/3] backlight arcxcnn devicetree bindings for ArcticSand
From: Lee Jones @ 2017-04-25 8:17 UTC (permalink / raw)
To: Olimpiu Dejeu
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, bdodge-eV7fy4qpoLhpLGFMi4vTTA,
joe-6d6DIl74uiNBDgjK7y7TUQ, medasaro-eV7fy4qpoLhpLGFMi4vTTA,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1489607133-7870-2-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
On Wed, 15 Mar 2017, Olimpiu Dejeu wrote:
> backlight: Add devicetree bindings for the Arctic Sand backlight driver
> This patch provides devicetree bindings for the Arctic Sand
> driver submitted in the previous patch
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Daniel Thompson <daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> ---
> v7 => v8:
> - Version updated to match other patch in set. No other changes.
> v6 => v7:
> - Version updated to match other patch in set. No other changes.
> v5 => v6:
> - Version updated to match other patch in set. No other changes.
> v4 => v5:
> - Added spaces for increased readability per Lee Jones
> v3 => v4:
> - Added spaces for increased readability per Lee Jones
> v2 => v3:
> - Version updated to match other patch in set. No other changes.
> v1 => v2:
> - Version updated to match other patch in set. No other changes.
>
> .../bindings/leds/backlight/arcxcnn_bl.txt | 33 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
Applied, thanks.
> diff --git a/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> new file mode 100644
> index 0000000..ecb7731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
> @@ -0,0 +1,33 @@
> +Binding for ArcticSand arc2c0608 LED driver
> +
> +Required properties:
> +- compatible: should be "arc,arc2c0608"
> +- reg: slave address
> +
> +Optional properties:
> +- default-brightness: brightness value on boot, value from: 0-4095
> +- label: The name of the backlight device
> + See Documentation/devicetree/bindings/leds/common.txt
> +- led-sources: List of enabled channels from 0 to 5.
> + See Documentation/devicetree/bindings/leds/common.txt
> +
> +- arc,led-config-0: setting for register ILED_CONFIG_0
> +- arc,led-config-1: setting for register ILED_CONFIG_1
> +- arc,dim-freq: PWM mode frequence setting (bits [3:0] used)
> +- arc,comp-config: setting for register CONFIG_COMP
> +- arc,filter-config: setting for register FILTER_CONFIG
> +- arc,trim-config: setting for register IMAXTUNE
> +
> +Note: Optional properties not specified will default to values in IC EPROM
> +
> +Example:
> +
> +arc2c0608@30 {
> + compatible = "arc,arc2c0608";
> + reg = <0x30>;
> + default-brightness = <500>;
> + label = "lcd-backlight";
> + linux,default-trigger = "backlight";
> + led-sources = <0 1 2 5>;
> +};
> +
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v9 3/3] backlight arcxcnn add support for ArcticSand devices
From: Lee Jones @ 2017-04-25 8:19 UTC (permalink / raw)
To: Olimpiu Dejeu
Cc: robh-DgEjT+Ai2ygdnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, bdodge-eV7fy4qpoLhpLGFMi4vTTA,
joe-6d6DIl74uiNBDgjK7y7TUQ, medasaro-eV7fy4qpoLhpLGFMi4vTTA,
daniel.thompson-QSEj5FYQhm4dnm+yROfE0A,
lkp-ral2JQCrhuEAvxtiuMwx3w, fengguang.wu-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <1490115528-11044-3-git-send-email-olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
On Tue, 21 Mar 2017, Olimpiu Dejeu wrote:
> backlight: Add support for Arctic Sand LED backlight driver chips
> This driver provides support for the Arctic Sand arc2c0608 chip,
> and provides a framework to support future devices.
> Reviewed-by: Daniel Thompson <daniel.thompson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> ---
> v8 => v9:
> - Addressing kbuild test robot WARNING: PTR_ERR_OR_ZERO can be used
> v7 => v8:
> - Version updated to match other patch in set. No other changes.
> v6 => v7:
> - Addressing issues brought up by Daniel Thompson
> v5 => v6:
> - Addressing issues brought up by Daniel Thompson
> v4 => v5:
> - Code style changes per Joe Perches and Jingoo Han
> v3 => v4:
> - Code style changes per Joe Perches and Jingoo Han
> v2 => v3:
> - Renamed variables to comply with conventions on naming
> - Corrected device name in arcxcnn.h
> v1 => v2:
> - Removed "magic numbers" to initialize registers
> - Cleaned up device tree bindings
> - Fixed code style to address comments and pass "checkpatch"
> - Removed unneeded debug and testing code
>
>
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/arcxcnn_bl.c | 419 +++++++++++++++++++++++++++++++++++
> 3 files changed, 427 insertions(+)
> create mode 100644 drivers/video/backlight/arcxcnn_bl.c
Applied this (and the correct (v9) version of [PATCH 2/3] too).
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 5ffa4b4..4e1d2ad 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -460,6 +460,13 @@ config BACKLIGHT_BD6107
> help
> If you have a Rohm BD6107 say Y to enable the backlight driver.
>
> +config BACKLIGHT_ARCXCNN
> + tristate "Backlight driver for the Arctic Sands ARCxCnnnn family"
> + depends on I2C
> + help
> + If you have an ARCxCnnnn family backlight say Y to enable
> + the backlight driver.
> +
> endif # BACKLIGHT_CLASS_DEVICE
>
> endif # BACKLIGHT_LCD_SUPPORT
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 16ec534..8905129 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -55,3 +55,4 @@ obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
> obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
> obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
> +obj-$(CONFIG_BACKLIGHT_ARCXCNN) += arcxcnn_bl.o
> diff --git a/drivers/video/backlight/arcxcnn_bl.c b/drivers/video/backlight/arcxcnn_bl.c
> new file mode 100644
> index 0000000..e01b1b0
> --- /dev/null
> +++ b/drivers/video/backlight/arcxcnn_bl.c
> @@ -0,0 +1,419 @@
> +/*
> + * Backlight driver for ArcticSand ARC_X_C_0N_0N Devices
> + *
> + * Copyright 2016 ArcticSand, Inc.
> + * Author : Brian Dodge <bdodge-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +enum arcxcnn_chip_id {
> + ARC2C0608
> +};
> +
> +/**
> + * struct arcxcnn_platform_data
> + * @name : Backlight driver name (NULL will use default)
> + * @initial_brightness : initial value of backlight brightness
> + * @leden : initial LED string enables, upper bit is global on/off
> + * @led_config_0 : fading speed (period between intensity steps)
> + * @led_config_1 : misc settings, see datasheet
> + * @dim_freq : pwm dimming frequency if in pwm mode
> + * @comp_config : misc config, see datasheet
> + * @filter_config : RC/PWM filter config, see datasheet
> + * @trim_config : full scale current trim, see datasheet
> + */
> +struct arcxcnn_platform_data {
> + const char *name;
> + u16 initial_brightness;
> + u8 leden;
> + u8 led_config_0;
> + u8 led_config_1;
> + u8 dim_freq;
> + u8 comp_config;
> + u8 filter_config;
> + u8 trim_config;
> +};
> +
> +#define ARCXCNN_CMD 0x00 /* Command Register */
> +#define ARCXCNN_CMD_STDBY 0x80 /* I2C Standby */
> +#define ARCXCNN_CMD_RESET 0x40 /* Reset */
> +#define ARCXCNN_CMD_BOOST 0x10 /* Boost */
> +#define ARCXCNN_CMD_OVP_MASK 0x0C /* --- Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_XXV 0x0C /* <rsvrd> Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_20V 0x08 /* 20v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_24V 0x04 /* 24v Over Voltage Threshold */
> +#define ARCXCNN_CMD_OVP_31V 0x00 /* 31.4v Over Voltage Threshold */
> +#define ARCXCNN_CMD_EXT_COMP 0x01 /* part (0) or full (1) ext. comp */
> +
> +#define ARCXCNN_CONFIG 0x01 /* Configuration */
> +#define ARCXCNN_STATUS1 0x02 /* Status 1 */
> +#define ARCXCNN_STATUS2 0x03 /* Status 2 */
> +#define ARCXCNN_FADECTRL 0x04 /* Fading Control */
> +#define ARCXCNN_ILED_CONFIG 0x05 /* ILED Configuration */
> +#define ARCXCNN_ILED_DIM_PWM 0x00 /* config dim mode pwm */
> +#define ARCXCNN_ILED_DIM_INT 0x04 /* config dim mode internal */
> +#define ARCXCNN_LEDEN 0x06 /* LED Enable Register */
> +#define ARCXCNN_LEDEN_ISETEXT 0x80 /* Full-scale current set extern */
> +#define ARCXCNN_LEDEN_MASK 0x3F /* LED string enables mask */
> +#define ARCXCNN_LEDEN_BITS 0x06 /* Bits of LED string enables */
> +#define ARCXCNN_LEDEN_LED1 0x01
> +#define ARCXCNN_LEDEN_LED2 0x02
> +#define ARCXCNN_LEDEN_LED3 0x04
> +#define ARCXCNN_LEDEN_LED4 0x08
> +#define ARCXCNN_LEDEN_LED5 0x10
> +#define ARCXCNN_LEDEN_LED6 0x20
> +
> +#define ARCXCNN_WLED_ISET_LSB 0x07 /* LED ISET LSB (in upper nibble) */
> +#define ARCXCNN_WLED_ISET_LSB_SHIFT 0x04 /* ISET LSB Left Shift */
> +#define ARCXCNN_WLED_ISET_MSB 0x08 /* LED ISET MSB (8 bits) */
> +
> +#define ARCXCNN_DIMFREQ 0x09
> +#define ARCXCNN_COMP_CONFIG 0x0A
> +#define ARCXCNN_FILT_CONFIG 0x0B
> +#define ARCXCNN_IMAXTUNE 0x0C
> +#define ARCXCNN_ID_MSB 0x1E
> +#define ARCXCNN_ID_LSB 0x1F
> +
> +#define MAX_BRIGHTNESS 4095
> +#define INIT_BRIGHT 60
> +
> +struct arcxcnn {
> + struct i2c_client *client;
> + struct backlight_device *bl;
> + struct device *dev;
> + struct arcxcnn_platform_data *pdata;
> +};
> +
> +static int arcxcnn_update_field(struct arcxcnn *lp, u8 reg, u8 mask, u8 data)
> +{
> + int ret;
> + u8 tmp;
> +
> + ret = i2c_smbus_read_byte_data(lp->client, reg);
> + if (ret < 0) {
> + dev_err(lp->dev, "failed to read 0x%.2x\n", reg);
> + return ret;
> + }
> +
> + tmp = (u8)ret;
> + tmp &= ~mask;
> + tmp |= data & mask;
> +
> + return i2c_smbus_write_byte_data(lp->client, reg, tmp);
> +}
> +
> +static int arcxcnn_set_brightness(struct arcxcnn *lp, u32 brightness)
> +{
> + int ret;
> + u8 val;
> +
> + /* lower nibble of brightness goes in upper nibble of LSB register */
> + val = (brightness & 0xF) << ARCXCNN_WLED_ISET_LSB_SHIFT;
> + ret = i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_WLED_ISET_LSB, val);
> + if (ret < 0)
> + return ret;
> +
> + /* remaining 8 bits of brightness go in MSB register */
> + val = (brightness >> 4);
> + return i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_WLED_ISET_MSB, val);
> +}
> +
> +static int arcxcnn_bl_update_status(struct backlight_device *bl)
> +{
> + struct arcxcnn *lp = bl_get_data(bl);
> + u32 brightness = bl->props.brightness;
> + int ret;
> +
> + if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> + brightness = 0;
> +
> + ret = arcxcnn_set_brightness(lp, brightness);
> + if (ret)
> + return ret;
> +
> + /* set power-on/off/save modes */
> + return arcxcnn_update_field(lp, ARCXCNN_CMD, ARCXCNN_CMD_STDBY,
> + (bl->props.power == 0) ? 0 : ARCXCNN_CMD_STDBY);
> +}
> +
> +static const struct backlight_ops arcxcnn_bl_ops = {
> + .options = BL_CORE_SUSPENDRESUME,
> + .update_status = arcxcnn_bl_update_status,
> +};
> +
> +static int arcxcnn_backlight_register(struct arcxcnn *lp)
> +{
> + struct backlight_properties *props;
> + const char *name = lp->pdata->name ? : "arctic_bl";
> +
> + props = devm_kzalloc(lp->dev, sizeof(*props), GFP_KERNEL);
> + if (!props)
> + return -ENOMEM;
> +
> + props->type = BACKLIGHT_PLATFORM;
> + props->max_brightness = MAX_BRIGHTNESS;
> +
> + if (lp->pdata->initial_brightness > props->max_brightness)
> + lp->pdata->initial_brightness = props->max_brightness;
> +
> + props->brightness = lp->pdata->initial_brightness;
> +
> + lp->bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> + &arcxcnn_bl_ops, props);
> + return PTR_ERR_OR_ZERO(lp->bl);
> +}
> +
> +static void arcxcnn_parse_dt(struct arcxcnn *lp)
> +{
> + struct device *dev = lp->dev;
> + struct device_node *node = dev->of_node;
> + u32 prog_val, num_entry, entry, sources[ARCXCNN_LEDEN_BITS];
> + int ret;
> +
> + /* device tree entry isn't required, defaults are OK */
> + if (!node)
> + return;
> +
> + ret = of_property_read_string(node, "label", &lp->pdata->name);
> + if (ret < 0)
> + lp->pdata->name = NULL;
> +
> + ret = of_property_read_u32(node, "default-brightness", &prog_val);
> + if (ret == 0)
> + lp->pdata->initial_brightness = prog_val;
> +
> + ret = of_property_read_u32(node, "arc,led-config-0", &prog_val);
> + if (ret == 0)
> + lp->pdata->led_config_0 = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,led-config-1", &prog_val);
> + if (ret == 0)
> + lp->pdata->led_config_1 = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,dim-freq", &prog_val);
> + if (ret == 0)
> + lp->pdata->dim_freq = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,comp-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->comp_config = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,filter-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->filter_config = (u8)prog_val;
> +
> + ret = of_property_read_u32(node, "arc,trim-config", &prog_val);
> + if (ret == 0)
> + lp->pdata->trim_config = (u8)prog_val;
> +
> + ret = of_property_count_u32_elems(node, "led-sources");
> + if (ret < 0) {
> + lp->pdata->leden = ARCXCNN_LEDEN_MASK; /* all on is default */
> + } else {
> + num_entry = ret;
> + if (num_entry > ARCXCNN_LEDEN_BITS)
> + num_entry = ARCXCNN_LEDEN_BITS;
> +
> + ret = of_property_read_u32_array(node, "led-sources", sources,
> + num_entry);
> + if (ret < 0) {
> + dev_err(dev, "led-sources node is invalid.\n");
> + return;
> + }
> +
> + lp->pdata->leden = 0;
> +
> + /* for each enable in source, set bit in led enable */
> + for (entry = 0; entry < num_entry; entry++) {
> + u8 onbit = 1 << sources[entry];
> +
> + lp->pdata->leden |= onbit;
> + }
> + }
> +}
> +
> +static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> +{
> + struct arcxcnn *lp;
> + int ret;
> +
> + if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> + return -EIO;
> +
> + lp = devm_kzalloc(&cl->dev, sizeof(*lp), GFP_KERNEL);
> + if (!lp)
> + return -ENOMEM;
> +
> + lp->client = cl;
> + lp->dev = &cl->dev;
> + lp->pdata = dev_get_platdata(&cl->dev);
> +
> + /* reset the device */
> + ret = i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> + if (ret)
> + goto probe_err;
> +
> + if (!lp->pdata) {
> + lp->pdata = devm_kzalloc(lp->dev,
> + sizeof(*lp->pdata), GFP_KERNEL);
> + if (!lp->pdata)
> + return -ENOMEM;
> +
> + /* Setup defaults based on power-on defaults */
> + lp->pdata->name = NULL;
> + lp->pdata->initial_brightness = INIT_BRIGHT;
> + lp->pdata->leden = ARCXCNN_LEDEN_MASK;
> +
> + lp->pdata->led_config_0 = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_FADECTRL);
> +
> + lp->pdata->led_config_1 = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_ILED_CONFIG);
> + /* insure dim mode is not default pwm */
> + lp->pdata->led_config_1 |= ARCXCNN_ILED_DIM_INT;
> +
> + lp->pdata->dim_freq = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_DIMFREQ);
> +
> + lp->pdata->comp_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_COMP_CONFIG);
> +
> + lp->pdata->filter_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_FILT_CONFIG);
> +
> + lp->pdata->trim_config = i2c_smbus_read_byte_data(
> + lp->client, ARCXCNN_IMAXTUNE);
> +
> + if (IS_ENABLED(CONFIG_OF))
> + arcxcnn_parse_dt(lp);
> + }
> +
> + i2c_set_clientdata(cl, lp);
> +
> + /* constrain settings to what is possible */
> + if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
> + lp->pdata->initial_brightness = MAX_BRIGHTNESS;
> +
> + /* set initial brightness */
> + ret = arcxcnn_set_brightness(lp, lp->pdata->initial_brightness);
> + if (ret)
> + goto probe_err;
> +
> + /* set other register values directly */
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_FADECTRL,
> + lp->pdata->led_config_0);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_ILED_CONFIG,
> + lp->pdata->led_config_1);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_DIMFREQ,
> + lp->pdata->dim_freq);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_COMP_CONFIG,
> + lp->pdata->comp_config);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_FILT_CONFIG,
> + lp->pdata->filter_config);
> + if (ret)
> + goto probe_err;
> +
> + ret = i2c_smbus_write_byte_data(lp->client, ARCXCNN_IMAXTUNE,
> + lp->pdata->trim_config);
> + if (ret)
> + goto probe_err;
> +
> + /* set initial LED Enables */
> + arcxcnn_update_field(lp, ARCXCNN_LEDEN,
> + ARCXCNN_LEDEN_MASK, lp->pdata->leden);
> +
> + ret = arcxcnn_backlight_register(lp);
> + if (ret)
> + goto probe_register_err;
> +
> + backlight_update_status(lp->bl);
> +
> + return 0;
> +
> +probe_register_err:
> + dev_err(lp->dev,
> + "failed to register backlight.\n");
> +
> +probe_err:
> + dev_err(lp->dev,
> + "failure ret: %d\n", ret);
> + return ret;
> +}
> +
> +static int arcxcnn_remove(struct i2c_client *cl)
> +{
> + struct arcxcnn *lp = i2c_get_clientdata(cl);
> +
> + /* disable all strings (ignore errors) */
> + i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_LEDEN, 0x00);
> + /* reset the device (ignore errors) */
> + i2c_smbus_write_byte_data(lp->client,
> + ARCXCNN_CMD, ARCXCNN_CMD_RESET);
> +
> + lp->bl->props.brightness = 0;
> +
> + backlight_update_status(lp->bl);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id arcxcnn_dt_ids[] = {
> + { .compatible = "arc,arc2c0608" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);
> +
> +static const struct i2c_device_id arcxcnn_ids[] = {
> + {"arc2c0608", ARC2C0608},
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, arcxcnn_ids);
> +
> +static struct i2c_driver arcxcnn_driver = {
> + .driver = {
> + .name = "arcxcnn_bl",
> + .of_match_table = of_match_ptr(arcxcnn_dt_ids),
> + },
> + .probe = arcxcnn_probe,
> + .remove = arcxcnn_remove,
> + .id_table = arcxcnn_ids,
> +};
> +module_i2c_driver(arcxcnn_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Brian Dodge <bdodge-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>");
> +MODULE_DESCRIPTION("ARCXCNN Backlight driver");
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-25 8:32 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
OpenBMC Maillist, Ryan Chen
In-Reply-To: <1493086747.25766.264.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Adding Ryan.
On Mon, Apr 24, 2017 at 7:19 PM, Benjamin Herrenschmidt
<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> wrote:
> On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
>> > > +struct aspeed_i2c_bus {
>> > > + struct i2c_adapter adap;
>> > > + struct device *dev;
>> > > + void __iomem *base;
>> > > + /* Synchronizes I/O mem access to base. */
>> > > + spinlock_t lock;
>> >
>> > I am not entirely convinced we need that lock. The i2c core will
>> > take a mutex protecting all operations on the bus. So we only need
>> > to synchronize between our "xfer" code and our interrupt handler.
>>
>> You are right if both having slave and master active at the same time
>> was not possible; however, it is.
>
> Right, I somewhat forgot about the slave case.
>
> ...
>
>> > Some of those error states probably also warrant a reset of the
>> > controller,
>> > I think aspeed does that in the SDK.
>>
>> For timeout and cmd_err, I do not see any argument against it; it
>> sounds like we are in a very messed up, very unknown state, so full
>> reset is probably the best last resort.
>
> Yup.
>
>> For SDA staying pulled down, I
>> think we can say with reasonable confidence that some device on our
>> bus is behaving very badly and I am not convinced that resetting the
>> controller is likely to do anything to help;
>
> Right. Hammering with STOPs and pray ...
I think sending recovery mode sends stops as a part of the recovery
algorithm it executes.
>
>> that being said, I really
>> do not have any good ideas to address that. So maybe praying and
>> resetting the controller is *the most reasonable thing to do.* I
>> would like to know what you think we should do in that case.
>
> Well, there's a (small ?) chance that it's a controller bug asserting
> the line so ... but there's little we can do if not.
True.
>
>> While I was thinking about this I also realized that the SDA line
>> check after recovery happens in the else branch, but SCL line check
>> does not happen after we attempt to STOP if SCL is hung. If we decide
>> to make special note SDA being hung by a device that won't let go, we
>> might want to make a special note that SCL is hung by a device that
>> won't let go. Just a thought.
>
> Maybe. Or just "unrecoverable error"... hopefully these don't happen
> too often ... We had cases of a TPM misbehaving like that.
Yeah, definitely should print something out.
>
>> > > +out:
>>
>> ...
>> > What about I2C_M_NOSTART ?
>> >
>> > Not that I've ever seen it used... ;-)
>>
>> Right now I am not doing any of the protocol mangling options, but I
>> can add them in if you think it is important for initial support.
>
> No, not important, we can add that later if it ever becomes useful.
>
> ...
>
>> > In general, you always ACK all interrupts first. Then you handle
>> > the bits you have harvested.
>> >
>>
>> The documentation says to ACK the interrupt after handling in the RX
>> case:
>>
>> <<<
>> S/W needs to clear this status bit to allow next data receiving.
>> > > >
>>
>> I will double check with Ryan to make sure TX works the same way.
>>
>> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
>> > > + (!bus->msgs && bus->master_state !=
>> > > ASPEED_I2C_MASTER_STOP)) {
>>
>> ...
>> >
>> > I would set master_state to "RECOVERY" (new state ?) and ensure
>> > those things are caught if they happen outside of a recovery.
>
> I replied privately ... as long as we ack before we start a new command
> we should be ok but we shouldn't ack after.
>
> Your latest patch still does that. It will do things like start a STOP
> command *then* ack the status bits. I'm pretty sure that's bogus.
>
> That way it's a lot simpler to simply move the
>
> writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>
> To either right after the readl of the status reg at the beginning of
> aspeed_i2c_master_irq().
>
> I would be very surprised if that didn't work properly and wasn't much
> safer than what you are currently doing.
I think I tried your way and it worked. In anycase, Ryan will be able
to clarify for us.
>
>> Let me know if you still think we need a "RECOVERY" state.
>
> The way you just switch to stop state and store the error for later
> should work I think.
>
>> >
>> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>
>> ...
>> >
>> > > + dev_dbg(bus->dev,
>> > > + "no slave present at %02x", msg-
>> > > >addr);
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> > > + bus->cmd_err = -EIO;
>> > > + do_stop(bus);
>> > > + goto out_no_complete;
>> > > + } else {
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> > > + if (msg->flags & I2C_M_RD)
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_RX;
>> > > + else
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_TX_FIRST;
>> >
>> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need
>> > to handle this here ? A quick look at the TX_FIRST case makes
>> > me think we are ok there but I'm not sure about the RX case.
>>
>> I did not think that there is an SMBUS_QUICK RX. Could you point me
>> to an example?
>
> Not so much an RX, it's more like you are sending a 1-bit data in
> the place of the Rd/Wr bit. So you have a read with a lenght of 0,
> I don't think in that case you should set ASPEED_I2CD_M_RX_CMD in
> __aspeed_i2c_do_start
Forget what I said, I was just not thinking about the fact that SMBus
emulation causes the data bit to be encoded as the R/W flag. I see
what you are saying; you are correct.
>
>> > I'm not sure the RX case is tight also. What completion does the
>> > HW give you for the address cycle ? Won't you get that before it
>> > has received the first character ? IE. You fall through to
>> > the read case of the state machine with the read potentially
>> > not complete yet no ?
>>
>> ...
>> > > + case ASPEED_I2C_MASTER_RX:
>> > > + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> > > + dev_err(bus->dev, "master failed to RX");
>> > > + goto out_complete;
>> > > + }
>> >
>> > See my comment above for a bog standard i2c_read. Aren't you
>> > getting
>> > the completion for the address before the read is even started ?
>>
>> In practice no, but it is probably best to be safe :-)
>
> Yup :)
>> >
>> > > + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> > > +
>> > > + recv_byte = aspeed_i2c_read(bus,
>> > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> > > + msg->buf[bus->buf_index++] = recv_byte;
>> > > +
>> > > + if (msg->flags & I2C_M_RECV_LEN &&
>> > > + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
>> > > + msg->len = recv_byte +
>> > > + ((msg->flags &
>> > > I2C_CLIENT_PEC) ? 2 : 1);
>>
>> ...
>> > > + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> > > + | ((clk_low <<
>> > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> > > + | (base_clk &
>> > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> > > +}
>> >
>> > As I think I mentioned earlier, the AST2500 has a slightly
>> > different
>> > register layout which support larger values for high and low, thus
>> > allowing a finer granularity.
>>
>> I am developing against the 2500.
>
> Yes but we'd like the driver to work with both :-)
Right, I thought you were making an assertion about the 2500, if you
are making an assertion about the 2400, I do not know and do not have
one handy.
>
>> > BTW. In case you haven't, I would suggest you copy/paste the above
>> > in
>> > a userspace app and run it for all frequency divisors and see if
>> > your
>> > results match the aspeed table :)
>>
>> Good call.
>
> If you end up doing that, can you shoot it my way ? I can take care
> of making sure it's all good for the 2400.
Will do.
>
>> > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> > > + struct platform_device *pdev)
>> > > +{
>> > > + u32 clk_freq, divisor;
>> > > + struct clk *pclk;
>> > > + int ret;
>> > > +
>> > > + pclk = devm_clk_get(&pdev->dev, NULL);
>> > > + if (IS_ERR(pclk)) {
>> > > + dev_err(&pdev->dev, "clk_get failed\n");
>> > > + return PTR_ERR(pclk);
>> > > + }
>> > > + ret = of_property_read_u32(pdev->dev.of_node,
>> > > + "clock-frequency", &clk_freq);
>> >
>> > See my previous comment about calling that 'bus-frequency' rather
>> > than 'clock-frequency'.
>> >
>> > > + if (ret < 0) {
>> > > + dev_err(&pdev->dev,
>> > > + "Could not read clock-frequency
>> > > property\n");
>> > > + clk_freq = 100000;
>> > > + }
>> > > + divisor = clk_get_rate(pclk) / clk_freq;
>> > > + /* We just need the clock rate, we don't actually use the
>> > > clk object. */
>> > > + devm_clk_put(&pdev->dev, pclk);
>> > > +
>> > > + /* Set AC Timing */
>> > > + if (clk_freq / 1000 > 1000) {
>> > > + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> > > + ASPEED_I2C_FU
>> > > N_CTRL_REG) |
>> > > + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> > > + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> > > + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> > > + ASPEED_I2C_FUN_CTRL_REG);
>> > > +
>> > > + aspeed_i2c_write(bus, 0x3,
>> > > ASPEED_I2C_AC_TIMING_REG2);
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> >
>> > I already discussed by doubts about the above. I can try to scope
>> > it with the EVB if you don't get to it. For now I'd rather take the
>> > code out.
>> >
>> > We should ask aspeed from what frequency the "1T" stuff is useful.
>>
>> Will do, I will try to rope Ryan in on the next review; it will be
>> good for him to get used to working with upstream anyway.
>
> Yup. However, for the sake of getting something upstream (and in
> OpenBMC 4.10 kernel) asap, I would suggest just dropping support
> for those fast speeds for now, we can add them back later.
Alright, that's fine. Still, Ryan, could you provide some context on this?
>
>> >
>> > > + } else {
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> > > + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> > > + ASPEED_I2C_AC_TIMING_REG2);
>> > > + }
>>
>> ...
>> > > + spin_lock_init(&bus->lock);
>> > > + init_completion(&bus->cmd_complete);
>> > > + bus->adap.owner = THIS_MODULE;
>> > > + bus->adap.retries = 0;
>> > > + bus->adap.timeout = 5 * HZ;
>> > > + bus->adap.algo = &aspeed_i2c_algo;
>> > > + bus->adap.algo_data = bus;
>> > > + bus->adap.dev.parent = &pdev->dev;
>> > > + bus->adap.dev.of_node = pdev->dev.of_node;
>> > > + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed
>> > > i2c");
>> >
>> > Another trivial one, should we put some kind of bus number
>> > in that string ?
>>
>> Whoops, looks like I missed this one; I will get to it in the next
>> revision.
>
> Ok. I noticed you missed that in v7, so I assume you mean v8 :-)
Yep, I will get it in v8.
>
>> >
>> > > + bus->dev = &pdev->dev;
>> > > +
>> > > + /* reset device: disable master & slave functions */
>> > > + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
>>
>> ...
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree"
>> in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
From: Brendan Higgins @ 2017-04-25 8:35 UTC (permalink / raw)
To: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, Benjamin Herrenschmidt
Cc: linux-i2c, devicetree, Linux Kernel Mailing List,
OpenBMC Maillist, Brendan Higgins, Ryan Chen
In-Reply-To: <20170424181818.2754-6-brendanhiggins@google.com>
Adding Ryan (sorry to everyone else for the spam).
^ permalink raw reply
* Re: Re: [PATCH v4 07/10] mfd: axp20x: add axp20x-regulator cell for AXP803
From: Icenowy Zheng @ 2017-04-25 8:37 UTC (permalink / raw)
To: lee.jones-QSEj5FYQhm4dnm+yROfE0A
Cc: Thomas Gleixner, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
Liam Girdwood, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170425075717.l4x3qektdgt6kaiq@dell>
于 2017年4月25日 GMT+08:00 下午3:57:17, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 写到:
>On Tue, 25 Apr 2017, Icenowy Zheng wrote:
>
>> As axp20x-regulator now supports AXP803, add a cell for it.
>>
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> Changes in v4:
>> - Added a trailing comma for new cell, for easier further cell
>addition.
>> Changes in v3:
>> - Make the new cell one-liner.
>>
>> drivers/mfd/axp20x.c | 3 ++-
>> drivers/regulator/axp20x-regulator.c | 6 +++---
>
>These 2 changes are orthogonal, thus there is no reason to send them
>bundled into a single patch. Doing so complicates things greatly.
>Please resubmit the two changes separately, so that they may be
>absorbed by our respective subsystems.
Yes. As Chen-Yu said, I wrongly squashed the regulator renaming patch here...
It should be squashed into the regulator patch.
Sorry for it.
>
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 1dc6235778eb..917b6ddc4f15 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -848,7 +848,8 @@ static struct mfd_cell axp803_cells[] = {
>> .name = "axp20x-pek",
>> .num_resources = ARRAY_SIZE(axp803_pek_resources),
>> .resources = axp803_pek_resources,
>> - }
>> + },
>> + { .name = "axp20x-regulator" },
>> };
>>
>> static struct mfd_cell axp806_cells[] = {
>> diff --git a/drivers/regulator/axp20x-regulator.c
>b/drivers/regulator/axp20x-regulator.c
>> index 9356ec8a9a1f..e2608fe770b9 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -311,13 +311,13 @@ static const struct regulator_desc
>axp803_regulators[] = {
>> AXP803_FLDO1_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(2)),
>> AXP_DESC(AXP803, FLDO2, "fldo2", "fldoin", 700, 1450, 50,
>> AXP803_FLDO2_V_OUT, 0x0f, AXP22X_PWR_OUT_CTRL3, BIT(3)),
>> - AXP_DESC_IO(AXP803, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
>> + AXP_DESC_IO(AXP803, LDO_IO0, "ldo-io0", "ips", 700, 3300, 100,
>> AXP22X_LDO_IO0_V_OUT, 0x1f, AXP20X_GPIO0_CTRL, 0x07,
>> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
>> - AXP_DESC_IO(AXP803, LDO_IO1, "ldo_io1", "ips", 700, 3300, 100,
>> + AXP_DESC_IO(AXP803, LDO_IO1, "ldo-io1", "ips", 700, 3300, 100,
>> AXP22X_LDO_IO1_V_OUT, 0x1f, AXP20X_GPIO1_CTRL, 0x07,
>> AXP22X_IO_ENABLED, AXP22X_IO_DISABLED),
>> - AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc_ldo", "ips", 3000),
>> + AXP_DESC_FIXED(AXP803, RTC_LDO, "rtc-ldo", "ips", 3000),
>> };
>>
>> static const struct regulator_linear_range axp806_dcdca_ranges[] = {
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v3 0/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle" and "linux,phandle" properties from the internal
device tree. The phandle will still be in the struct device_node
phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
Patch 1 is the phandle related changes.
Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.
Changes from v1:
- patch 1: Remove check in __of_add_phandle_sysfs() that would not
add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
Changes from v1:
- Remove phandle properties in of_attach_node(), before attaching
the node to the live tree.
- Add the phandle sysfs entry for the node in of_attach_node().
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
Frank Rowand (4):
of: remove *phandle properties from expanded device tree
of: make __of_attach_node() static
of: be consistent in form of file mode
of: detect invalid phandle in overlay
drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
drivers/of/of_private.h | 2 +-
drivers/of/overlay.c | 8 ++++---
drivers/of/resolver.c | 23 +-------------------
include/linux/of.h | 1 +
7 files changed, 120 insertions(+), 60 deletions(-)
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/4] of: remove *phandle properties from expanded device tree
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
Remove "phandle", "linux,phandle", and "ibm,phandle" properties from
the internal device tree. The phandle will still be in the struct
device_node phandle field.
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
attached to the live tree. Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
"ibm,phandle" properties will no longer appear in /proc/device-tree (they
will appear as "phandle").
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 48 ++++++++++++++++++++++++++++++++++++++++---
drivers/of/dynamic.c | 54 +++++++++++++++++++++++++++++++++++++------------
drivers/of/fdt.c | 40 +++++++++++++++++++++---------------
drivers/of/of_private.h | 1 +
drivers/of/overlay.c | 4 +---
drivers/of/resolver.c | 23 +--------------------
include/linux/of.h | 1 +
7 files changed, 114 insertions(+), 57 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..41473b1e2445 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
}
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t offset, size_t count)
+{
+ phandle phandle;
+ struct device_node *np;
+
+ np = container_of(bin_attr, struct device_node, attr_phandle);
+ phandle = cpu_to_be32(np->phandle);
+ return memory_read_from_buffer(buf, count, &offset, &phandle,
+ sizeof(phandle));
+}
+
/* always return newly allocated name, caller must free after use */
static const char *safe_name(struct kobject *kobj, const char *orig_name)
{
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
return rc;
}
+/*
+ * In the imported device tree (fdt), phandle is a property. In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_SYSFS))
+ return 0;
+
+ if (!of_kset || !of_node_is_attached(np))
+ return 0;
+
+ if (!np->phandle || np->phandle == 0xffffffff)
+ return 0;
+
+ sysfs_bin_attr_init(&pp->attr);
+ np->attr_phandle.attr.name = "phandle";
+ np->attr_phandle.attr.mode = 0444;
+ np->attr_phandle.size = sizeof(np->phandle);
+ np->attr_phandle.read = of_node_phandle_read;
+
+ rc = sysfs_create_bin_file(&np->kobj, &np->attr_phandle);
+ WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+ return rc;
+}
+
int __of_attach_node_sysfs(struct device_node *np)
{
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;
+ __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
@@ -2097,9 +2141,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
int id, len;
/* Skip those we do not want to proceed */
- if (!strcmp(pp->name, "name") ||
- !strcmp(pp->name, "phandle") ||
- !strcmp(pp->name, "linux,phandle"))
+ if (!strcmp(pp->name, "name"))
continue;
np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..59545b8fbf46 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
void __of_attach_node(struct device_node *np)
{
- const __be32 *phandle;
- int sz;
-
- np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
- np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
-
- phandle = __of_get_property(np, "phandle", &sz);
- if (!phandle)
- phandle = __of_get_property(np, "linux,phandle", &sz);
- if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
- phandle = __of_get_property(np, "ibm,phandle", &sz);
- np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
-
np->child = NULL;
np->sibling = np->parent->child;
np->parent->child = np;
@@ -243,11 +230,47 @@ void __of_attach_node(struct device_node *np)
int of_attach_node(struct device_node *np)
{
struct of_reconfig_data rd;
+ struct property *prev;
+ struct property *prop;
+ struct property *save_next;
unsigned long flags;
+ const __be32 *phandle;
+ int sz;
memset(&rd, 0, sizeof(rd));
rd.dn = np;
+ np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
+ np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
+
+ phandle = __of_get_property(np, "phandle", &sz);
+ if (!phandle)
+ phandle = __of_get_property(np, "linux,phandle", &sz);
+ if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
+ phandle = __of_get_property(np, "ibm,phandle", &sz);
+ np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
+
+ /* remove phandle properties from node */
+ prev = NULL;
+ for (prop = np->properties; prop != NULL; ) {
+ save_next = prop->next;
+ if (!strcmp(prop->name, "phandle") ||
+ !strcmp(prop->name, "ibm,phandle") ||
+ !strcmp(prop->name, "linux,phandle")) {
+ if (prev)
+ prev->next = prop->next;
+ else
+ np->properties = prop->next;
+ prop->next = np->deadprops;
+ np->deadprops = prop;
+ } else {
+ prev = prop;
+ }
+ prop = save_next;
+ }
+
+ __of_add_phandle_sysfs(np);
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
__of_attach_node(np);
@@ -429,6 +452,7 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
/* Iterate over and duplicate all properties */
if (np) {
struct property *pp, *new_pp;
+ node->phandle = np->phandle;
for_each_property_of_node(np, pp) {
new_pp = __of_prop_dup(pp, GFP_KERNEL);
if (!new_pp)
@@ -441,6 +465,10 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
}
}
}
+
+ node->name = __of_get_property(node, "name", NULL) ? : "<NULL>";
+ node->type = __of_get_property(node, "device_type", NULL) ? : "<NULL>";
+
return node;
err_prop:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..270f31b0c259 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -181,6 +181,8 @@ static void populate_properties(const void *blob,
const __be32 *val;
const char *pname;
u32 sz;
+ int prop_is_phandle = 0;
+ int prop_is_ibm_phandle = 0;
val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
if (!val) {
@@ -196,11 +198,6 @@ static void populate_properties(const void *blob,
if (!strcmp(pname, "name"))
has_name = true;
- pp = unflatten_dt_alloc(mem, sizeof(struct property),
- __alignof__(struct property));
- if (dryrun)
- continue;
-
/* We accept flattened tree phandles either in
* ePAPR-style "phandle" properties, or the
* legacy "linux,phandle" properties. If both
@@ -208,23 +205,34 @@ static void populate_properties(const void *blob,
* will get weird. Don't do that.
*/
if (!strcmp(pname, "phandle") ||
- !strcmp(pname, "linux,phandle")) {
- if (!np->phandle)
- np->phandle = be32_to_cpup(val);
- }
+ !strcmp(pname, "linux,phandle"))
+ prop_is_phandle = 1;
/* And we process the "ibm,phandle" property
* used in pSeries dynamic device tree
* stuff
*/
- if (!strcmp(pname, "ibm,phandle"))
- np->phandle = be32_to_cpup(val);
+ if (!strcmp(pname, "ibm,phandle")) {
+ prop_is_phandle = 1;
+ prop_is_ibm_phandle = 1;
+ }
+
+ if (!prop_is_phandle)
+ pp = unflatten_dt_alloc(mem, sizeof(struct property),
+ __alignof__(struct property));
- pp->name = (char *)pname;
- pp->length = sz;
- pp->value = (__be32 *)val;
- *pprev = pp;
- pprev = &pp->next;
+ if (dryrun)
+ continue;
+
+ if (!prop_is_phandle) {
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
+ *pprev = pp;
+ pprev = &pp->next;
+ } else if (prop_is_ibm_phandle || !np->phandle) {
+ np->phandle = be32_to_cpup(val);
+ }
}
/* With version 0x10 we may not have the name property,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..33f11a5384f3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -47,6 +47,7 @@ extern int of_property_notify(int action, struct device_node *np,
extern void of_node_release(struct kobject *kobj);
extern int __of_changeset_apply(struct of_changeset *ocs);
extern int __of_changeset_revert(struct of_changeset *ocs);
+extern int __of_add_phandle_sysfs(struct device_node *np);
#else /* CONFIG_OF_DYNAMIC */
static inline int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *old_prop)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..ca0b85f5deb1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -101,9 +101,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
tprop = of_find_property(target, prop->name, NULL);
/* special properties are not meant to be updated (silent NOP) */
- if (of_prop_cmp(prop->name, "name") == 0 ||
- of_prop_cmp(prop->name, "phandle") == 0 ||
- of_prop_cmp(prop->name, "linux,phandle") == 0)
+ if (!of_prop_cmp(prop->name, "name"))
return 0;
propn = __of_prop_dup(prop, GFP_KERNEL);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..624cbaeae0a4 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -71,30 +71,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
struct device_node *child;
- struct property *prop;
- phandle phandle;
/* adjust node's phandle in node */
if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
overlay->phandle += phandle_delta;
- /* copy adjusted phandle into *phandle properties */
- for_each_property_of_node(overlay, prop) {
-
- if (of_prop_cmp(prop->name, "phandle") &&
- of_prop_cmp(prop->name, "linux,phandle"))
- continue;
-
- if (prop->length < 4)
- continue;
-
- phandle = be32_to_cpup(prop->value);
- if (phandle == OF_PHANDLE_ILLEGAL)
- continue;
-
- *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
- }
-
for_each_child_of_node(overlay, child)
adjust_overlay_phandles(child, phandle_delta);
}
@@ -197,9 +178,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
for_each_property_of_node(local_fixups, prop_fix) {
/* skip properties added automatically */
- if (!of_prop_cmp(prop_fix->name, "name") ||
- !of_prop_cmp(prop_fix->name, "phandle") ||
- !of_prop_cmp(prop_fix->name, "linux,phandle"))
+ if (!of_prop_cmp(prop_fix->name, "name"))
continue;
if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..4e86e05853f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -61,6 +61,7 @@ struct device_node {
struct kobject kobj;
unsigned long _flags;
void *data;
+ struct bin_attribute attr_phandle;
#if defined(CONFIG_SPARC)
const char *path_component_name;
unsigned int unique_id;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/4] of: make __of_attach_node() static
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
__of_attach_node() is not used outside of drivers/of/dynamic.c. Make
it static and remove it from drivers/of/of_private.h.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/dynamic.c | 2 +-
drivers/of/of_private.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 59545b8fbf46..0b9cf6d5914c 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
return of_reconfig_notify(action, &pr);
}
-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
{
np->child = NULL;
np->sibling = np->parent->child;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 33f11a5384f3..b1ff70e1fdda 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,7 +79,6 @@ extern int __of_update_property(struct device_node *np,
extern void __of_update_property_sysfs(struct device_node *np,
struct property *newprop, struct property *oldprop);
-extern void __of_attach_node(struct device_node *np);
extern int __of_attach_node_sysfs(struct device_node *np);
extern void __of_detach_node(struct device_node *np);
extern void __of_detach_node_sysfs(struct device_node *np);
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 3/4] of: be consistent in form of file mode
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
checkpatch whined about using S_IRUGO instead of octal equivalent
when adding phandle sysfs code, so used octal in that patch.
Change other instances of the S_* constants in the same file to
the octal form.
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 41473b1e2445..9fe42346925b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -168,7 +168,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
sysfs_bin_attr_init(&pp->attr);
pp->attr.attr.name = safe_name(&np->kobj, pp->name);
- pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+ pp->attr.attr.mode = secure ? 0400 : 0444;
pp->attr.size = secure ? 0 : pp->length;
pp->attr.read = of_node_property_read;
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 4/4] of: detect invalid phandle in overlay
From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw)
To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up
properties that use the previously existing phandle.
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
drivers/of/overlay.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index ca0b85f5deb1..20ab49d2f7a4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -130,6 +130,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
/* NOTE: Multiple mods of created nodes not supported */
tchild = of_get_child_by_name(target, cname);
if (tchild != NULL) {
+ /* new overlay phandle value conflicts with existing value */
+ if (child->phandle)
+ return -EINVAL;
+
/* apply overlay recursively */
ret = of_overlay_apply_one(ov, tchild, child);
of_node_put(tchild);
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* 33027 devicetree
From: arywhite007-/E1597aS9LQAvxtiuMwx3w @ 2017-04-25 8:50 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: 09469566543158.zip --]
[-- Type: application/zip, Size: 2600 bytes --]
^ permalink raw reply
* RE: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Ryan Chen @ 2017-04-25 8:50 UTC (permalink / raw)
To: Brendan Higgins, Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, Linux Kernel Mailing List,
OpenBMC Maillist
In-Reply-To: <CAFd5g44ioW0PdFVOhY-ep8TXBNg6qC7E5yjOVf3MEdY9bPgEDA@mail.gmail.com>
Hello All,
ASPEED_I2CD_M_SDA_DRIVE_1T_EN, ASPEED_I2CD_SDA_DRIVE_1T_EN is specific for some case usage.
For example, if i2c bus is use on "high speed" and "single slave and master" and i2c bus is too long. It need drive SDA or SCL less lunacy. It would enable it.
Otherwise, don’t enable it. especially in multi-master. It can’t be enable.
Best Regards,
Ryan
信驊科技股份有限公司
ASPEED Technology Inc.
2F,No.15,Industry East Road 4.,Hsinchu Science Park, Hsinchu City 30077, Taiwan
Tel: 886-3-578-9568 #857
Fax: 886-3-578-9586
************* Email Confidentiality Notice ********************
DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.
-----Original Message-----
From: Brendan Higgins [mailto:brendanhiggins@google.com]
Sent: Tuesday, April 25, 2017 4:32 PM
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Wolfram Sang <wsa@the-dreams.de>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Thomas Gleixner <tglx@linutronix.de>; Jason Cooper <jason@lakedaemon.net>; Marc Zyngier <marc.zyngier@arm.com>; Joel Stanley <joel@jms.id.au>; Vladimir Zapolskiy <vz@mleia.com>; Kachalov Anton <mouse@mayc.ru>; Cédric Le Goater <clg@kaod.org>; linux-i2c@vger.kernel.org; devicetree@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; OpenBMC Maillist <openbmc@lists.ozlabs.org>; Ryan Chen <ryan_chen@aspeedtech.com>
Subject: Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
Adding Ryan.
On Mon, Apr 24, 2017 at 7:19 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Mon, 2017-04-24 at 11:56 -0700, Brendan Higgins wrote:
>> > > +struct aspeed_i2c_bus {
>> > > + struct i2c_adapter adap;
>> > > + struct device *dev;
>> > > + void __iomem *base;
>> > > + /* Synchronizes I/O mem access to base. */
>> > > + spinlock_t lock;
>> >
>> > I am not entirely convinced we need that lock. The i2c core will
>> > take a mutex protecting all operations on the bus. So we only need
>> > to synchronize between our "xfer" code and our interrupt handler.
>>
>> You are right if both having slave and master active at the same time
>> was not possible; however, it is.
>
> Right, I somewhat forgot about the slave case.
>
> ...
>
>> > Some of those error states probably also warrant a reset of the
>> > controller, I think aspeed does that in the SDK.
>>
>> For timeout and cmd_err, I do not see any argument against it; it
>> sounds like we are in a very messed up, very unknown state, so full
>> reset is probably the best last resort.
>
> Yup.
>
>> For SDA staying pulled down, I
>> think we can say with reasonable confidence that some device on our
>> bus is behaving very badly and I am not convinced that resetting the
>> controller is likely to do anything to help;
>
> Right. Hammering with STOPs and pray ...
I think sending recovery mode sends stops as a part of the recovery algorithm it executes.
>
>> that being said, I really
>> do not have any good ideas to address that. So maybe praying and
>> resetting the controller is *the most reasonable thing to do.* I
>> would like to know what you think we should do in that case.
>
> Well, there's a (small ?) chance that it's a controller bug asserting
> the line so ... but there's little we can do if not.
True.
>
>> While I was thinking about this I also realized that the SDA line
>> check after recovery happens in the else branch, but SCL line check
>> does not happen after we attempt to STOP if SCL is hung. If we decide
>> to make special note SDA being hung by a device that won't let go, we
>> might want to make a special note that SCL is hung by a device that
>> won't let go. Just a thought.
>
> Maybe. Or just "unrecoverable error"... hopefully these don't happen
> too often ... We had cases of a TPM misbehaving like that.
Yeah, definitely should print something out.
>
>> > > +out:
>>
>> ...
>> > What about I2C_M_NOSTART ?
>> >
>> > Not that I've ever seen it used... ;-)
>>
>> Right now I am not doing any of the protocol mangling options, but I
>> can add them in if you think it is important for initial support.
>
> No, not important, we can add that later if it ever becomes useful.
>
> ...
>
>> > In general, you always ACK all interrupts first. Then you handle
>> > the bits you have harvested.
>> >
>>
>> The documentation says to ACK the interrupt after handling in the RX
>> case:
>>
>> <<<
>> S/W needs to clear this status bit to allow next data receiving.
>> > > >
>>
>> I will double check with Ryan to make sure TX works the same way.
>>
>> > > + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
>> > > + (!bus->msgs && bus->master_state !=
>> > > ASPEED_I2C_MASTER_STOP)) {
>>
>> ...
>> >
>> > I would set master_state to "RECOVERY" (new state ?) and ensure
>> > those things are caught if they happen outside of a recovery.
>
> I replied privately ... as long as we ack before we start a new
> command we should be ok but we shouldn't ack after.
>
> Your latest patch still does that. It will do things like start a STOP
> command *then* ack the status bits. I'm pretty sure that's bogus.
>
> That way it's a lot simpler to simply move the
>
> writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
>
> To either right after the readl of the status reg at the beginning of
> aspeed_i2c_master_irq().
>
> I would be very surprised if that didn't work properly and wasn't much
> safer than what you are currently doing.
I think I tried your way and it worked. In anycase, Ryan will be able to clarify for us.
>
>> Let me know if you still think we need a "RECOVERY" state.
>
> The way you just switch to stop state and store the error for later
> should work I think.
>
>> >
>> > > + if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>
>> ...
>> >
>> > > + dev_dbg(bus->dev,
>> > > + "no slave present at %02x", msg-
>> > > >addr);
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> > > + bus->cmd_err = -EIO;
>> > > + do_stop(bus);
>> > > + goto out_no_complete;
>> > > + } else {
>> > > + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> > > + if (msg->flags & I2C_M_RD)
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_RX;
>> > > + else
>> > > + bus->master_state =
>> > > ASPEED_I2C_MASTER_TX_FIRST;
>> >
>> > What about the SMBUS_QUICK case ? (0-len transfer). Do we need to
>> > handle this here ? A quick look at the TX_FIRST case makes me think
>> > we are ok there but I'm not sure about the RX case.
>>
>> I did not think that there is an SMBUS_QUICK RX. Could you point me
>> to an example?
>
> Not so much an RX, it's more like you are sending a 1-bit data in the
> place of the Rd/Wr bit. So you have a read with a lenght of 0, I don't
> think in that case you should set ASPEED_I2CD_M_RX_CMD in
> __aspeed_i2c_do_start
Forget what I said, I was just not thinking about the fact that SMBus emulation causes the data bit to be encoded as the R/W flag. I see what you are saying; you are correct.
>
>> > I'm not sure the RX case is tight also. What completion does the HW
>> > give you for the address cycle ? Won't you get that before it has
>> > received the first character ? IE. You fall through to the read
>> > case of the state machine with the read potentially not complete
>> > yet no ?
>>
>> ...
>> > > + case ASPEED_I2C_MASTER_RX:
>> > > + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> > > + dev_err(bus->dev, "master failed to RX");
>> > > + goto out_complete;
>> > > + }
>> >
>> > See my comment above for a bog standard i2c_read. Aren't you
>> > getting the completion for the address before the read is even
>> > started ?
>>
>> In practice no, but it is probably best to be safe :-)
>
> Yup :)
>> >
>> > > + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> > > +
>> > > + recv_byte = aspeed_i2c_read(bus,
>> > > ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> > > + msg->buf[bus->buf_index++] = recv_byte;
>> > > +
>> > > + if (msg->flags & I2C_M_RECV_LEN &&
>> > > + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
>> > > + msg->len = recv_byte +
>> > > + ((msg->flags &
>> > > I2C_CLIENT_PEC) ? 2 : 1);
>>
>> ...
>> > > + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> > > + | ((clk_low <<
>> > > ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> > > + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> > > + | (base_clk &
>> > > ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> > > +}
>> >
>> > As I think I mentioned earlier, the AST2500 has a slightly
>> > different register layout which support larger values for high and
>> > low, thus allowing a finer granularity.
>>
>> I am developing against the 2500.
>
> Yes but we'd like the driver to work with both :-)
Right, I thought you were making an assertion about the 2500, if you are making an assertion about the 2400, I do not know and do not have one handy.
>
>> > BTW. In case you haven't, I would suggest you copy/paste the above
>> > in a userspace app and run it for all frequency divisors and see if
>> > your results match the aspeed table :)
>>
>> Good call.
>
> If you end up doing that, can you shoot it my way ? I can take care of
> making sure it's all good for the 2400.
Will do.
>
>> > > +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> > > + struct platform_device *pdev) {
>> > > + u32 clk_freq, divisor;
>> > > + struct clk *pclk;
>> > > + int ret;
>> > > +
>> > > + pclk = devm_clk_get(&pdev->dev, NULL);
>> > > + if (IS_ERR(pclk)) {
>> > > + dev_err(&pdev->dev, "clk_get failed\n");
>> > > + return PTR_ERR(pclk);
>> > > + }
>> > > + ret = of_property_read_u32(pdev->dev.of_node,
>> > > + "clock-frequency", &clk_freq);
>> >
>> > See my previous comment about calling that 'bus-frequency' rather
>> > than 'clock-frequency'.
>> >
>> > > + if (ret < 0) {
>> > > + dev_err(&pdev->dev,
>> > > + "Could not read clock-frequency
>> > > property\n");
>> > > + clk_freq = 100000;
>> > > + }
>> > > + divisor = clk_get_rate(pclk) / clk_freq;
>> > > + /* We just need the clock rate, we don't actually use the
>> > > clk object. */
>> > > + devm_clk_put(&pdev->dev, pclk);
>> > > +
>> > > + /* Set AC Timing */
>> > > + if (clk_freq / 1000 > 1000) {
>> > > + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> > > + ASPEED_I2C_FU
>> > > N_CTRL_REG) |
>> > > + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> > > + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> > > + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> > > + ASPEED_I2C_FUN_CTRL_REG);
>> > > +
>> > > + aspeed_i2c_write(bus, 0x3,
>> > > ASPEED_I2C_AC_TIMING_REG2);
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> >
>> > I already discussed by doubts about the above. I can try to scope
>> > it with the EVB if you don't get to it. For now I'd rather take the
>> > code out.
>> >
>> > We should ask aspeed from what frequency the "1T" stuff is useful.
>>
>> Will do, I will try to rope Ryan in on the next review; it will be
>> good for him to get used to working with upstream anyway.
>
> Yup. However, for the sake of getting something upstream (and in
> OpenBMC 4.10 kernel) asap, I would suggest just dropping support for
> those fast speeds for now, we can add them back later.
Alright, that's fine. Still, Ryan, could you provide some context on this?
>
>> >
>> > > + } else {
>> > > + aspeed_i2c_write(bus,
>> > > aspeed_i2c_get_clk_reg_val(divisor),
>> > > + ASPEED_I2C_AC_TIMING_REG1);
>> > > + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> > > + ASPEED_I2C_AC_TIMING_REG2);
>> > > + }
>>
>> ...
>> > > + spin_lock_init(&bus->lock);
>> > > + init_completion(&bus->cmd_complete);
>> > > + bus->adap.owner = THIS_MODULE;
>> > > + bus->adap.retries = 0;
>> > > + bus->adap.timeout = 5 * HZ;
>> > > + bus->adap.algo = &aspeed_i2c_algo;
>> > > + bus->adap.algo_data = bus;
>> > > + bus->adap.dev.parent = &pdev->dev;
>> > > + bus->adap.dev.of_node = pdev->dev.of_node;
>> > > + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed
>> > > i2c");
>> >
>> > Another trivial one, should we put some kind of bus number in that
>> > string ?
>>
>> Whoops, looks like I missed this one; I will get to it in the next
>> revision.
>
> Ok. I noticed you missed that in v7, so I assume you mean v8 :-)
Yep, I will get it in v8.
>
>> >
>> > > + bus->dev = &pdev->dev;
>> > > +
>> > > + /* reset device: disable master & slave functions */
>> > > + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
>>
>> ...
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree"
>> in
>> the body of a message to majordomo@vger.kernel.org More majordomo
>> info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 0/4] of: remove *phandle properties from expanded device tree
From: Frank Rowand @ 2017-04-25 8:51 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493110049-30165-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 04/25/17 01:47, frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree. The phandle will still be in the struct device_node
> phandle field.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *. As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
>
> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Patch 1 is the phandle related changes.
>
> Patches 2 - 4 are minor fixups for issues that became visible
> while implementing patch 1.
>
> Changes from v1:
^^^^^^^^
Changes from v2
> - patch 1: Remove check in __of_add_phandle_sysfs() that would not
> add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
>
> Changes from v1:
> - Remove phandle properties in of_attach_node(), before attaching
> the node to the live tree.
> - Add the phandle sysfs entry for the node in of_attach_node().
> - When creating an overlay changeset, duplicate the node phandle in
> __of_node_dup().
>
> Frank Rowand (4):
> of: remove *phandle properties from expanded device tree
> of: make __of_attach_node() static
> of: be consistent in form of file mode
> of: detect invalid phandle in overlay
>
> drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
> drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
> drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
> drivers/of/of_private.h | 2 +-
> drivers/of/overlay.c | 8 ++++---
> drivers/of/resolver.c | 23 +-------------------
> include/linux/of.h | 1 +
> 7 files changed, 120 insertions(+), 60 deletions(-)
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox