* Re: [PATCH 3/6] pinctrl: sh-pfc: r8a77990: Add bias pinconf support
From: Geert Uytterhoeven @ 2018-05-14 20:13 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Laurent Pinchart, Geert Uytterhoeven, Linus Walleij, Rob Herring,
Mark Rutland, Linux-Renesas, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Takeshi Kihara
In-Reply-To: <1526008947-26667-4-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Hi Shimoda-san,
On Fri, May 11, 2018 at 5:22 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch implements control of pull-up and pull-down. On this SoC there
> is no simple mapping of GP pins to bias register bits, so we need a table.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Thanks for your patch!
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
> @@ -1227,10 +1248,55 @@ enum {
>
> PINMUX_IPSR_GPSR(IP15_31_28, USB30_OVC),
> PINMUX_IPSR_MSEL(IP15_31_28, USB0_OVC_A, SEL_USB_20_CH0_0),
> +
> +/*
> + * Static pins can not be muxed between different functions but
> + * still needs a mark entry in the pinmux list. Add each static
need mark entries
> + * pin to the list without an associated function. The sh-pfc
> + * core will do the right thing and skip trying to mux then pin
mux the pin
> + * while still applying configuration to it
period
I have just sent a patch to fix the other copies, in the hope these grammar
atrocities will stop spreading ;-)
> @@ -1708,8 +1774,263 @@ enum {
> { },
> };
>
> +static const struct pinmux_bias_reg pinmux_bias_regs[] = {
The register definitions look OK to me.
I'll review the actual pin mappings later.
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 v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Dan Murphy @ 2018-05-14 20:13 UTC (permalink / raw)
To: Pavel Machek
Cc: Jacek Anaszewski, robh+dt, mark.rutland, afd, devicetree,
linux-kernel, linux-leds
In-Reply-To: <20180514200503.GB3276@amd>
Pavel
On 05/14/2018 03:05 PM, Pavel Machek wrote:
> Hi!
>
>>> OK.
>>
>> OK I looked at the max776973 driver and well if the flash-max-microamp and
>> flash-max-timeout-us nodes are missing it sets a default value for each if the
>> node is not present.
>>
>> So should we remove this code from the Max77693 driver too and fail probe as being asked
>> in this driver?
>
> Well, modifying driver without access to the hardware is tricky
> :-(. If it does something stupid (like using other than minimum values
> for the flash-max-microamp/flash-max-timeout-us), it needs to be
> fixed.
Well we should be able to test the probe/parse dt node and reject the probe if the node is not
present. That can be done without HW the setup is done pretty early in the probe without even attempting to communicate
with the hardware.
Dan
>
> And maybe comment "don't copy this, its wrong" would be in order...
> > Best regards,
> Pavel
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Dan Murphy @ 2018-05-14 20:09 UTC (permalink / raw)
To: Jacek Anaszewski, robh+dt, mark.rutland, pavel, afd
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <f7a98b66-977a-887c-d31a-ed908c9d1409@gmail.com>
On 05/14/2018 03:05 PM, Jacek Anaszewski wrote:
> Hi Dan,
>
> On 05/14/2018 09:40 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 05/11/2018 06:56 AM, Dan Murphy wrote:
>> <snip>
>>
>>>>> + }
>>>>> +
>>>>> + if (led->strobe_node) {
>>>>> + ret = of_property_read_string(led->strobe_node, "label", &name);
>>>>> + if (!ret)
>>>>> + snprintf(led->strobe, sizeof(led->strobe),
>>>>> + "%s:%s", led->strobe_node->name, name);
>>>>> + else
>>>>> + snprintf(led->strobe, sizeof(led->strobe),
>>>>> + "%s::strobe", led->strobe_node->name);
>>>>> +
>>>>> + ret = of_property_read_u32(led->strobe_node,
>>>>> + "flash-max-microamp",
>>>>> + &led->strobe_current_max);
>>>>> + if (ret < 0) {
>>>>> + led->strobe_current_max = LM3601X_MIN_STROBE_I_MA;
>>>>> + dev_warn(&led->client->dev,
>>>>> + "flash-max-microamp DT property missing\n");
>>>>> + }
>>>>> +
>>>>> + ret = of_property_read_u32(led->strobe_node,
>>>>> + "flash-max-timeout-us",
>>>>> + &led->max_strobe_timeout);
>>>>> + if (ret < 0) {
>>>>> + led->max_strobe_timeout = strobe_timeouts[0].reg_val;
>>>>> + dev_warn(&led->client->dev,
>>>>> + "flash-max-timeout-us DT property missing\n");
>>>>> + }
>>>>
>>>> Common LED bindings state that flash-max-microamp and
>>>> flash-max-timeout-us properties are mandatory.
>>>
>>> OK.
>>
>> OK I looked at the max776973 driver and well if the flash-max-microamp and
>> flash-max-timeout-us nodes are missing it sets a default value for each if the
>> node is not present.
>
> Ah, yes, this driver was being introduced as the first LED flash class driver and we were being iteratively adjusting LED common bindings
> according to the new findings, so some details could have been left
> out of sync.
>
>> So should we remove this code from the Max77693 driver too and fail probe as being asked
>> in this driver?
>
> Yes, that would match what the bindings require.
Did you want me to remove it and submit? I don't have a board to verify but I can definitely test out the probe and parse dt functionality.
Don't need HW for that.
Dan
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v5 1/2] dt: bindings: lm3601x: Introduce the lm3601x driver
From: Dan Murphy @ 2018-05-14 20:07 UTC (permalink / raw)
To: Jacek Anaszewski, Pavel Machek
Cc: robh+dt, mark.rutland, afd, devicetree, linux-kernel, linux-leds
In-Reply-To: <2dd155f5-ce82-88b2-699e-897ef7d55fee@gmail.com>
Jacek
On 05/11/2018 03:27 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 05/11/2018 02:12 PM, Dan Murphy wrote:
>> Jacek
>>
>> Thanks for the review
>>
>> On 05/10/2018 03:17 PM, Jacek Anaszewski wrote:
>>> Hi Dan,
>>>
>>> On 05/10/2018 09:10 PM, Dan Murphy wrote:
>>>> On 05/10/2018 02:06 PM, Dan Murphy wrote:
>>>>> Pavel
>>>>>
>>>>> On 05/10/2018 01:54 PM, Pavel Machek wrote:
>>>>>> Hi!
>>>>>>
>>>>>>> Introduce the device tree bindings for the lm3601x
>>>>>>> family of LED torch, flash and IR drivers.
>>>>>>>
>>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>>
>>>>>> Better, thanks.
>>>>>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3601x.txt
>>>>>>> @@ -0,0 +1,50 @@
>>>>>>> +* Texas Instruments - lm3601x Single-LED Flash Driver
>>>>>>
>>>>>> Ok, so is it single-LED driver, or can it driver ir & white LEDs at
>>>>>> the same time?
>>>>>
>>>>> It is a single LED driver. It can drive a Torch white LED or IR LED indefinitely or if the driver
>>>>> is programmed to strobe mode the driver will drive the configured LED for the flash timeout specified.
>>>>>
>>>>> Basically a flash and a flash light. IR or White LED.
>>>>>
>>>>>
>>>>>>
>>>>>>> +Example:
>>>>>>> +led-controller@64 {
>>>>>>> + compatible = "ti,lm36010";
>>>>>>> + #address-cells = <1>;
>>>>>>> + #size-cells = <0>;
>>>>>>> + reg = <0x64>;
>>>>>>> +
>>>>>>> + led@0 {
>>>>>>> + reg = <0>;
>>>>>>> + label = "white:torch";
>>>>>>> + led-max-microamp = <10000>;
>>>>>>> + };
>>>>>>> +
>>>>>>> + led@1 {
>>>>>>> + reg = <1>;
>>>>>>> + label = "white:flash";
>>>>>>> + flash-max-microamp = <10000>;
>>>>>>> + flash-max-timeout-us = <800>;
>>>>>>> + };
>>>>>>
>>>>>> Is this realistic config? I'd expect flash to use more power than
>>>>>> torch, and would expect longer timeout than 0.8msec.
>>>>>
>>>>> Timeout in the spreadsheet is ms I will update this example and code
>>>>> Current in the spreadsheet is mA I will update this example and code
>>>>>
>>>>>>
>>>>>> Also.. if this is physically one white LED, it should not be
>>>>>> spread over reg = <0> and reg = <1>...
>>>>>
>>>>> If the torch LED and strobe LED are the same LED how do I expose them both to the user.
>>>>> It is up to consumer to configure the required interfaces they want to expose to the filesystem.
>>>
>>> LED flash class interface is prepared for it.
>>> led_clasdev_flash_register() internally calls led_classdev_register(),
>>> so there is brightness file available for torch related operations.
>>
>> Yes the flash class may handle this and expose the brightness node but the HW has two different registers to set the
>> corresponding brightness so one set brightness node does not work.
>> There is no way to differentiate between the strobe brightness (register 4) and the torch brightness (register 3).
>
> Hmm? There is brightness file (with brightness_set{_blocking} op) from
> LED class and flash_brightness file (with flash_brightness_set op) from
> LED class flash.
>
> I've only now realized that you're not using flash_brightness_set op for
> the "strobe_node" in your driver. I assume that you overlooked it.
> Please don't introduce "strobe_brightness" naming convention - we
> already have "flash_brightness" for that.
>
>> When the enable register is written the driver will read the corresponding register and set
>> the current for the LED.
>>
>> This is why I separated out the strobe from the torch into 2 different LED nodes.
>>
>> I cannot seem to find the data sheet on line for the max77693 so I cannot verify that the device only has
>> a single brightness register for both torch and strobe.
>
> It wasn't openly available at least at the time when I had an access
> to it.
>
> But - please look at the functions max77693_set_torch_current() and
> max77693_set_flash_current(). The device has separate registers
> for torch and flash brightnesses.
I have looked at these functions and the parse dt function that dictates if there is 1 fled or 2 fled.
I am having trouble associating what these mean because the led-sources is being used to define how many LEDs.
In the common.txt it says
"- led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation."
I cannot find the max77693 dt documentation or even a dt file that defines what the led-sources could be defined as.
In addition, per the common.txt, the led sources should define the current outputs the LED is connected to.
Is one to assume that it is referring to the physical current pin connection or the devices internal current routing?
When I first referenced this driver as template driver it was misleading to say fled1 and fled2 as I took it to mean, and I still do, that the
driver supports 2 LED connections and not a single output pin with different internal current routing.
Without the data sheet or the dt documentation it makes understanding a little difficult.
I will fix it up how ever you would like it to be but I am thinking we need to fix up the max77693 as well so we can have 2 reference
drivers. It is very difficult to derive the driver without a public version of the data sheet.
Dan
>
>>>>> Maybe they don't want the strobe feature and just want LED on/off and switch between IR and White.
>>>>>
>>>>> The IR is driven via a different output pin.
>>>>
>>>> Correction. There is only one LED drive pin. The mode selected determines how the device will drive the
>>>> LED. So you may have one device to drive a Torch and another device to drive the LED.
>>>
>>> But only one type of LED can be soldered on the board at a time, right?
>>> If yes, then this is clearly a DT related configuration, and only
>>> one child DT node should be defined for given board.
>>
>> But see above. There is not a clean way to expose the torch/ir and strobe separately.
>>
>> Dan
>>
>>
>>>
>>>> Strobe is available in either case but not always required if strobe is not desired by the customer hence
>>>> why I have a separate DT node for it.
>>>>
>>>> Dan
>>>>
>>>>>
>>>>> Dan
>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Pavel
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
--
------------------
Dan Murphy
^ permalink raw reply
* Re: [PATCH v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Jacek Anaszewski @ 2018-05-14 20:05 UTC (permalink / raw)
To: Dan Murphy, robh+dt, mark.rutland, pavel, afd
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <86c2bc2f-e622-9447-e4bb-b4ee37e2d44a@ti.com>
Hi Dan,
On 05/14/2018 09:40 PM, Dan Murphy wrote:
> Jacek
>
> On 05/11/2018 06:56 AM, Dan Murphy wrote:
> <snip>
>
>>>> + }
>>>> +
>>>> + if (led->strobe_node) {
>>>> + ret = of_property_read_string(led->strobe_node, "label", &name);
>>>> + if (!ret)
>>>> + snprintf(led->strobe, sizeof(led->strobe),
>>>> + "%s:%s", led->strobe_node->name, name);
>>>> + else
>>>> + snprintf(led->strobe, sizeof(led->strobe),
>>>> + "%s::strobe", led->strobe_node->name);
>>>> +
>>>> + ret = of_property_read_u32(led->strobe_node,
>>>> + "flash-max-microamp",
>>>> + &led->strobe_current_max);
>>>> + if (ret < 0) {
>>>> + led->strobe_current_max = LM3601X_MIN_STROBE_I_MA;
>>>> + dev_warn(&led->client->dev,
>>>> + "flash-max-microamp DT property missing\n");
>>>> + }
>>>> +
>>>> + ret = of_property_read_u32(led->strobe_node,
>>>> + "flash-max-timeout-us",
>>>> + &led->max_strobe_timeout);
>>>> + if (ret < 0) {
>>>> + led->max_strobe_timeout = strobe_timeouts[0].reg_val;
>>>> + dev_warn(&led->client->dev,
>>>> + "flash-max-timeout-us DT property missing\n");
>>>> + }
>>>
>>> Common LED bindings state that flash-max-microamp and
>>> flash-max-timeout-us properties are mandatory.
>>
>> OK.
>
> OK I looked at the max776973 driver and well if the flash-max-microamp and
> flash-max-timeout-us nodes are missing it sets a default value for each if the
> node is not present.
Ah, yes, this driver was being introduced as the first LED flash class
driver and we were being iteratively adjusting LED common bindings
according to the new findings, so some details could have been left
out of sync.
> So should we remove this code from the Max77693 driver too and fail probe as being asked
> in this driver?
Yes, that would match what the bindings require.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Pavel Machek @ 2018-05-14 20:05 UTC (permalink / raw)
To: Dan Murphy
Cc: Jacek Anaszewski, robh+dt, mark.rutland, afd, devicetree,
linux-kernel, linux-leds
In-Reply-To: <86c2bc2f-e622-9447-e4bb-b4ee37e2d44a@ti.com>
[-- Attachment #1: Type: text/plain, Size: 775 bytes --]
Hi!
> > OK.
>
> OK I looked at the max776973 driver and well if the flash-max-microamp and
> flash-max-timeout-us nodes are missing it sets a default value for each if the
> node is not present.
>
> So should we remove this code from the Max77693 driver too and fail probe as being asked
> in this driver?
Well, modifying driver without access to the hardware is tricky
:-(. If it does something stupid (like using other than minimum values
for the flash-max-microamp/flash-max-timeout-us), it needs to be
fixed.
And maybe comment "don't copy this, its wrong" would be in order...
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Andy Shevchenko @ 2018-05-14 19:49 UTC (permalink / raw)
To: Dan Murphy
Cc: Jacek Anaszewski, Rob Herring, Mark Rutland, Pavel Machek,
Andrew F. Davis, devicetree, Linux Kernel Mailing List,
Linux LED Subsystem
In-Reply-To: <86c2bc2f-e622-9447-e4bb-b4ee37e2d44a@ti.com>
On Mon, May 14, 2018 at 10:40 PM, Dan Murphy <dmurphy@ti.com> wrote:
> On 05/11/2018 06:56 AM, Dan Murphy wrote:
>>>> + ret = of_property_read_string(led->strobe_node, "label", &name);
>>>> + ret = of_property_read_u32(led->strobe_node,
>>>> + ret = of_property_read_u32(led->strobe_node,
>>> Common LED bindings state that flash-max-microamp and
>>> flash-max-timeout-us properties are mandatory.
>>
>> OK.
>
> OK I looked at the max776973 driver and well if the flash-max-microamp and
> flash-max-timeout-us nodes are missing it sets a default value for each if the
> node is not present.
>
> So should we remove this code from the Max77693 driver too and fail probe as being asked
> in this driver?
I would also add that using device_property_*() API is much better
then using OF specific one. It will help IoT / DIY entusiasts use them
on non-DT platforms.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Dan Murphy @ 2018-05-14 19:40 UTC (permalink / raw)
To: Jacek Anaszewski, robh+dt, mark.rutland, pavel, afd
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <17630aca-1225-df61-7a5d-a921ecd9c78c@ti.com>
Jacek
On 05/11/2018 06:56 AM, Dan Murphy wrote:
<snip>
>>> + }
>>> +
>>> + if (led->strobe_node) {
>>> + ret = of_property_read_string(led->strobe_node, "label", &name);
>>> + if (!ret)
>>> + snprintf(led->strobe, sizeof(led->strobe),
>>> + "%s:%s", led->strobe_node->name, name);
>>> + else
>>> + snprintf(led->strobe, sizeof(led->strobe),
>>> + "%s::strobe", led->strobe_node->name);
>>> +
>>> + ret = of_property_read_u32(led->strobe_node,
>>> + "flash-max-microamp",
>>> + &led->strobe_current_max);
>>> + if (ret < 0) {
>>> + led->strobe_current_max = LM3601X_MIN_STROBE_I_MA;
>>> + dev_warn(&led->client->dev,
>>> + "flash-max-microamp DT property missing\n");
>>> + }
>>> +
>>> + ret = of_property_read_u32(led->strobe_node,
>>> + "flash-max-timeout-us",
>>> + &led->max_strobe_timeout);
>>> + if (ret < 0) {
>>> + led->max_strobe_timeout = strobe_timeouts[0].reg_val;
>>> + dev_warn(&led->client->dev,
>>> + "flash-max-timeout-us DT property missing\n");
>>> + }
>>
>> Common LED bindings state that flash-max-microamp and
>> flash-max-timeout-us properties are mandatory.
>
> OK.
OK I looked at the max776973 driver and well if the flash-max-microamp and
flash-max-timeout-us nodes are missing it sets a default value for each if the
node is not present.
So should we remove this code from the Max77693 driver too and fail probe as being asked
in this driver?
Dan
>
>>
>>> +
>>> + lm3601x_init_flash_timeout(led);
<snip>
>>>
>>
>
>
--
------------------
Dan Murphy
^ permalink raw reply
* RE: [PATCH v6 09/11] firmware: xilinx: Add debugfs for clock control APIs
From: Jolly Shah @ 2018-05-14 19:18 UTC (permalink / raw)
To: Sudeep Holla, ard.biesheuvel@linaro.org, mingo@kernel.org,
gregkh@linuxfoundation.org, matt@codeblueprint.co.uk,
hkallweit1@gmail.com, keescook@chromium.org,
dmitry.torokhov@gmail.com, mturquette@baylibre.com,
sboyd@codeaurora.org, michal.simek@xilinx.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux-clk@vger.kernel.org
Cc: Rajan Vaja, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <5bcc47f7-3352-f0e6-29c5-78d5be96c1d1@arm.com>
Hi Sudeep,
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Thursday, May 10, 2018 7:31 AM
> To: Jolly Shah <JOLLYS@xilinx.com>; ard.biesheuvel@linaro.org;
> mingo@kernel.org; gregkh@linuxfoundation.org; matt@codeblueprint.co.uk;
> hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; mturquette@baylibre.com;
> sboyd@codeaurora.org; michal.simek@xilinx.com; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-clk@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; Rajan Vaja <RAJANV@xilinx.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v6 09/11] firmware: xilinx: Add debugfs for clock control
> APIs
>
>
>
> On 10/04/18 20:38, Jolly Shah wrote:
> > From: Rajan Vaja <rajanv@xilinx.com>
> >
> > Add debugfs file to control clocks using firmware APIs through debugfs
> > interface.
> >
> > Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
> > drivers/firmware/xilinx/zynqmp-debug.c | 48
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp-debug.c
> > b/drivers/firmware/xilinx/zynqmp-debug.c
> > index 1cb69f7..837fcd1 100644
> > --- a/drivers/firmware/xilinx/zynqmp-debug.c
> > +++ b/drivers/firmware/xilinx/zynqmp-debug.c
> > @@ -34,6 +34,15 @@ static struct pm_api_info pm_api_list[] = {
> > PM_API(PM_GET_API_VERSION),
> > PM_API(PM_IOCTL),
> > PM_API(PM_QUERY_DATA),
> > + PM_API(PM_CLOCK_ENABLE),
> > + PM_API(PM_CLOCK_DISABLE),
> > + PM_API(PM_CLOCK_GETSTATE),
> > + PM_API(PM_CLOCK_SETDIVIDER),
> > + PM_API(PM_CLOCK_GETDIVIDER),
> > + PM_API(PM_CLOCK_SETRATE),
> > + PM_API(PM_CLOCK_GETRATE),
> > + PM_API(PM_CLOCK_SETPARENT),
> > + PM_API(PM_CLOCK_GETPARENT),
> > };
> >
> > /**
> > @@ -87,6 +96,7 @@ static int process_api_request(u32 pm_id, u64
> *pm_api_arg, u32 *pm_api_ret)
> > const struct zynqmp_eemi_ops *eemi_ops =
> zynqmp_pm_get_eemi_ops();
> > u32 pm_api_version;
> > int ret;
> > + u64 rate;
> >
> > if (!eemi_ops)
> > return -ENXIO;
> > @@ -132,6 +142,44 @@ static int process_api_request(u32 pm_id, u64
> *pm_api_arg, u32 *pm_api_ret)
> > pm_api_ret[2], pm_api_ret[3]);
> > break;
> > }
> > + case PM_CLOCK_ENABLE:
> > + ret = eemi_ops->clock_enable(pm_api_arg[0]);
> > + break;
>
> As I mentioned in earlier patch, I don't see the need for this debugfs interface.
> Clock lay has read-only interface in debugfs already. Also if you want to debug
> clocks, you can do so using the driver which uses these clocks. Do you really
> want to manage clocks in user-space ?
> The whole idea of EEMI kind of interface is to abstract and hide the fine details
> even from non-trusted rich OS like Linux kernel, but by providing this you are
> doing exactly opposite.
>
> --
> Regards,
> Sudeep
No we don't want to manage clocks in user-space. This debugfs is meant for developer who wants to debug APIs behavior in case something doesn't work as expected. Debugfs should be off by default in production images.
Thanks,
Jolly Shah
^ permalink raw reply
* RE: [PATCH v6 04/11] firmware: xilinx: Add query data API
From: Jolly Shah @ 2018-05-14 19:16 UTC (permalink / raw)
To: Sudeep Holla, ard.biesheuvel@linaro.org, mingo@kernel.org,
gregkh@linuxfoundation.org, matt@codeblueprint.co.uk,
hkallweit1@gmail.com, keescook@chromium.org,
dmitry.torokhov@gmail.com, mturquette@baylibre.com,
sboyd@codeaurora.org, michal.simek@xilinx.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux-clk@vger.kernel.org
Cc: Rajan Vaja, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <85f8f8c5-a177-dad8-e095-a9dd1c513e8d@arm.com>
HI Sudeep,
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Thursday, May 10, 2018 7:12 AM
> To: Jolly Shah <JOLLYS@xilinx.com>; ard.biesheuvel@linaro.org;
> mingo@kernel.org; gregkh@linuxfoundation.org; matt@codeblueprint.co.uk;
> hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; mturquette@baylibre.com;
> sboyd@codeaurora.org; michal.simek@xilinx.com; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-clk@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; Rajan Vaja <RAJANV@xilinx.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v6 04/11] firmware: xilinx: Add query data API
>
>
>
> On 10/04/18 20:38, Jolly Shah wrote:
> > From: Rajan Vaja <rajanv@xilinx.com>
> >
> > Add ZynqMP firmware query data API to query platform specific
> > information(clocks, pins) from firmware.
> >
> > Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
> > drivers/firmware/xilinx/zynqmp.c | 14 ++++++++++++++
> > include/linux/firmware/xlnx-zynqmp.h | 20 ++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp.c
> > b/drivers/firmware/xilinx/zynqmp.c
> > index 44b43fa..ef09c44 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -258,9 +258,23 @@ static int zynqmp_pm_ioctl(u32 node_id, u32
> ioctl_id, u32 arg1, u32 arg2,
> > arg1, arg2, out);
> > }
> >
> > +/**
> > + * zynqmp_pm_query_data() - Get query data from firmware
> > + * @qdata: Variable to the zynqmp_pm_query_data structure
> > + * @out: Returned output value
> > + *
> > + * Return: Returns status, either success or error+reason */ static
> > +int zynqmp_pm_query_data(struct zynqmp_pm_query_data qdata, u32 *out)
> > +{
> > + return zynqmp_pm_invoke_fn(PM_QUERY_DATA, qdata.qid,
> qdata.arg1,
> > + qdata.arg2, qdata.arg3, out);
> > +}
> > +
> > static const struct zynqmp_eemi_ops eemi_ops = {
> > .get_api_version = zynqmp_pm_get_api_version,
> > .ioctl = zynqmp_pm_ioctl,
> > + .query_data = zynqmp_pm_query_data,
>
> Can you give more insight into this ? How will be this used ?
> How this aligns with data we get from DT ? I am just trying to understand how is
> this information split between this API and DT for example.
>
> --
> Regards,
> Sudeep
This API is used to get clock information from firmware and register clocks accordingly in driver. In our case, firmware maintains database of all clocks available on chip. DT will provide information for off chip reference clocks only.
This is to avoid duplication of clocks data in DT and firmware both as firmware anyways need clock data to manage them.
Thanks,
Jolly Shah
^ permalink raw reply
* RE: [PATCH v6 03/11] firmware: xilinx: Add zynqmp IOCTL API for device control
From: Jolly Shah @ 2018-05-14 19:11 UTC (permalink / raw)
To: Sudeep Holla, ard.biesheuvel@linaro.org, mingo@kernel.org,
gregkh@linuxfoundation.org, matt@codeblueprint.co.uk,
hkallweit1@gmail.com, keescook@chromium.org,
dmitry.torokhov@gmail.com, mturquette@baylibre.com,
sboyd@codeaurora.org, michal.simek@xilinx.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux-clk@vger.kernel.org
Cc: Rajan Vaja, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <c351b9a3-5f26-7202-7171-0dcc7adef7c0@arm.com>
Hi Sudeep,
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Thursday, May 10, 2018 7:09 AM
> To: Jolly Shah <JOLLYS@xilinx.com>; ard.biesheuvel@linaro.org;
> mingo@kernel.org; gregkh@linuxfoundation.org; matt@codeblueprint.co.uk;
> hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; mturquette@baylibre.com;
> sboyd@codeaurora.org; michal.simek@xilinx.com; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-clk@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; Rajan Vaja <RAJANV@xilinx.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v6 03/11] firmware: xilinx: Add zynqmp IOCTL API for device
> control
>
>
>
> On 10/04/18 20:38, Jolly Shah wrote:
> > From: Rajan Vaja <rajanv@xilinx.com>
> >
> > Add ZynqMP firmware IOCTL API to control and configure devices like
> > PLLs, SD, Gem, etc.
> >
> > Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
> > drivers/firmware/xilinx/zynqmp.c | 20 ++++++++++++++++++++
> > include/linux/firmware/xlnx-zynqmp.h | 2 ++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/firmware/xilinx/zynqmp.c
> > b/drivers/firmware/xilinx/zynqmp.c
> > index 490561a..44b43fa 100644
> > --- a/drivers/firmware/xilinx/zynqmp.c
> > +++ b/drivers/firmware/xilinx/zynqmp.c
> > @@ -239,8 +239,28 @@ static int get_set_conduit_method(struct
> device_node *np)
> > return 0;
> > }
> >
> > +/**
> > + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs
> > + * @node_id: Node ID of the device
> > + * @ioctl_id: ID of the requested IOCTL
> > + * @arg1: Argument 1 to requested IOCTL call
> > + * @arg2: Argument 2 to requested IOCTL call
> > + * @out: Returned output value
> > + *
> > + * This function calls IOCTL to firmware for device control and configuration.
> > + *
> > + * Return: Returns status, either success or error+reason */ static
> > +int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2,
> > + u32 *out)
> > +{
> > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id,
> > + arg1, arg2, out);
> > +}
> > +
> > static const struct zynqmp_eemi_ops eemi_ops = {
> > .get_api_version = zynqmp_pm_get_api_version,
> > + .ioctl = zynqmp_pm_ioctl,
> > };
> >
> > /**
> > diff --git a/include/linux/firmware/xlnx-zynqmp.h
> > b/include/linux/firmware/xlnx-zynqmp.h
> > index cb63bed..2eec6e7 100644
> > --- a/include/linux/firmware/xlnx-zynqmp.h
> > +++ b/include/linux/firmware/xlnx-zynqmp.h
> > @@ -34,6 +34,7 @@
> >
> > enum pm_api_id {
> > PM_GET_API_VERSION = 1,
> > + PM_IOCTL = 34,
>
> I am not for this API. IIUC there are more fined grained well defined APIs(if I am
> not wrong from enum 2 upto 33). This is open ended API which allows user to do
> whatever setting it needs. And that defeats the purpose of other APIs.
>
> I will look through the series for the usage of this to understand this better, but I
> am guessing how it can be (ab)used.
>
> --
> Regards,
> Sudeep
There are well defined APIs for general clock controls. Ioctl is for some specific operations which may not apply to all platforms.
For clock driver, ioctl is used to get/set Pll fraction/integer mode and data.
Thanks,
Jolly Shah
^ permalink raw reply
* [PATCH v3 RESEND] display: panel: Add AUO g070vvn01 display support (800x480)
From: Lukasz Majewski @ 2018-05-14 19:08 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Thierry Reding, David Airlie
Cc: dri-devel, devicetree, Rob Herring, linux-kernel, Lukasz Majewski
In-Reply-To: <20180410102927.17787-1-lukma@denx.de>
This commit adds support for AUO's 7.0" display.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes for v3:
- Remove not used 'bus-format-override = "rgb565";' property
Changes for v2:
- Add *.txt suffix to the auo,g070wn01 file
---
.../bindings/display/panel/auo,g070vvn01.txt | 29 ++++++++++++++++++++
drivers/gpu/drm/panel/panel-simple.c | 31 ++++++++++++++++++++++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
diff --git a/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
new file mode 100644
index 000000000000..49e4105378f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
@@ -0,0 +1,29 @@
+AU Optronics Corporation 7.0" FHD (800 x 480) TFT LCD panel
+
+Required properties:
+- compatible: should be "auo,g070vvn01"
+- backlight: phandle of the backlight device attached to the panel
+- power-supply: single regulator to provide the supply voltage
+
+Required nodes:
+- port: Parallel port mapping to connect this display
+
+This panel needs single power supply voltage. Its backlight is conntrolled
+via PWM signal.
+
+Example:
+--------
+
+Example device-tree definition when connected to iMX6Q based board
+
+ lcd_panel: lcd-panel {
+ compatible = "auo,g070vvn01";
+ backlight = <&backlight_lcd>;
+ power-supply = <®_display>;
+
+ port {
+ lcd_panel_in: endpoint {
+ remote-endpoint = <&lcd_display_out>;
+ };
+ };
+ };
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index cbf1ab404ee7..d9984bdb5bb5 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -581,6 +581,34 @@ static const struct panel_desc auo_b133htn01 = {
},
};
+static const struct display_timing auo_g070vvn01_timings = {
+ .pixelclock = { 33300000, 34209000, 45000000 },
+ .hactive = { 800, 800, 800 },
+ .hfront_porch = { 20, 40, 200 },
+ .hback_porch = { 87, 40, 1 },
+ .hsync_len = { 1, 48, 87 },
+ .vactive = { 480, 480, 480 },
+ .vfront_porch = { 5, 13, 200 },
+ .vback_porch = { 31, 31, 29 },
+ .vsync_len = { 1, 1, 3 },
+};
+
+static const struct panel_desc auo_g070vvn01 = {
+ .timings = &auo_g070vvn01_timings,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 152,
+ .height = 91,
+ },
+ .delay = {
+ .prepare = 200,
+ .enable = 50,
+ .disable = 50,
+ .unprepare = 1000,
+ },
+};
+
static const struct drm_display_mode auo_g104sn02_mode = {
.clock = 40000,
.hdisplay = 800,
@@ -2095,6 +2123,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b133xtn01",
.data = &auo_b133xtn01,
}, {
+ .compatible = "auo,g070vvn01",
+ .data = &auo_g070vvn01,
+ }, {
.compatible = "auo,g104sn02",
.data = &auo_g104sn02,
}, {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH RESEND net-next v2 0/8] net: stmmac: dwmac-sun8i: Support R40
From: David Miller @ 2018-05-14 19:07 UTC (permalink / raw)
To: wens
Cc: peppe.cavallaro, linux-arm-kernel, devicetree, netdev,
clabbe.montjoie, icenowy, maxime.ripard, robh+dt
In-Reply-To: <20180513191425.9801-1-wens@csie.org>
From: Chen-Yu Tsai <wens@csie.org>
Date: Mon, 14 May 2018 03:14:17 +0800
> This is a resend of the patches for net-next split out from my R40
> Ethernet support v2 series, as requested by David Miller. The arm-soc
> bits will follow, once I rework the A64 system controller compatible.
Series applied, but please continue to follow-up with Andrew Lunn about
how best to handle these RX and TX delays.
Thank you.
^ permalink raw reply
* RE: [PATCH v6 02/11] firmware: xilinx: Add Zynqmp firmware driver
From: Jolly Shah @ 2018-05-14 19:06 UTC (permalink / raw)
To: Sudeep Holla, ard.biesheuvel@linaro.org, mingo@kernel.org,
gregkh@linuxfoundation.org, matt@codeblueprint.co.uk,
hkallweit1@gmail.com, keescook@chromium.org,
dmitry.torokhov@gmail.com, mturquette@baylibre.com,
sboyd@codeaurora.org, michal.simek@xilinx.com, robh+dt@kernel.org,
mark.rutland@arm.com, linux-clk@vger.kernel.org
Cc: Rajan Vaja, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <9cbfb3cd-daa3-f145-c6f2-f15f61d1bea0@arm.com>
Hi Sudeep,
> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: Thursday, May 10, 2018 7:05 AM
> To: Jolly Shah <JOLLYS@xilinx.com>; ard.biesheuvel@linaro.org;
> mingo@kernel.org; gregkh@linuxfoundation.org; matt@codeblueprint.co.uk;
> hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; mturquette@baylibre.com;
> sboyd@codeaurora.org; michal.simek@xilinx.com; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-clk@vger.kernel.org
> Cc: Sudeep Holla <sudeep.holla@arm.com>; Rajan Vaja <RAJANV@xilinx.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v6 02/11] firmware: xilinx: Add Zynqmp firmware driver
>
>
>
> On 10/04/18 20:38, Jolly Shah wrote:
> > From: Rajan Vaja <rajanv@xilinx.com>
> >
> > This patch is adding communication layer with firmware.
> > Firmware driver provides an interface to firmware APIs.
> > Interface APIs can be used by any driver to communicate to
> > PMUFW(Platform Management Unit). All requests go through ATF.
> >
> > Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
>
> [...]
>
> > +
> > +/**
> > + * get_set_conduit_method() - Choose SMC or HVC based communication
> > + * @np: Pointer to the device_node structure
> > + *
> > + * Use SMC or HVC-based functions to communicate with EL2/EL3.
> > + *
> > + * Return: Returns 0 on success or error code */ static int
> > +get_set_conduit_method(struct device_node *np) {
> > + const char *method;
> > +
> > + if (of_property_read_string(np, "method", &method)) {
> > + pr_warn("%s missing \"method\" property\n", __func__);
> > + return -ENXIO;
> > + }
> > +
> > + if (!strcmp("hvc", method)) {
> > + do_fw_call = do_fw_call_hvc;
> > + } else if (!strcmp("smc", method)) {
> > + do_fw_call = do_fw_call_smc;
> > + } else {
> > + pr_warn("%s Invalid \"method\" property: %s\n",
> > + __func__, method);
> > + return -EINVAL;
> > + }
> > +
>
> Mark R did some cleanup around SMCCC conduits[1]. It makes sense to base this
> on top that. But if you manage to push this for v4.18, then you may need to wait
> for that to be merged and clean it up after v4.18
>
> --
> Regards,
> Sudeep
Mark R did change for SMCCC enums and we are not using any SMCCC enums so we don't have any dependency on that.
Thanks,
Jolly Shah
>
> [1] https://www.spinics.net/lists/arm-kernel/msg650305.html
^ permalink raw reply
* Re: [PATCH v2 2/2] drm/bridge: sii902x: add optional power supplies
From: Philippe CORNU @ 2018-05-14 18:58 UTC (permalink / raw)
To: Andrzej Hajda, Archit Taneja, Laurent Pinchart, David Airlie,
Rob Herring, Mark Rutland, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Yannick FERTRE, Alexandre TORGUE
In-Reply-To: <69a9da79-63df-6d0a-52e3-729ec8937484@samsung.com>
Hi Andrzej,
On 05/14/2018 12:33 PM, Andrzej Hajda wrote:
> On 14.05.2018 11:38, Philippe CORNU wrote:
>> Hi Laurent, Archit, Andrzej & Yannick,
>>
>> Do you have any comments on this v2 driver part?
>> (more details regarding v1/v2 differences in the cover letter
>> https://www.spinics.net/lists/dri-devel/msg174137.html)
>>
>> Thank you,
>> Philippe :-)
>>
>>
>> On 04/25/2018 09:53 AM, Philippe Cornu wrote:
>>> Add the optional power supplies using the description found in
>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)".
>>>
>>> The sii902x input IOs are not "io safe" so it is important to
>>> enable/disable voltage regulators during probe/remove phases to
>>> avoid damages.
>
> What exactly does it mean? Ie I understand that the chip has some
> limitations, but why enabling/disabling regulators in probe/remove
> should solve it?
thank you for your comment.
And sorry for the "bad" explanation in the 2nd paragraph about the fact
that inputs are not "io safe". I added this 2nd paragraph in v2
following a good comment from Laurent on adding the management of the
regulators outside the probe/remove for a better power consumption
management (enable/disable regulators only when the ic is used for
displaying something for instance...). But after a deeper analysis, I
realized that the only way to improve the power consumption is to
implement & test the sii902x various sleep modes, that is out-of-scope
of this small patch and also out-of-scope of my test board I use on
which the sii902x bridge ic power consumption is very low compare to the
rest of the system...
I will remove this "explanation" in v3 as it creates confusion.
Many thanks,
Philippe :-)
>
> Regards
> Andrzej
>
>>>
>>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
>>> ---
>>> drivers/gpu/drm/bridge/sii902x.c | 38 ++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>>> index 60373d7eb220..c367d7b91ade 100644
>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>>> @@ -24,6 +24,7 @@
>>> #include <linux/i2c.h>
>>> #include <linux/module.h>
>>> #include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_atomic_helper.h>
>>> @@ -86,6 +87,7 @@ struct sii902x {
>>> struct drm_bridge bridge;
>>> struct drm_connector connector;
>>> struct gpio_desc *reset_gpio;
>>> + struct regulator_bulk_data supplies[2];
>>> };
>>>
>>> static inline struct sii902x *bridge_to_sii902x(struct drm_bridge *bridge)
>>> @@ -392,23 +394,42 @@ static int sii902x_probe(struct i2c_client *client,
>>> return PTR_ERR(sii902x->reset_gpio);
>>> }
>>>
>>> + sii902x->supplies[0].supply = "iovcc";
>>> + sii902x->supplies[1].supply = "vcc12";
>>> + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
>>> + sii902x->supplies);
>>> + if (ret) {
>>> + dev_err(dev, "Failed to get power supplies: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
>>> + sii902x->supplies);
>>> + if (ret) {
>>> + dev_err(dev, "Failed to enable power supplies: %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + usleep_range(10000, 20000);
>>> +
>>> sii902x_reset(sii902x);
>>>
>>> ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
>>> if (ret)
>>> - return ret;
>>> + goto err_disable_regulator;
>>>
>>> ret = regmap_bulk_read(sii902x->regmap, SII902X_REG_CHIPID(0),
>>> &chipid, 4);
>>> if (ret) {
>>> dev_err(dev, "regmap_read failed %d\n", ret);
>>> - return ret;
>>> + goto err_disable_regulator;
>>> }
>>>
>>> if (chipid[0] != 0xb0) {
>>> dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
>>> chipid[0]);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto err_disable_regulator;
>>> }
>>>
>>> /* Clear all pending interrupts */
>>> @@ -424,7 +445,7 @@ static int sii902x_probe(struct i2c_client *client,
>>> IRQF_ONESHOT, dev_name(dev),
>>> sii902x);
>>> if (ret)
>>> - return ret;
>>> + goto err_disable_regulator;
>>> }
>>>
>>> sii902x->bridge.funcs = &sii902x_bridge_funcs;
>>> @@ -434,6 +455,12 @@ static int sii902x_probe(struct i2c_client *client,
>>> i2c_set_clientdata(client, sii902x);
>>>
>>> return 0;
>>> +
>>> +err_disable_regulator:
>>> + regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
>>> + sii902x->supplies);
>>> +
>>> + return ret;
>>> }
>>>
>>> static int sii902x_remove(struct i2c_client *client)
>>> @@ -443,6 +470,9 @@ static int sii902x_remove(struct i2c_client *client)
>>>
>>> drm_bridge_remove(&sii902x->bridge);
>>>
>>> + regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
>>> + sii902x->supplies);
>>> +
>>> return 0;
>>> }
>>>
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH 2/2] ARM: dts: iimx51-eukrea-mbimxsd51-baseboard: Fix USB PHY duplicate unit-address
From: Fabio Estevam @ 2018-05-14 18:29 UTC (permalink / raw)
To: shawnguo; +Cc: Fabio Estevam, devicetree, robh+dt, linux-arm-kernel
In-Reply-To: <1526322576-5838-1-git-send-email-festevam@gmail.com>
From: Fabio Estevam <fabio.estevam@nxp.com>
Currently the following DTC warning is seen with W=1:
arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dtb: Warning (unique_unit_address): /usbphy/usbphy@0: duplicate unit-address (also used in node /usbphy/usbh1phy@0)
Fix it by moving the USB PHY node outside of simple-bus and drop the
unneeded unit-address.
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
.../arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
index b3d952f..29b0b7b 100644
--- a/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
+++ b/arch/arm/boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts
@@ -83,19 +83,12 @@
fsl,mux-ext-port = <3>;
};
- usbphy {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "simple-bus";
-
- usbh1phy: usbh1phy@0 {
- compatible = "usb-nop-xceiv";
- reg = <0>;
- clocks = <&clks IMX5_CLK_USB_PHY_GATE>;
- clock-names = "main_clk";
- clock-frequency = <19200000>;
- #phy-cells = <0>;
- };
+ usbh1phy: usbphy {
+ compatible = "usb-nop-xceiv";
+ clocks = <&clks IMX5_CLK_USB_PHY_GATE>;
+ clock-names = "main_clk";
+ clock-frequency = <19200000>;
+ #phy-cells = <0>;
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] ARM: dts: imx51-babbage: Fix USB PHY duplicate unit-address
From: Fabio Estevam @ 2018-05-14 18:29 UTC (permalink / raw)
To: shawnguo; +Cc: Fabio Estevam, devicetree, robh+dt, linux-arm-kernel
From: Fabio Estevam <fabio.estevam@nxp.com>
Currently the following DTC warning is seen with W=1:
arch/arm/boot/dts/imx51-babbage.dtb: Warning (unique_unit_address): /usbphy/usbphy@0: duplicate unit-address (also used in node /usbphy/usbh1phy@0)
Fix it by moving the USB PHY node outside of simple-bus and drop the
unneeded unit-address.
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
arch/arm/boot/dts/imx51-babbage.dts | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index b8ca73d..de46906 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -170,20 +170,13 @@
mux-ext-port = <3>;
};
- usbphy {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "simple-bus";
-
- usbh1phy: usbh1phy@0 {
- compatible = "usb-nop-xceiv";
- reg = <0>;
- clocks = <&clk_usb>;
- clock-names = "main_clk";
- reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
- vcc-supply = <&vusb_reg>;
- #phy-cells = <0>;
- };
+ usbh1phy: usbphy1 {
+ compatible = "usb-nop-xceiv";
+ clocks = <&clk_usb>;
+ clock-names = "main_clk";
+ reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+ vcc-supply = <&vusb_reg>;
+ #phy-cells = <0>;
};
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] ARM: dts: imx6/7: Remove unit-address from anatop regulators
From: Rob Herring @ 2018-05-14 18:15 UTC (permalink / raw)
To: Fabio Estevam
Cc: Shawn Guo,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree, Fabio Estevam
In-Reply-To: <1526304714-23821-1-git-send-email-festevam@gmail.com>
On Mon, May 14, 2018 at 8:31 AM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Remove unit-address and reg property from anatop regulators to fix
> the following DTC warnings with W=1:
>
> arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddcore@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddpu@20c8140)
> arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddcore@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddsoc@20c8140)
> arch/arm/boot/dts/imx6dl-apf6dev.dtb: Warning (unique_unit_address): /soc/aips-bus@2000000/anatop@20c8000/regulator-vddpu@20c8140: duplicate unit-address (also used in node /soc/aips-bus@2000000/anatop@20c8000/regulator-vddsoc@20c8140)
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Send it is a standalone patch instead of a patch series.
>
> arch/arm/boot/dts/imx6qdl.dtsi | 20 ++++++--------------
> arch/arm/boot/dts/imx6sl.dtsi | 20 ++++++--------------
> arch/arm/boot/dts/imx6sx.dtsi | 20 ++++++--------------
> arch/arm/boot/dts/imx6ul.dtsi | 11 +++--------
> arch/arm/boot/dts/imx7s.dtsi | 8 ++------
> 5 files changed, 23 insertions(+), 56 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 69648e2..22942dd 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -692,11 +692,8 @@
> interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
> <0 54 IRQ_TYPE_LEVEL_HIGH>,
> <0 127 IRQ_TYPE_LEVEL_HIGH>;
> - #address-cells = <1>;
> - #size-cells = <0>;
>
> - regulator-1p1@20c8110 {
> - reg = <0x20c8110>;
> + regulator-1p1 {
This could break any users dependent on
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p1";
> regulator-min-microvolt = <1000000>;
> @@ -711,8 +708,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-3p0@20c8120 {
> - reg = <0x20c8120>;
> + regulator-3p0 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd3p0";
> regulator-min-microvolt = <2800000>;
> @@ -727,8 +723,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-2p5@20c8130 {
> - reg = <0x20c8130>;
> + regulator-2p5 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd2p5";
> regulator-min-microvolt = <2250000>;
> @@ -743,8 +738,7 @@
> anatop-enable-bit = <0>;
> };
>
> - reg_arm: regulator-vddcore@20c8140 {
> - reg = <0x20c8140>;
> + reg_arm: regulator-vddcore {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddarm";
> regulator-min-microvolt = <725000>;
> @@ -761,8 +755,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_pu: regulator-vddpu@20c8140 {
> - reg = <0x20c8140>;
> + reg_pu: regulator-vddpu {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddpu";
> regulator-min-microvolt = <725000>;
> @@ -779,8 +772,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_soc: regulator-vddsoc@20c8140 {
> - reg = <0x20c8140>;
> + reg_soc: regulator-vddsoc {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddsoc";
> regulator-min-microvolt = <725000>;
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index 2002db2..8c838ba 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -524,11 +524,8 @@
> interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
> <0 54 IRQ_TYPE_LEVEL_HIGH>,
> <0 127 IRQ_TYPE_LEVEL_HIGH>;
> - #address-cells = <1>;
> - #size-cells = <0>;
>
> - regulator-1p1@20c8110 {
> - reg = <0x20c8110>;
> + regulator-1p1 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p1";
> regulator-min-microvolt = <800000>;
> @@ -543,8 +540,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-3p0@20c8120 {
> - reg = <0x20c8120>;
> + regulator-3p0 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd3p0";
> regulator-min-microvolt = <2800000>;
> @@ -559,8 +555,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-2p5@20c8130 {
> - reg = <0x20c8130>;
> + regulator-2p5 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd2p5";
> regulator-min-microvolt = <2100000>;
> @@ -575,8 +570,7 @@
> anatop-enable-bit = <0>;
> };
>
> - reg_arm: regulator-vddcore@20c8140 {
> - reg = <0x20c8140>;
> + reg_arm: regulator-vddcore {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddarm";
> regulator-min-microvolt = <725000>;
> @@ -593,8 +587,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_pu: regulator-vddpu@20c8140 {
> - reg = <0x20c8140>;
> + reg_pu: regulator-vddpu {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddpu";
> regulator-min-microvolt = <725000>;
> @@ -611,8 +604,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_soc: regulator-vddsoc@20c8140 {
> - reg = <0x20c8140>;
> + reg_soc: regulator-vddsoc {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddsoc";
> regulator-min-microvolt = <725000>;
> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
> index 7e463d2..4a97513 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -591,11 +591,8 @@
> interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
> - #address-cells = <1>;
> - #size-cells = <0>;
>
> - regulator-1p1@20c8110 {
> - reg = <0x20c8110>;
> + regulator-1p1 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p1";
> regulator-min-microvolt = <800000>;
> @@ -610,8 +607,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-3p0@20c8120 {
> - reg = <0x20c8120>;
> + regulator-3p0 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd3p0";
> regulator-min-microvolt = <2800000>;
> @@ -626,8 +622,7 @@
> anatop-enable-bit = <0>;
> };
>
> - regulator-2p5@20c8130 {
> - reg = <0x20c8130>;
> + regulator-2p5 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd2p5";
> regulator-min-microvolt = <2100000>;
> @@ -642,8 +637,7 @@
> anatop-enable-bit = <0>;
> };
>
> - reg_arm: regulator-vddcore@20c8140 {
> - reg = <0x20c8140>;
> + reg_arm: regulator-vddcore {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddarm";
> regulator-min-microvolt = <725000>;
> @@ -660,8 +654,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_pcie: regulator-vddpcie@20c8140 {
> - reg = <0x20c8140>;
> + reg_pcie: regulator-vddpcie {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddpcie";
> regulator-min-microvolt = <725000>;
> @@ -677,8 +670,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_soc: regulator-vddsoc@20c8140 {
> - reg = <0x20c8140>;
> + reg_soc: regulator-vddsoc {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddsoc";
> regulator-min-microvolt = <725000>;
> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
> index 2b854d1..1818b6c 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -551,11 +551,8 @@
> interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
> - #address-cells = <1>;
> - #size-cells = <0>;
>
> - reg_3p0: regulator-3p0@20c8110 {
> - reg = <0x20c8110>;
> + reg_3p0: regulator-3p0 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd3p0";
> regulator-min-microvolt = <2625000>;
> @@ -569,8 +566,7 @@
> anatop-enable-bit = <0>;
> };
>
> - reg_arm: regulator-vddcore@20c8140 {
> - reg = <0x20c8140>;
> + reg_arm: regulator-vddcore {
> compatible = "fsl,anatop-regulator";
> regulator-name = "cpu";
> regulator-min-microvolt = <725000>;
> @@ -587,8 +583,7 @@
> anatop-max-voltage = <1450000>;
> };
>
> - reg_soc: regulator-vddsoc@20c8140 {
> - reg = <0x20c8140>;
> + reg_soc: regulator-vddsoc {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vddsoc";
> regulator-min-microvolt = <725000>;
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index b416d2b..99f92ec 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -557,11 +557,8 @@
> reg = <0x30360000 0x10000>;
> interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> - #address-cells = <1>;
> - #size-cells = <0>;
>
> - reg_1p0d: regulator-vdd1p0d@30360210 {
> - reg = <0x30360210>;
> + reg_1p0d: regulator-vdd1p0d {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p0d";
> regulator-min-microvolt = <800000>;
> @@ -575,8 +572,7 @@
> anatop-enable-bit = <0>;
> };
>
> - reg_1p2: regulator-vdd1p2@30360220 {
> - reg = <0x30360220>;
> + reg_1p2: regulator-vdd1p2 {
> compatible = "fsl,anatop-regulator";
> regulator-name = "vdd1p2";
> regulator-min-microvolt = <1100000>;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v3] display: panel: Add AUO g070vvn01 display support (800x480)
From: Rob Herring @ 2018-05-14 18:12 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Mark Rutland, devicetree, David Airlie,
linux-kernel@vger.kernel.org, dri-devel, Thierry Reding
In-Reply-To: <20180514173621.797c6b8f@jawa>
On Mon, May 14, 2018 at 10:36 AM, Lukasz Majewski <lukma@denx.de> wrote:
> Dear All,
It is only Thierry that you need to ping. It goes thru his tree as I
mentioned on another panel you sent.
>> This commit adds support for AUO's 7.0" display.
>
> If I may gentle remind about this patch....
>
>>
>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
You failed to add my Reviewed-by.
Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight
From: Adam Ford @ 2018-05-14 18:04 UTC (permalink / raw)
To: Sekhar Nori
Cc: devicetree, Kevin Hilman, jsarha, Rob Herring, Tomi Valkeinen,
Laurent Pinchart, arm-soc
In-Reply-To: <825847d0-c29c-859b-43fd-0987f94e28a5@ti.com>
On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Monday 14 May 2018 04:22 PM, Adam Ford wrote:
>> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote:
>>> Hi Adam,
Added Tomi, Laurent, and Jyri for feedback.
>>>
>>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote:
>>>> When using the board files the LCD works, but not with the DT.
>>>> This adds enables the original da850-evm to work with the same
>>>> LCD in device tree mode.
>>>>
>>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and
>>>> the vpif display pins are mutually exclusive, so if using the LCD,
>>>> do not load the vpif driver.
>>>
>>> Its not sufficient just note this in patch description.
>>>
>>> a) Disable (status = "disabled") the VPIF node which clashes for pins
>>> with LCD.
>>> b) Add a comment on top of the status = "disabled" giving information on
>>> how user can enable it (disable lcdc node and then change to status =
>>> "okay").
>>>
>>>>
>>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>>> ---
>>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to
>>>> backlight which better matches the schematic. Updated the description to explain
>>>> that it cannot be used at the same time as the vpif driver.
>>>>
>>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
>>>> index 2e817da37fdb..3f1c8be07efe 100644
>>>> --- a/arch/arm/boot/dts/da850-evm.dts
>>>> +++ b/arch/arm/boot/dts/da850-evm.dts
>>>> @@ -27,6 +27,50 @@
>>>> spi0 = &spi1;
>>>> };
>>>>
>>>> + backlight {
>>>> + compatible = "gpio-backlight";
>>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */
>>>
>>> The gpio-backlight binding does not describe a property called
>>> enable-gpios. It should just be gpios.
>>
>> I will fix that.
>>
>>>
>>> a) Are you using gpio-backlight because you are not able to get the PWM
>>> to work?
>>>
>> Yes, You told me not to worry about doing a PWM backlight because the
>> legacy board does not PWM either.
>
> Yeah, I meant not to add backlight control till the time we are able to
> get it working using PWM. Is this needed for the basic LCD functionality
> to work? I would like to avoid the churn of adding it using GPIO now and
> changing to PWM later, if possible.
>
>>
>>> b) What is GP0[7] connected to in the schematic you have? In the
>>> schematic I have I see LCD_PWM0 is connected to
>>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12.
>>
>> I have schematic 1016572 dated Wednesday, August 18, 2010. According
>> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25,
>> Pin 46 to generate M_LCD_PWM0. You might have one of the early,
>> pre-release versions.
>
> Ah, okay. In your schematic, is GP2[14] connected to anything?
>
>>
>>>
>>> c) The /* GP0[7] */ comment is not really useful on its own as it can be
>>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0".
>>> Same for other places like this below.
>>
>> I can do that.
>>>
>>>> @@ -35,6 +79,16 @@
>>>> regulator-boot-on;
>>>> };
>>>>
>>>> + backlight_reg: backlight-regulator {
>>>> + compatible = "regulator-fixed";
>>>> + regulator-name = "lcd_backlight_pwr";
>>>> + regulator-min-microvolt = <3300000>;
>>>> + regulator-max-microvolt = <3300000>;
>>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */
>>>> + regulator-always-on;
>>>
>>> Why should this regulator never be disabled?
>>
>> The gpio-backlight does not have a way that I can see to associate the
>> regulator to it. I read through the bindings, but I didn't see an
>> option to associate a regulator it. I use this regulator to drive
>> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without
>> this option, the system disables lcd_backlight_pwr and the screen is
>> blank
>
> It sounds like this is a hack to enable backlight on this board. I think
> either the backlight driver needs to gain functionality to enable the
> GPIO. Or backlight could be treated as part of the panel and enabled
> using enable-gpios property in the panel. TBH, I will be okay either
> way. Can you check with Jyri, Tomi and rest of the DRM folks on what
> should be right way of dealing with this?
Per your request I added them into this thread. I added Tomi, Jyri,
and Laurent to this as Laurent's name is associated with the gpio
backlight driver.
I am not sure why you think it's a hack. I pulled up the schematic
for the LCD to see what it's doing, and the lcd_backlight_pwr pin
controls the power-on sequence of the back-light controller. Without
this, there is no power, so it seems to me that the 'regulator-fixed'
device is the correct way to do it.
The separate pin associated to the gpio is used to tell the backlight
IC to actually turn on/off the back-light. Ideally it seems like it
would nice to have the gpio-backlight driver be able to specify the
regulator, so when the backlight is in use, it would power the
regulator, but until that's available, the it seems like
'regulator-always-on' is the way to make it stay on.
Laurent, Jyri, Tomi, do you have any thoughts on this matter?
Thanks,
adam
>
> Thanks,
> Sekhar
^ permalink raw reply
* Re: [PATCH 4/6] drm/panel: simple: Add support for Banana Pi 7" S070WV20-CT16 panel
From: Jagan Teki @ 2018-05-14 18:03 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, David Airlie, Thierry Reding, Rob Herring,
Mark Rutland, dri-devel, linux-arm-kernel, linux-kernel,
devicetree, Jonathan Liu
In-Reply-To: <20180419093225.614-5-wens@csie.org>
On Thu, Apr 19, 2018 at 3:02 PM, Chen-Yu Tsai <wens@csie.org> wrote:
> This panel is marketed as Banana Pi 7" LCD display. On the back is
> a sticker denoting the model name S070WV20-CT16.
>
> This is a 7" 800x480 panel connected through a 24-bit RGB interface.
> However the panel only does 262k colors.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> .../display/panel/bananapi,s070wv20-ct16.txt | 7 ++++++
> drivers/gpu/drm/panel/panel-simple.c | 25 +++++++++++++++++++
> 2 files changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
>
> diff --git a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> new file mode 100644
> index 000000000000..2ec35ce36e9a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt
> @@ -0,0 +1,7 @@
> +Banana Pi 7" (S070WV20-CT16) TFT LCD Panel
> +
> +Required properties:
> +- compatible: should be "bananapi,s070wv20-ct16"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index cbf1ab404ee7..9bc037f74d6c 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -745,6 +745,28 @@ static const struct panel_desc avic_tm070ddh03 = {
> },
> };
>
> +static const struct drm_display_mode bananapi_s070wv20_ct16_mode = {
> + .clock = 30000,
> + .hdisplay = 800,
> + .hsync_start = 800 + 40,
> + .hsync_end = 800 + 40 + 48,
> + .htotal = 800 + 40 + 48 + 40,
> + .vdisplay = 480,
> + .vsync_start = 480 + 13,
> + .vsync_end = 480 + 13 + 3,
> + .vtotal = 480 + 13 + 3 + 29,
> +};
> +
> +static const struct panel_desc bananapi_s070wv20_ct16 = {
> + .modes = &bananapi_s070wv20_ct16_mode,
> + .num_modes = 1,
> + .bpc = 6,
> + .size = {
> + .width = 154,
> + .height = 86,
> + },
> +};
I think this parallel RGB interface right? I too have same display
with DSI I'm sure these setting will not useful right? do we need to
write separate panel driver for that?
^ permalink raw reply
* Re: [patch v20 1/4] drivers: jtag: Add JTAG core driver
From: Randy Dunlap @ 2018-05-14 18:01 UTC (permalink / raw)
To: Oleksandr Shamray, gregkh, arnd
Cc: system-sw-low-level, devicetree, jiri, vadimp, linux-api, openbmc,
linux-kernel, openocd-devel-owner, Jiri Pirko, robh+dt, joel,
linux-serial, tklauser, mchehab, davem, linux-arm-kernel
In-Reply-To: <1526314803-3532-2-git-send-email-oleksandrs@mellanox.com>
On 05/14/2018 09:20 AM, Oleksandr Shamray wrote:
Hi,
Just a bit more Kconfig fixing below.
> ---
> Documentation/ioctl/ioctl-number.txt | 2 +
> MAINTAINERS | 10 ++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/jtag/Kconfig | 18 +++
> drivers/jtag/Makefile | 1 +
> drivers/jtag/jtag.c | 274 ++++++++++++++++++++++++++++++++++
> include/linux/jtag.h | 41 +++++
> include/uapi/linux/jtag.h | 105 +++++++++++++
> 9 files changed, 454 insertions(+), 0 deletions(-)
> create mode 100644 drivers/jtag/Kconfig
> create mode 100644 drivers/jtag/Makefile
> create mode 100644 drivers/jtag/jtag.c
> create mode 100644 include/linux/jtag.h
> create mode 100644 include/uapi/linux/jtag.h
> diff --git a/drivers/jtag/Kconfig b/drivers/jtag/Kconfig
> new file mode 100644
> index 0000000..ba72a2b
> --- /dev/null
> +++ b/drivers/jtag/Kconfig
> @@ -0,0 +1,18 @@
> +menuconfig JTAG
> + tristate "JTAG support"
> + help
> + This provides basic core functionality support for JTAG class devices
(end sentence above with '.') devices.
> + Hardware that is equipped with a JTAG microcontroller can be
> + supported by using this driver's interfaces.
> + This driver exposes a set of IOCTL to the user space for
IOCTLs
> + another commands:
various commands:
or
the following commands:
or
associated commands:
> + SDR: (Scan Data Register) Performs an IEEE 1149.1 Data Register scan
> + SIR: (Scan Instruction Register) Performs an IEEE 1149.1 Instruction
> + Register scan.
> + RUNTEST: Forces the IEEE 1149.1 bus to a run state for a specified
> + number of clocks or a specified time period.
> +
> + If you want this support, you should say Y here.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called jtag.
thanks,
--
~Randy
^ permalink raw reply
* Re: [PATCH] thermal: qcom: tsens: Allow number of sensors to come from DT
From: Bjorn Andersson @ 2018-05-14 17:46 UTC (permalink / raw)
To: Amit Kucheria
Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
Rajendra Nayak, Linux PM list, devicetree, LKML, linux-arm-msm
In-Reply-To: <CAHLCerN9zQEJK4WoV6BY6hga2BxwzMsyScWnTdYcLgEk0Bjg4w@mail.gmail.com>
On Mon 14 May 07:45 PDT 2018, Amit Kucheria wrote:
> On Tue, May 8, 2018 at 2:53 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > For platforms that has multiple copies of the TSENS hardware block it's
> > necessary to be able to specify the number of sensors per block in DeviceTree.
>
> I assume you want to replace the hardcoded num_sensors values in the
> tsens-xxxx.c files with values in device tree atleast on platforms
> that use devicetree?
>
Right, e.g. in msm8998 we have two blocks with 12 and 8 sensors
respectively. With the patch we will overwrite the num_sensors with a
value from DT, iff specified.
As probe will fail if num_sensors is 0 we can reuse ops_8996 and just
not provide num_sensors for 8998, hence requiring it to be specified in
DT.
PS. Looking at the register spec 8996 too seems to have two tsens
blocks, so in order to access sensors off the second bank this patch
would be needed (and would work and be backwards compatible).
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Andy Shevchenko @ 2018-05-14 17:38 UTC (permalink / raw)
To: Radu Pirea, devicetree, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, linux-arm Mailing List, linux-spi,
Mark Rutland, Rob Herring, Lee Jones, Greg Kroah-Hartman,
Jiri Slaby, Richard Genoud, alexandre.belloni, Nicolas Ferre,
Mark Brown
In-Reply-To: <5a3930b867cf8c279953d08c5d5dd1d93113a43b.camel@microchip.com>
First of all, do not remove mailing lists from Cc and people if you
are not sure they do not need your stuff.
On Mon, May 14, 2018 at 11:11 AM, Radu Pirea <radu.pirea@microchip.com> wrote:
> On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
>> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
>> > wrote:
>> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
>> > +{
>> > + struct at91_usart_spi_device *ausd = spi->controller_state;
>> > +
>> > + if (!ausd)
>> > + return;
>>
>> Is it even possible?
> Theoretically yes.
I would like to know real circumstances when it might happen.
>>
>> Anyway the code below will work fine even if it's the case.
>>
>> > +
>> > + spi->controller_state = NULL;
>> > + kfree(ausd);
>> > +}
>> The question is, why you didn't utilize what SPI core provides you?
> I tried, but it did not work the way I expected.
So, what is not going as expected in "SPI core takes care of CSs" case?
Did you use oscilloscope for that?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings/display/bridge: sii902x: add optional power supplies
From: Laurent Pinchart @ 2018-05-14 17:06 UTC (permalink / raw)
To: Philippe CORNU
Cc: Mark Rutland, Alexandre TORGUE, devicetree@vger.kernel.org,
David Airlie, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, Yannick FERTRE
In-Reply-To: <7fe8e766-56f5-38a3-b4dd-b44d4a5bdcf8@st.com>
Hi Philippe,
On Monday, 14 May 2018 12:22:16 EEST Philippe CORNU wrote:
> On 04/26/2018 12:05 AM, Laurent Pinchart wrote:
> > On Wednesday, 25 April 2018 20:11:23 EEST Rob Herring wrote:
> >> On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote:
> >>> On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote:
> >>>> On 04/25/2018 11:01 AM, Laurent Pinchart wrote:
> >>>>> On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote:
> >>>>>> Add optional power supplies using the description found in
> >>>>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)".
> >>>>>>
> >>>>>> There is a single 1v2 supply voltage named vcc12 from which cvcc12
> >>>>>> (digital core) and avcc12 (TMDS analog) are derived because
> >>>>>> according to this data sheet:
> >>>>>> "cvcc12 and avcc12 can be derived from the same power source"
> >>>>>
> >>>>> Shouldn't the power supplies be mandatory, as explained by Mark in
> >>>>> https://lists.freedesktop.org/archives/dri-devel/2018-April/
> >>>>> 172400.html ?
> >>>>
> >>>> Laurent,
> >>>> Many thanks Laurent for your comment, I understood the merge of the
> >>>> two 1v2 power supplies but missed the "mandatory" part... maybe because
> >>>> this patch (with optional power supplies) already got the reviewed-by
> >>>> from Rob, I thought the discussion thread you pointed out was
> >>>> applicable "only" to totally new driver documentation.
> >>>>
> >>>> So, on my side, as a "new user" of sii902x IC, no problem to put these
> >>>> power supplies as mandatory instead of optional properties but I would
> >>>> like to be sure this is applicable to both old and new bindings doc :
> >>>> )
> >>>
> >>> We obviously need to retain backward compatibility, so on the driver
> >>> side you need to treat those power supplies as optional. From a DT
> >>> bindings point of view, however, I think they should be mandatory for
> >>> new DT.
> >>
> >> We don't really have a way to describe these 3 conditions (required for
> >> all, optional for all, and required for new). So generally we make
> >> additions optional. The exception sometimes is if we update all the dts
> >> files.
> >
> > Can't we just make it mandatory in the bindings, as long as we treat it
> > as optional in drivers ?
>
> How to progress on this patch? Do you have any suggestions?
By seeing what Rob thinks about my proposal above ? :-)
> >>>> Rob,
> >>>> could you please confirm these power supply properties should be
> >>>> "mandatory"? if yes, should we then modify other optional properties
> >>>> like the reset-gpios too in the future?
> >>>
> >>> The GPIOs properties are different in my opinion, as there's no
> >>> requirement to connect for instance the reset pin to a GPIO controllable
> >>> by the SoC. The pin could be hardwired to VCC, or connected to a system
> >>> reset that is automatically managed without SoC intervention. The power
> >>> supplies, however, are mandatory, in the sense that the chip will not
> >>> work if you leave the power supplies unconnected.
> >>
> >> DT only needs to describe what matters to s/w. If a regulator is
> >> fixed and you don't need to know its voltage (or other read-only
> >> parameters), then there's not much point in putting it in DT.
> >>
> >> I'd probably base this more at a platform level and you either use
> >> regulator binding or you don't. It's perfectly valid that you want to do
> >> things like regulator setup, pin ctrl and muxing setup, etc. all in
> >> firmware and the OS doesn't touch any of that.
> >>
> >> That's all a big can of worms which we shouldn't solve on this 2 line
> >> change. I think this change is fine as-is, so:
> >>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ 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