* Re: [PATCH v4 2/4] net: phy: Add function to retrieve LED configuration from the DT
From: Matthias Kaehlcke @ 2019-08-02 17:59 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190802163810.GL2099@lunn.ch>
On Fri, Aug 02, 2019 at 06:38:10PM +0200, Andrew Lunn wrote:
> On Thu, Aug 01, 2019 at 12:07:57PM -0700, Matthias Kaehlcke wrote:
> > Add a phylib function for retrieving PHY LED configuration that
> > is specified in the device tree using the generic binding. LEDs
> > can be configured to be 'on' for a certain link speed or to blink
> > when there is TX/RX activity.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > Changes in v4:
> > - patch added to the series
> > ---
> > drivers/net/phy/phy_device.c | 50 ++++++++++++++++++++++++++++++++++++
> > include/linux/phy.h | 15 +++++++++++
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 6b5cb87f3866..b4b48de45712 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -2188,6 +2188,56 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> > return phydrv->config_intr && phydrv->ack_interrupt;
> > }
> >
> > +int of_get_phy_led_cfg(struct phy_device *phydev, int led,
> > + struct phy_led_config *cfg)
> > +{
> > + struct device_node *np, *child;
> > + const char *trigger;
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_OF_MDIO))
> > + return -ENOENT;
> > +
> > + np = of_find_node_by_name(phydev->mdio.dev.of_node, "leds");
> > + if (!np)
> > + return -ENOENT;
> > +
> > + for_each_child_of_node(np, child) {
> > + u32 val;
> > +
> > + if (!of_property_read_u32(child, "reg", &val)) {
> > + if (val == (u32)led)
> > + break;
> > + }
> > + }
>
> Hi Matthias
>
> This is leaking references to np and child. In the past we have not
> cared about this too much, but we are now getting patches adding the
> missing releases. So it would be good to fix this.
Good point, I'll fix it in the next revision.
Thanks
Matthias
^ permalink raw reply
* Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support
From: Stephen Boyd @ 2019-08-02 17:51 UTC (permalink / raw)
To: Dmitry Osipenko, Sowjanya Komatineni, jason, jonathanh,
linus.walleij, marc.zyngier, mark.rutland, stefan, tglx,
thierry.reding
Cc: pdeschrijver, pgaikwad, linux-clk, linux-gpio, jckuo, josephl,
talho, linux-tegra, linux-kernel, mperttunen, spatra, robh+dt,
devicetree
In-Reply-To: <8c259511-d8ea-51b2-0b1d-c85b964bc44c@gmail.com>
Quoting Dmitry Osipenko (2019-07-22 00:12:17)
> 22.07.2019 10:09, Dmitry Osipenko пишет:
> > 22.07.2019 9:52, Sowjanya Komatineni пишет:
> >>
> >> On 7/21/19 11:10 PM, Dmitry Osipenko wrote:
> >>> 22.07.2019 1:45, Sowjanya Komatineni пишет:
> >>>> On 7/21/19 2:38 PM, Dmitry Osipenko wrote:
> >>>>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
> >>>>>> @@ -2853,9 +2859,8 @@ static int tegra210_enable_pllu(void)
> >>>>>> reg |= PLL_ENABLE;
> >>>>>> writel(reg, clk_base + PLLU_BASE);
> >>>>>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
> >>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
> >>>>>> - if (!(reg & PLL_BASE_LOCK)) {
> >>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
> >>>>>> + if (ret) {
> >>>>> Why this is needed? Was there a bug?
> >>>>>
> >>>> during resume pllu init is needed and to use same terga210_init_pllu,
> >>>> poll_timeout_atomic can't be used as its ony for atomic context.
> >>>>
> >>>> So changed to use wait_for_mask which should work in both cases.
> >>> Atomic variant could be used from any context, not sure what do you
> >>> mean. The 'atomic' part only means that function won't cause scheduling
> >>> and that's it.
> >>
> >> Sorry, replied incorrect. readx_poll_timeout_atomic uses ktime_get() and
> >> during resume timekeeping suspend/resume happens later than clock
> >> suspend/resume. So using tegra210_wait_for_mask.
> >>
> >> both timekeeping and clk-tegra210 drivers are registered as syscore but
> >> not ordered.
> >
> > Okay, thank you for the clarification.
> >
> > [snip]
> >
>
> You should remove the 'iopoll.h' then, since it's not used anymore.
And also add a comment to this location in the code because it's
non-obvious that we can't use iopoll here.
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Dmitry Osipenko @ 2019-08-02 17:35 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <20190802130537.GB3883@pdeschrijver-desktop.Nvidia.com>
02.08.2019 16:05, Peter De Schrijver пишет:
> On Thu, Jul 25, 2019 at 01:59:09PM +0300, Dmitry Osipenko wrote:
>> 25.07.2019 13:38, Peter De Schrijver пишет:
>>> On Thu, Jul 25, 2019 at 01:33:48PM +0300, Peter De Schrijver wrote:
>>>> On Thu, Jul 25, 2019 at 01:05:13PM +0300, Dmitry Osipenko wrote:
>>>>> 25.07.2019 12:55, Peter De Schrijver пишет:
>>>>>> On Mon, Jul 22, 2019 at 12:54:51PM +0300, Dmitry Osipenko wrote:
>>>>>>>
>>>>>>> All Tegra SoCs support SC7, hence the 'supports_sc7' and the comment
>>>>>>> doesn't sound correct to me. Something like 'firmware_sc7' should suit
>>>>>>> better here.
>>>>>>>
>>>>>>>> + writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
>>>>>>>
>>>>>>> Secondly, I'm also not sure why COP interrupts need to be disabled for
>>>>>>> pre-T210 at all, since COP is unused. This looks to me like it was
>>>>>>> cut-n-pasted from downstream kernel without a good reason and could be
>>>>>>> simply removed.
>>>>>>
>>>>>> I don't think we can rely on the fact that COP is unused. People can
>>>>>> write their own code to run on COP.
>>>>>
>>>>> 1. Not upstream - doesn't matter.
>>>>>
>>>>
>>>> The code is not part of the kernel, so obviously it's not upstream?
>>>>
>>>>> 2. That's not very good if something unknown is running on COP and then
>>>>> kernel suddenly intervenes, don't you think so?
>>>>
>>>> Unless the code was written with this in mind.
>>>>
>>
>> In that case, please see 1. ;)
>>
>
> In general the kernel should not touch the COP interrupts I think.
>
>>>
>>> Looking at this again, I don't think we need to enable the IRQ at all.
>>
>> Could you please clarify? The code only saves/restores COP's interrupts
>> context across suspend-resume.
>
> The sc7 entry firmware doesn't use interrupts.
Okay, it shouldn't hurt to clean up the LIC's code a tad by removing the
COP's bits, will make a patch.
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 3/3] ASoC: TDA7802: Add turn-on diagnostic routine
From: Mark Brown @ 2019-08-02 17:27 UTC (permalink / raw)
To: Thomas Preston
Cc: Mark Rutland, devicetree, alsa-devel, Charles Keepax,
Kuninori Morimoto, Kirill Marinushkin, Liam Girdwood,
Marco Felsch, Annaliese McDermond, Takashi Iwai, Paul Cercueil,
Vinod Koul, Rob Herring, Srinivas Kandagatla, Jerome Brunet,
linux-kernel, Cheng-Yi Chiang
In-Reply-To: <ab0a2d14-90c0-6c28-2c80-351fccd85e68@codethink.co.uk>
[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]
On Fri, Aug 02, 2019 at 03:51:09PM +0100, Thomas Preston wrote:
> On 02/08/2019 12:10, Mark Brown wrote:
> > You can use a read only control for the readback, or just have it be
> > triggered by overwriting the readback value. You can cache the result.
> Keeping the trigger and result together like that would be better I think,
> although the routine isn't supposed to run mid way through playback. If
> we're mid playback the debugfs routine has to turn off AMP_ON, take the
> device back to a known state, run diagnostics, then restore. Which causes
> a gap in the audible sound.
Whatever method is used to do the triggering can always return -EBUSY
when you someone tries to do so during playback.
> >> Kirill Marinushkin mentioned this in the first review [0], it just didn't
> >> really sink in until now!
> > You could do that too, yeah. Depends on what this is diagnosing and if
> > that'd be useful.
> The diagnostic status bits describe situations such as:
> - open load (no speaker connected)
> - short to GND
> - short to VCC
> - etc
> The intention is to test if all the speakers are connected. So, one might
> have a self test which runs the diagnostic and verifies it outputs:
...
> I think the module parameter method is more appropriate for a
> "Turn-on diagnostic", even though I don't really like grepping dmesg
> for the result. I'll go ahead and implement that unless anyone has a
> particular preference for the kcontrol-trigger.
Right. It's not ideal for use in production systems for example but
perhaps fine for support techs or whoever. Up to you anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 3/8] of/fdt: add function to get the SoC wide DMA addressable memory size
From: Rob Herring @ 2019-08-02 17:17 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Catalin Marinas, Christoph Hellwig, wahrenst, Marc Zyngier,
Robin Murphy,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree, Linux IOMMU, linux-mm, Frank Rowand, phill,
Florian Fainelli, Will Deacon, linux-kernel@vger.kernel.org,
Eric Anholt, Matthias Brugger, Andrew Morton, Marek Szyprowski
In-Reply-To: <20190731154752.16557-4-nsaenzjulienne@suse.de>
On Wed, Jul 31, 2019 at 9:48 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Some SoCs might have multiple interconnects each with their own DMA
> addressing limitations. This function parses the 'dma-ranges' on each of
> them and tries to guess the maximum SoC wide DMA addressable memory
> size.
>
> This is specially useful for arch code in order to properly setup CMA
> and memory zones.
We already have a way to setup CMA in reserved-memory, so why is this
needed for that?
I have doubts this can really be generic...
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>
> drivers/of/fdt.c | 72 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_fdt.h | 2 ++
> 2 files changed, 74 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 9cdf14b9aaab..f2444c61a136 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -953,6 +953,78 @@ int __init early_init_dt_scan_chosen_stdout(void)
> }
> #endif
>
> +/**
> + * early_init_dt_dma_zone_size - Look at all 'dma-ranges' and provide the
> + * maximum common dmable memory size.
> + *
> + * Some devices might have multiple interconnects each with their own DMA
> + * addressing limitations. For example the Raspberry Pi 4 has the following:
> + *
> + * soc {
> + * dma-ranges = <0xc0000000 0x0 0x00000000 0x3c000000>;
> + * [...]
> + * }
> + *
> + * v3dbus {
> + * dma-ranges = <0x00000000 0x0 0x00000000 0x3c000000>;
> + * [...]
> + * }
> + *
> + * scb {
> + * dma-ranges = <0x0 0x00000000 0x0 0x00000000 0xfc000000>;
> + * [...]
> + * }
> + *
> + * Here the area addressable by all devices is [0x00000000-0x3bffffff]. Hence
> + * the function will write in 'data' a size of 0x3c000000.
> + *
> + * Note that the implementation assumes all interconnects have the same physical
> + * memory view and that the mapping always start at the beginning of RAM.
Not really a valid assumption for general code.
> + */
> +int __init early_init_dt_dma_zone_size(unsigned long node, const char *uname,
> + int depth, void *data)
Don't use the old fdt scanning interface with depth/data. It's not
really needed now because you can just use libfdt calls.
> +{
> + const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> + u64 phys_addr, dma_addr, size;
> + u64 *dma_zone_size = data;
> + int dma_addr_cells;
> + const __be32 *reg;
> + const void *prop;
> + int len;
> +
> + if (depth == 0)
> + *dma_zone_size = 0;
> +
> + /*
> + * We avoid pci host controllers as they have their own way of using
> + * 'dma-ranges'.
> + */
> + if (type && !strcmp(type, "pci"))
> + return 0;
> +
> + reg = of_get_flat_dt_prop(node, "dma-ranges", &len);
> + if (!reg)
> + return 0;
> +
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + if (prop)
> + dma_addr_cells = be32_to_cpup(prop);
> + else
> + dma_addr_cells = 1; /* arm64's default addr_cell size */
Relying on the defaults has been a dtc error longer than arm64 has
existed. If they are missing, just bail.
> +
> + if (len < (dma_addr_cells + dt_root_addr_cells + dt_root_size_cells))
> + return 0;
> +
> + dma_addr = dt_mem_next_cell(dma_addr_cells, ®);
> + phys_addr = dt_mem_next_cell(dt_root_addr_cells, ®);
> + size = dt_mem_next_cell(dt_root_size_cells, ®);
> +
> + if (!*dma_zone_size || *dma_zone_size > size)
> + *dma_zone_size = size;
> +
> + return 0;
> +}
It's possible to have multiple levels of nodes and dma-ranges. You
need to handle that case too.
Doing that and handling differing address translations will be
complicated. IMO, I'd just do:
if (of_fdt_machine_is_compatible(blob, "brcm,bcm2711"))
dma_zone_size = XX;
2 lines of code is much easier to maintain than 10s of incomplete code
and is clearer who needs this. Maybe if we have dozens of SoCs with
this problem we should start parsing dma-ranges.
Rob
^ permalink raw reply
* Re: [PATCH v4 1/4] dt-bindings: net: phy: Add subnode for LED configuration
From: Andrew Lunn @ 2019-08-02 16:57 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190801190759.28201-2-mka@chromium.org>
On Thu, Aug 01, 2019 at 12:07:56PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Ethernet PHYs is configurable. Add an
> optional 'leds' subnode with a child node for each LED to be
> configured. The binding aims to be compatible with the common
> LED binding (see devicetree/bindings/leds/common.txt).
>
> A LED can be configured to be 'on' when a link with a certain speed
> is active, or to blink on RX/TX activity. For the configuration to
> be effective it needs to be supported by the hardware and the
> corresponding PHY driver.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - patch added to the series
> ---
> .../devicetree/bindings/net/ethernet-phy.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index f70f18ff821f..81c5aacc89a5 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -153,6 +153,38 @@ properties:
> Delay after the reset was deasserted in microseconds. If
> this property is missing the delay will be skipped.
>
> +patternProperties:
> + "^leds$":
> + type: object
> + description:
> + Subnode with configuration of the PHY LEDs.
> +
> + patternProperties:
> + "^led@[0-9]+$":
> + type: object
> + description:
> + Subnode with the configuration of a single PHY LED.
> +
> + properties:
> + reg:
> + description:
> + The ID number of the LED, typically corresponds to a hardware ID.
> + $ref: "/schemas/types.yaml#/definitions/uint32"
> +
> + linux,default-trigger:
> + description:
> + This parameter, if present, is a string specifying the trigger
> + assigned to the LED. Supported triggers are:
> + "phy_link_10m_active" - LED will be on when a 10Mb/s link is active
> + "phy_link_100m_active" - LED will be on when a 100Mb/s link is active
> + "phy_link_1g_active" - LED will be on when a 1Gb/s link is active
> + "phy_link_10g_active" - LED will be on when a 10Gb/s link is active
> + "phy_activity" - LED will blink when data is received or transmitted
Matthias
We should think a bit more about these names.
I can see in future needing 1G link, but it blinks off when there is
active traffic? So phy_link_1g_active could be confusing, and very similar to
phy_link_1g_activity? So maybe
> + "phy_link_10m" - LED will be solid on when a 10Mb/s link is active
> + "phy_link_100m" - LED will be solid on when a 100Mb/s link is active
> + "phy_link_1g" - LED will be solid on when a 1Gb/s link is active
etc.
And then in the future we can have
"phy_link_1g_activity' - LED will be on when 1Gbp/s
link is active and blink off
with activity.
What other use cases do we have? I don't want to support everything,
but we should be able to represent the most common modes without the
names getting too confusing.
Andrew
^ permalink raw reply
* Re: [PATCH 2/4] counter: new TI eQEP driver
From: William Breathitt Gray @ 2019-08-02 16:40 UTC (permalink / raw)
To: David Lechner
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm
In-Reply-To: <680ed555-c5db-6640-8fd3-121422077eff@lechnology.com>
On Fri, Aug 02, 2019 at 11:17:11AM -0500, David Lechner wrote:
> On 8/2/19 4:27 AM, William Breathitt Gray wrote:
> >> +static const struct counter_ops ti_eqep_counter_ops = {
> >> + .count_read = ti_eqep_count_read,
> >> + .count_write = ti_eqep_count_write,
> >> + .function_get = ti_eqep_function_get,
> >> + .function_set = ti_eqep_function_set,
> >> +};
> > Are you able to provide a signal_read function, or are the Signals not
> > exposed to the user by this device? Sometimes quadrature encoder devices
> > provide an instanteous read of the signal lines to tell whether they are
> > high or low, so I figured I'd ask.
>
> No, it does not look like these signals can be read directly.
All right, in that case you can ignore signal_read.
> >
> > You should define an action_get function as well along with Synapses
> > corresponding to each Signal. This will allow users to know whether the
> > Synapse fires on a rising edge, falling edge, no edge, or both edges.
> >
> > For example, consider the drivers/counter/104-quad-8.c file. Each count
> > register has three associated signal lines: Quadrature A, Quadrature B,
> > and Index.
> >
> > Quadrature A and B are your typical quadrature encoder lines and
> > depending on the function mode selected (quadrature x4, pulse-direction,
> > etc.) could have a Synapse action mode of none, rising edge, falling
> > edge, or both edges; see the quad8_synapse_actions_list array.
> >
> > In contrast, the Index signal line only has two Synapse action modes:
> > rising edge (in the case preset functionality is enabled) or none.
>
> The encoders I have don't use the index or strobe signals, so I was
> thinking maybe I should omit those two signals from the driver for the
> time being since I don't have a way of testing.
That should be fine for now. We can add them in a later patch down the
road and keep this introduction patch simple.
William Breathitt Gray
^ permalink raw reply
* Re: [PATCH v4 2/4] net: phy: Add function to retrieve LED configuration from the DT
From: Andrew Lunn @ 2019-08-02 16:38 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190801190759.28201-3-mka@chromium.org>
On Thu, Aug 01, 2019 at 12:07:57PM -0700, Matthias Kaehlcke wrote:
> Add a phylib function for retrieving PHY LED configuration that
> is specified in the device tree using the generic binding. LEDs
> can be configured to be 'on' for a certain link speed or to blink
> when there is TX/RX activity.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - patch added to the series
> ---
> drivers/net/phy/phy_device.c | 50 ++++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 15 +++++++++++
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 6b5cb87f3866..b4b48de45712 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2188,6 +2188,56 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> return phydrv->config_intr && phydrv->ack_interrupt;
> }
>
> +int of_get_phy_led_cfg(struct phy_device *phydev, int led,
> + struct phy_led_config *cfg)
> +{
> + struct device_node *np, *child;
> + const char *trigger;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_OF_MDIO))
> + return -ENOENT;
> +
> + np = of_find_node_by_name(phydev->mdio.dev.of_node, "leds");
> + if (!np)
> + return -ENOENT;
> +
> + for_each_child_of_node(np, child) {
> + u32 val;
> +
> + if (!of_property_read_u32(child, "reg", &val)) {
> + if (val == (u32)led)
> + break;
> + }
> + }
Hi Matthias
This is leaking references to np and child. In the past we have not
cared about this too much, but we are now getting patches adding the
missing releases. So it would be good to fix this.
Andrew
^ permalink raw reply
* Re: [PATCH 2/4] counter: new TI eQEP driver
From: David Lechner @ 2019-08-02 16:17 UTC (permalink / raw)
To: William Breathitt Gray
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm
In-Reply-To: <20190802092727.GB30522@icarus>
On 8/2/19 4:27 AM, William Breathitt Gray wrote:
>> +static const struct counter_ops ti_eqep_counter_ops = {
>> + .count_read = ti_eqep_count_read,
>> + .count_write = ti_eqep_count_write,
>> + .function_get = ti_eqep_function_get,
>> + .function_set = ti_eqep_function_set,
>> +};
> Are you able to provide a signal_read function, or are the Signals not
> exposed to the user by this device? Sometimes quadrature encoder devices
> provide an instanteous read of the signal lines to tell whether they are
> high or low, so I figured I'd ask.
No, it does not look like these signals can be read directly.
>
> You should define an action_get function as well along with Synapses
> corresponding to each Signal. This will allow users to know whether the
> Synapse fires on a rising edge, falling edge, no edge, or both edges.
>
> For example, consider the drivers/counter/104-quad-8.c file. Each count
> register has three associated signal lines: Quadrature A, Quadrature B,
> and Index.
>
> Quadrature A and B are your typical quadrature encoder lines and
> depending on the function mode selected (quadrature x4, pulse-direction,
> etc.) could have a Synapse action mode of none, rising edge, falling
> edge, or both edges; see the quad8_synapse_actions_list array.
>
> In contrast, the Index signal line only has two Synapse action modes:
> rising edge (in the case preset functionality is enabled) or none.
The encoders I have don't use the index or strobe signals, so I was
thinking maybe I should omit those two signals from the driver for the
time being since I don't have a way of testing.
>
> For the TI eQEP driver, there will be four Synapses corresponding to the
> four Signals: QEPA, QEPB, QEPI, and QEPS. See if you are able to
> implement the Synapses and action_get function by using the 104-quad-8.c
> file as a reference. That file does have a lot of extra functionality
> tossed in compared to yours, so if you have trouble groking it, just let
> me know and I'll try to help.
>
^ permalink raw reply
* Re: [PATCH 2/4] counter: new TI eQEP driver
From: David Lechner @ 2019-08-02 16:09 UTC (permalink / raw)
To: William Breathitt Gray
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm, Uwe Kleine-König
In-Reply-To: <20190802092727.GB30522@icarus>
On 8/2/19 4:27 AM, William Breathitt Gray wrote:
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index a7e57516959e..ddcbb8573894 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -499,7 +499,7 @@ config PWM_TIEHRPWM
>>
>> config PWM_TIPWMSS
>> bool
>> - default y if (ARCH_OMAP2PLUS) && (PWM_TIECAP || PWM_TIEHRPWM)
>> + default y if (ARCH_OMAP2PLUS) && (PWM_TIECAP || PWM_TIEHRPWM || TI_EQEP)
>> help
>> PWM Subsystem driver support for AM33xx SOC.
> I was surprised to see this pwm Kconfig change in this patch. Is
> PWM_TIPWMSS required for TI_EQEP to work? If not required, then this
> could be a separate patch; otherwise, put in a mention about why in the
> commit message so that the purpose of this change is clearer.
>
This enables the parent bus for power management. Since this is the second
comment about this, I wonder if it would make sense to move this out of the
PWM subsystem and into drivers/bus/ since it is no longer exclusive to PWM
devices.
^ permalink raw reply
* Re: [RFC 5/9] dt-bindings: arm: amlogic: amlogic, meson-gx-ao-secure: convert to yaml
From: Rob Herring @ 2019-08-02 15:32 UTC (permalink / raw)
To: Neil Armstrong
Cc: linux-amlogic,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree
In-Reply-To: <90dbcb33-74a2-68de-eb1a-ce84040298b8@baylibre.com>
On Fri, Aug 2, 2019 at 8:37 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi Rob,
>
> Thanks for reviews.
>
> On 01/08/2019 15:56, Neil Armstrong wrote:
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > ---
> > .../amlogic/amlogic,meson-gx-ao-secure.txt | 28 -------------
> > .../amlogic/amlogic,meson-gx-ao-secure.yaml | 42 +++++++++++++++++++
> > 2 files changed, 42 insertions(+), 28 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> > create mode 100644 Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt b/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> > deleted file mode 100644
> > index c67d9f48fb91..000000000000
> > --- a/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> > +++ /dev/null
> > @@ -1,28 +0,0 @@
> > -Amlogic Meson Firmware registers Interface
> > -------------------------------------------
> > -
> > -The Meson SoCs have a register bank with status and data shared with the
> > -secure firmware.
> > -
> > -Required properties:
> > - - compatible: For Meson GX SoCs, must be "amlogic,meson-gx-ao-secure", "syscon"
>
> I have a hard time find how to define "syscon" here, if I put syscon in the compatible
> it gets matched on other bindings and I get lot of warnings.
>
> How should I model it ?
You have to add a custom 'select' key that doesn't include 'syscon'.
There should be a few examples in the tree.
Rob
^ permalink raw reply
* Re: [PATCH] drivers/amba: add reset control to primecell probe
From: Rob Herring @ 2019-08-02 15:29 UTC (permalink / raw)
To: Dinh Nguyen
Cc: devicetree, linux-kernel@vger.kernel.org, Frank Rowand, Kees Cook,
Anton Vorontsov, Colin Cross, Tony Luck, Dinh Nguyen
In-Reply-To: <92009928-3df1-1573-7d67-40e79d77c77e@kernel.org>
On Fri, Aug 2, 2019 at 8:42 AM Dinh Nguyen <dinguyen@kernel.org> wrote:
>
>
>
> On 8/2/19 9:37 AM, Rob Herring wrote:
> > On Thu, Aug 1, 2019 at 12:44 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
> >>
> >> From: Dinh Nguyen <dinh.nguyen@intel.com>
> >>
> >> The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
> >> default. Until recently, the DMA controller was brought out of reset by the
> >> bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals that
> >> are not used are held in reset and are left to Linux to bring them out of
> >> reset.
> >
> > You can fix this in the kernel, but any versions before this change
> > will remain broken. IMO, the u-boot change should be reverted because
> > it is breaking an ABI (though not a good one).
> >
>
> Right, there exists in U-Boot to support legacy platforms before this
> recent change. This would be for future versions.
>
> >> Add a mechanism for getting the reset property and de-assert the primecell
> >> module from reset if found. This is a not a hard fail if the reset property
> >> is not present in the device tree node, so the driver will continue to probe.
> >
> > I think this belongs in the AMBA bus code, not the DT code, as that is
> > where we already have clock control code for similar reasons.
> >
>
> Ok.
>
> >>
> >> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> >> ---
> >> drivers/of/platform.c | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> >> index 7801e25e6895..d8945705313d 100644
> >> --- a/drivers/of/platform.c
> >> +++ b/drivers/of/platform.c
> >> @@ -21,6 +21,7 @@
> >> #include <linux/of_irq.h>
> >> #include <linux/of_platform.h>
> >> #include <linux/platform_device.h>
> >> +#include <linux/reset.h>
> >>
> >> const struct of_device_id of_default_bus_match_table[] = {
> >> { .compatible = "simple-bus", },
> >> @@ -229,6 +230,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> >> struct amba_device *dev;
> >> const void *prop;
> >> int i, ret;
> >> + struct reset_control *rstc;
> >>
> >> pr_debug("Creating amba device %pOF\n", node);
> >>
> >> @@ -270,6 +272,18 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> >> goto err_free;
> >> }
> >>
> >> + /*
> >> + * reset control of the primecell block is optional
> >> + * and will not fail if the reset property is not found.
> >> + */
> >> + rstc = of_reset_control_get_exclusive(node, "dma");
> >
> > 'dma' doesn't sound very generic.
> >
>
> how about 'primecell' ?
It should be based on what is in the TRMs. Unlike pclk, there doesn't
appear to be a standard name or number of resets:
pl011: PRESETn and nUARTRST
pl330: ARESETn
Can't you just retrieve all of them and deassert them all and ignore the name?
Also, you might need to use the shared variant as the core code has to
work for either dedicated or shared resets.
Rob
^ permalink raw reply
* Re: [PATCH v4 4/4] edac: Add support for Amazon's Annapurna Labs L2 EDAC
From: James Morse @ 2019-08-02 15:11 UTC (permalink / raw)
To: Hanna Hawa
Cc: robh+dt, mark.rutland, bp, mchehab, davem, gregkh, linus.walleij,
Jonathan.Cameron, nicolas.ferre, paulmck, dwmw, benh, ronenk,
talel, jonnyc, hanochu, devicetree, linux-kernel, linux-edac
In-Reply-To: <20190801130956.26388-5-hhhawa@amazon.com>
Hi Hanna,
On 01/08/2019 14:09, Hanna Hawa wrote:
> Adds support for Amazon's Annapurna Labs L2 EDAC driver to detect and
> report L2 errors.
> diff --git a/drivers/edac/al_l2_edac.c b/drivers/edac/al_l2_edac.c
> new file mode 100644
> index 000000000000..6c6d37cf82ab
> --- /dev/null
> +++ b/drivers/edac/al_l2_edac.c
> @@ -0,0 +1,189 @@
> +#include <asm/sysreg.h>
> +#include <linux/bitfield.h>
#include <linux/cpumask.h> ?
> +#include <linux/of.h>
> +#include <linux/smp.h>
[...]
> +static void al_l2_edac_l2merrsr(void *arg)
> +{
> + struct edac_device_ctl_info *edac_dev = arg;
> + int cpu, i;
> + u32 ramid, repeat, other, fatal;
> + u64 val = read_sysreg_s(ARM_CA57_L2MERRSR_EL1);
> + char msg[AL_L2_EDAC_MSG_MAX];
> + int space, count;
> + char *p;
> +
> + if (!(FIELD_GET(ARM_CA57_L2MERRSR_VALID, val)))
> + return;
> +
> + write_sysreg_s(0, ARM_CA57_L2MERRSR_EL1);
> +
> + cpu = smp_processor_id();
> + ramid = FIELD_GET(ARM_CA57_L2MERRSR_RAMID, val);
> + repeat = FIELD_GET(ARM_CA57_L2MERRSR_REPEAT, val);
> + other = FIELD_GET(ARM_CA57_L2MERRSR_OTHER, val);
> + fatal = FIELD_GET(ARM_CA57_L2MERRSR_FATAL, val);
> +
> + space = sizeof(msg);
> + p = msg;
> + count = scnprintf(p, space, "CPU%d L2 %serror detected", cpu,
> + (fatal) ? "Fatal " : "");
> + p += count;
> + space -= count;
> +
> + switch (ramid) {
> + case ARM_CA57_L2_TAG_RAM:
> + count = scnprintf(p, space, " RAMID='L2 Tag RAM'");
> + break;
> + case ARM_CA57_L2_DATA_RAM:
> + count = scnprintf(p, space, " RAMID='L2 Data RAM'");
> + break;
> + case ARM_CA57_L2_SNOOP_RAM:
> + count = scnprintf(p, space, " RAMID='L2 Snoop RAM'");
Nit: The TRMs both call this 'L2 Snoop Tag RAM'. Could we include 'tag' in the
description. 'tag' implies its some kind of metadata, so an uncorrected error here affect
a now unknown location, its more series than a 'data RAM' error. v8.2 would term this kind
of error 'uncontained'.
> + break;
> + case ARM_CA57_L2_DIRTY_RAM:
> + count = scnprintf(p, space, " RAMID='L2 Dirty RAM'");
> + break;
> + case ARM_CA57_L2_INC_PF_RAM:
> + count = scnprintf(p, space, " RAMID='L2 internal metadat'");
Nit: metadata
> + break;
> + default:
> + count = scnprintf(p, space, " RAMID='unknown'");
> + break;
> + }
> +
> + p += count;
> + space -= count;
> +
> + count = scnprintf(p, space,
> + " repeat=%d, other=%d (L2MERRSR_EL1=0x%llx)",
> + repeat, other, val);
> +
> + for (i = 0; i < repeat; i++) {
> + if (fatal)
> + edac_device_handle_ue(edac_dev, 0, 0, msg);
> + else
> + edac_device_handle_ce(edac_dev, 0, 0, msg);
> + }
> +}
[...]
> +static int al_l2_edac_probe(struct platform_device *pdev)
> +{
> + struct edac_device_ctl_info *edac_dev;
> + struct al_l2_edac *al_l2;
> + struct device *dev = &pdev->dev;
> + int ret, i;
> +
> + edac_dev = edac_device_alloc_ctl_info(sizeof(*al_l2),
> + (char *)dev_name(dev), 1, "L", 1,
> + 2, NULL, 0,
> + edac_device_alloc_index());
> + if (IS_ERR_OR_NULL(edac_dev))
> + return -ENOMEM;
> +
> + al_l2 = edac_dev->pvt_info;
> + edac_dev->edac_check = al_l2_edac_check;
> + edac_dev->dev = dev;
> + edac_dev->mod_name = DRV_NAME;
> + edac_dev->dev_name = dev_name(dev);
> + edac_dev->ctl_name = "L2 cache";
> + platform_set_drvdata(pdev, edac_dev);
> + for_each_online_cpu(i) {
for_each_possible_cpu()?
If you boot with maxcpus= the driver's behaviour changes.
But you are only parsing information from the DT, so you don't really need the CPUs to be
online.
> + struct device_node *cpu;
> + struct device_node *cpu_cache, *l2_cache;
> +
> + cpu = of_get_cpu_node(i, NULL);
(of_get_cpu_node() can return NULL, but I don't think it can ever happen like this)
> + cpu_cache = of_find_next_cache_node(cpu);
> + l2_cache = of_parse_phandle(dev->of_node, "l2-cache", 0);
> +
> + if (cpu_cache == l2_cache)
> + cpumask_set_cpu(i, &al_l2->cluster_cpus);
You need to of_node_put() these device_node pointers once you're done with them.
> + }
> +
> + if (cpumask_empty(&al_l2->cluster_cpus)) {
> + dev_err(dev, "CPU mask is empty for this L2 cache\n");
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = edac_device_add_device(edac_dev);
> + if (ret) {
> + dev_err(dev, "Failed to add L2 edac device\n");
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + edac_device_free_ctl_info(edac_dev);
> +
> + return ret;
> +}
With the of_node_put()ing:
Reviewed-by: James Morse <james.morse@arm.com>
Thanks,
James
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2 3/3] ASoC: TDA7802: Add turn-on diagnostic routine
From: Thomas Preston @ 2019-08-02 14:51 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, devicetree, alsa-devel, Charles Keepax,
Kuninori Morimoto, Kirill Marinushkin, Liam Girdwood,
Marco Felsch, Annaliese McDermond, Takashi Iwai, Paul Cercueil,
Vinod Koul, Rob Herring, Srinivas Kandagatla, Jerome Brunet,
linux-kernel, Cheng-Yi Chiang
In-Reply-To: <20190802111036.GB5387@sirena.org.uk>
On 02/08/2019 12:10, Mark Brown wrote:
> On Fri, Aug 02, 2019 at 09:32:17AM +0100, Thomas Preston wrote:
>> On 02/08/2019 00:42, Mark Brown wrote:
>
>>> Yes, that's definitely doable - we've got some other drivers with
>>> similar things like calibration triggers exposed that way.
>
>> One problem with using a kcontrol as a trigger for the turn-on diagnostic
>> is that the diagnostic routine has a "return value".
>
> You can use a read only control for the readback, or just have it be
> triggered by overwriting the readback value. You can cache the result.
>
Keeping the trigger and result together like that would be better I think,
although the routine isn't supposed to run mid way through playback. If
we're mid playback the debugfs routine has to turn off AMP_ON, take the
device back to a known state, run diagnostics, then restore. Which causes
a gap in the audible sound.
>> Hm, maybe a better idea is to have the turn on diagnostic only run on
>> device probe (as its name suggests!), and print something to dmesg:
>
>> modprobe tda7802 turn_on_diagnostic=1
>
>> tda7802-codec i2c-TDA7802:00: Turn on diagnostic 04 04 04 04
>
>> Kirill Marinushkin mentioned this in the first review [0], it just didn't
>> really sink in until now!
>
> You could do that too, yeah. Depends on what this is diagnosing and if
> that'd be useful.
>
The diagnostic status bits describe situations such as:
- open load (no speaker connected)
- short to GND
- short to VCC
- etc
The intention is to test if all the speakers are connected. So, one might
have a self test which runs the diagnostic and verifies it outputs:
00 00 00 00
For example, on my test rig there is only one speaker connected. So it
reads:
04 04 00 04
Where the second bit is "open load". So this would fail the test.
So in the kcontrol case the test would be something like:
amixer sset "AMP1 turn on diagnostic" on
amixer sget "AMP1 diagnostic"
And the module parameter case:
rmmod tda7802
modprobe tda7802 turn_on_diagnostic=1
dmesg | grep "Turn on diagnostic 04 04 04 04"
rmmod tda7802
modprobe tda7802
I think the module parameter method is more appropriate for a
"Turn-on diagnostic", even though I don't really like grepping dmesg
for the result. I'll go ahead and implement that unless anyone has a
particular preference for the kcontrol-trigger.
Thanks
^ permalink raw reply
* [PATCH] drm/stm: ltdc: add pinctrl for DPI encoder mode
From: Yannick Fertré @ 2019-08-02 14:47 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland,
linux-stm32, linux-arm-kernel, devicetree, linux-kernel,
Benjamin Gaignard, Yannick Fertre, Philippe Cornu,
Fabrice Gasnier
The implementation of functions encoder_enable and encoder_disable
make possible to control the pinctrl according to the encoder type.
The pinctrl must be activated only if the encoder type is DPI.
This helps to move the DPI-related pinctrl configuration from
all the panel or bridge to the LTDC dt node.
Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 3ab4fbf..1c4fde0 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_graph.h>
+#include <linux/pinctrl/consumer.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
@@ -1040,6 +1041,36 @@ static const struct drm_encoder_funcs ltdc_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
+static void ltdc_encoder_disable(struct drm_encoder *encoder)
+{
+ struct drm_device *ddev = encoder->dev;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ /* Set to sleep state the pinctrl whatever type of encoder */
+ pinctrl_pm_select_sleep_state(ddev->dev);
+}
+
+static void ltdc_encoder_enable(struct drm_encoder *encoder)
+{
+ struct drm_device *ddev = encoder->dev;
+
+ DRM_DEBUG_DRIVER("\n");
+
+ /*
+ * Set to default state the pinctrl only with DPI type.
+ * Others types like DSI, don't need pinctrl due to
+ * internal bridge (the signals do not come out of the chipset).
+ */
+ if (encoder->encoder_type == DRM_MODE_ENCODER_DPI)
+ pinctrl_pm_select_default_state(ddev->dev);
+}
+
+static const struct drm_encoder_helper_funcs ltdc_encoder_helper_funcs = {
+ .disable = ltdc_encoder_disable,
+ .enable = ltdc_encoder_enable,
+};
+
static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge)
{
struct drm_encoder *encoder;
@@ -1055,6 +1086,8 @@ static int ltdc_encoder_init(struct drm_device *ddev, struct drm_bridge *bridge)
drm_encoder_init(ddev, encoder, <dc_encoder_funcs,
DRM_MODE_ENCODER_DPI, NULL);
+ drm_encoder_helper_add(encoder, <dc_encoder_helper_funcs);
+
ret = drm_bridge_attach(encoder, bridge, NULL);
if (ret) {
drm_encoder_cleanup(encoder);
@@ -1280,6 +1313,8 @@ int ltdc_load(struct drm_device *ddev)
clk_disable_unprepare(ldev->pixel_clk);
+ pinctrl_pm_select_sleep_state(ddev->dev);
+
pm_runtime_enable(ddev->dev);
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 1/3] dt-bindings: regulator: Document regulators coupling of NVIDIA Tegra20/30 SoCs
From: Dmitry Osipenko @ 2019-08-02 14:47 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Peter De Schrijver, Jonathan Hunter,
Mark Brown
Cc: devicetree, linux-tegra, linux-kernel
In-Reply-To: <20190725151832.9802-2-digetx@gmail.com>
25.07.2019 18:18, Dmitry Osipenko пишет:
> There is voltage coupling between three regulators on Tegra20 boards and
> between two on Tegra30. The voltage coupling is a SoC-level feature and
> thus it is mandatory and common for all of the Tegra boards.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> .../nvidia,tegra-regulators-coupling.txt | 65 +++++++++++++++++++
> 1 file changed, 65 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/nvidia,tegra-regulators-coupling.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/nvidia,tegra-regulators-coupling.txt b/Documentation/devicetree/bindings/regulator/nvidia,tegra-regulators-coupling.txt
> new file mode 100644
> index 000000000000..4bf2dbf7c6cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/nvidia,tegra-regulators-coupling.txt
> @@ -0,0 +1,65 @@
> +NVIDIA Tegra Regulators Coupling
> +================================
> +
> +NVIDIA Tegra SoC's have a mandatory voltage-coupling between regulators.
> +Thus on Tegra20 there are 3 coupled regulators and on NVIDIA Tegra30
> +there are 2.
> +
> +Tegra20 voltage coupling
> +------------------------
> +
> +On Tegra20 SoC's there are 3 coupled regulators: CORE, RTC and CPU.
> +The CORE and RTC voltages shall be in a range of 170mV from each other
> +and they both shall be higher than the CPU voltage by at least 120mV.
> +
> +Tegra30 voltage coupling
> +------------------------
> +
> +On Tegra30 SoC's there are 2 coupled regulators: CORE and CPU. The CORE
> +and CPU voltages shall be in a range of 300mV from each other and CORE
> +voltage shall be higher than the CPU by N mV, where N depends on the CPU
> +voltage.
> +
> +Required properties:
> +- nvidia,tegra-core-regulator: Boolean property that designates regulator
> + as the "Core domain" voltage regulator.
> +- nvidia,tegra-rtc-regulator: Boolean property that designates regulator
> + as the "RTC domain" voltage regulator.
> +- nvidia,tegra-cpu-regulator: Boolean property that designates regulator
> + as the "CPU domain" voltage regulator.
> +
> +Example:
> +
> + pmic {
> + regulators {
> + core_vdd_reg: core {
> + regulator-name = "vdd_core";
> + regulator-min-microvolt = <950000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-coupled-with = <&rtc_vdd_reg &cpu_vdd_reg>;
> + regulator-coupled-max-spread = <170000 550000>;
> +
> + nvidia,tegra-core-regulator;
> + };
> +
> + rtc_vdd_reg: rtc {
> + regulator-name = "vdd_rtc";
> + regulator-min-microvolt = <950000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-coupled-with = <&core_vdd_reg &cpu_vdd_reg>;
> + regulator-coupled-max-spread = <170000 550000>;
> +
> + nvidia,tegra-rtc-regulator;
> + };
> +
> + cpu_vdd_reg: cpu {
> + regulator-name = "vdd_cpu";
> + regulator-min-microvolt = <750000>;
> + regulator-max-microvolt = <1125000>;
> + regulator-coupled-with = <&core_vdd_reg &rtc_vdd_reg>;
> + regulator-coupled-max-spread = <550000 550000>;
> +
> + nvidia,tegra-cpu-regulator;
> + };
> + };
> + };
>
Hello Rob,
Are you okay with this patch? We just need to mark the SoC voltage
regulators appropriately and regulators themselves vary from board to
board, hence this binding is not something that could be done using
YAML, I guess.
^ permalink raw reply
* Re: [PATCH] scripts/dtc: dtx_diff - add color output support
From: Frank Rowand @ 2019-08-02 14:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Geert Uytterhoeven, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CAMuHMdW5XmG-320uhAsqxC-oCq7POtZKOOE1V485nB5K1vzh8g@mail.gmail.com>
Hi Geert,
On 8/2/19 1:44 AM, Geert Uytterhoeven wrote:
> Hi Frank,
>
> On Thu, Aug 1, 2019 at 9:55 PM Frank Rowand <frowand.list@gmail.com> wrote:
>> On 8/1/19 5:13 AM, Geert Uytterhoeven wrote:
>>> On Wed, Jul 31, 2019 at 10:30 PM Frank Rowand <frowand.list@gmail.com> wrote:
>>>> On 7/31/19 5:37 AM, Geert Uytterhoeven wrote:
>>>>> Add new -c/--color options, to enhance the diff output with color, and
>>>>> improve the user's experience.
>>>>>
>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>>> ---
>>>>> scripts/dtc/dtx_diff | 10 +++++++++-
>>>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff
>>>>> index e9ad7834a22d9459..4e2c8617f69a333e 100755
>>>>> --- a/scripts/dtc/dtx_diff
>>>>> +++ b/scripts/dtc/dtx_diff
>>>>> @@ -20,6 +20,8 @@ Usage:
>>>>>
>>>>>
>>>>> --annotate synonym for -T
>>>>> + --color synonym for -c
>>>>> + -c enable colored output
>>>>> -f print full dts in diff (--unified=99999)
>>>>> -h synonym for --help
>>>>> -help synonym for --help
>>>
>>>> I like the idea, but...
>>>>
>>>> I have various linux distro releases across my many systems, but only one is
>>>> new enough to have the diff command that supports --color.
>>>
>>> Seems to have been added in diffutils release 3.4 (2016-08-08).
>>> I almost can't believe it was that recent, but then I remembered using a
>>> wrapper before (colordiff; other wrappers may exist).
>>>
>>>> Can you enhance this patch to test whether --color is supported? Maybe
>>>> something like (untested):
>>>>
>>>> -c | --color )
>>>> if `diff --color <(echo a) <(echo a) 2>/dev/null` ; then
>>>> diff_color="--color=always"
>>>> fi
>>>> shift
>>>> ;;
>>>>
>>>> Then add some text to the usage for -c and --color saying that they will
>>>> be silently ignored if diff does not support --color.
>>>>
>>>> I first wrote up a suggested version that printed an error message and
>>>> exited, but I think silently ignoring is more robust, even though it
>>>> may be more confusing to someone who is wondering why --color does not
>>>> work.
>>>
>>> Given this is an optional feature, to be enabled explicitly by the user,
>>> I'm not so fond of going through hoops to auto-detect the availability.
>>>
>>> So what about just documenting this in the help text instead?
>>>
>>> - -c enable colored output
>>> + -c enable colored output (requires diff with --color support)
>>
>> ----- thought 1 -----
>>
>> My first thought was:
>>
>> If the hoops were complex and ugly, I might agree with you. But since it is
>> a simple one line "if" (two lines including "fi") I prefer the check.
>>
>> The help text update looks good to me, along with the check.
>
> OK.
>
>> ----- thought 2 -----
>>
>> Then I reconsidered, and thought "well, Geert has a good idea". So I
>> decided to see how useful the diff error message would be. The message is:
>>
>> $ scripts/dtc/dtx_diff -c a.dts b.dts
>> diff: unrecognized option '--color=always'
>> diff: Try 'diff --help' for more information.
>> $
>> Possible hints to resolve the above error:
>> (hints might not fix the problem)
>>
>> No hints available.
>>
>> It is interesting that the shell prompt arrives before the full set of
>> messages from the script, but that is not my issue. My issue is that
>
> That is due to the output coming from the two "<(compile_to_dts ...)"
> sub-processes, not from the diff sub-process.
Thanks for figuring that out (or knowing it). I figured it probably
was from some sort of separate process issue, but had not chased it
down. Now I have a reminder in the back of my brain to be aware of
messages coming from the "<()" construct.
>
>> when the diff fails, the script tries to find suggestions to solve
>> the problem. (The suggestions exist to catch some likely problems
>> with the shell variable "ARCH".)
>
> Interesting. I didn't know about the hints (never saw them), and had to
> try hard to trigger them (I usually do DTB comparisons only).
> But I succeeded ;-)
> With a small tweak as my diff does support --color:
>
> $ scripts/dtc/dtx_diff -c
> arch/arm64/boot/dts/renesas/r8a7799*{ebisu,draak}.dts
> diff: unrecognized option '--olor=always'
> diff: Try 'diff --help' for more information.
> $
> Possible hints to resolve the above error:
> (hints might not fix the problem)
>
> shell variable $ARCH not set
>
> architecture arm64 is in file path,
> but does not match shell variable $ARCH
> >>$ARCH<< is: >><<
>
>
> Possible hints to resolve the above error:
> (hints might not fix the problem)
>
> shell variable $ARCH not set
>
> architecture arm64 is in file path,
> but does not match shell variable $ARCH
> >>$ARCH<< is: >><<
>
>> Unfortunately in the case of the "--color" problem, the useful warning
>> from diff becomes less visible because of the early prompt and the
>> not so helpful messages about hints.
>
> Yeah, they are not so helpful.
> In fact $ARCH is indeed not set, but that's not an issue at all.
>
>> If the hints related messages were not present, then I was ready to
>> accept that the diff warning was sufficient. But since the hints
>> messages are present, and hiding them would be more complex than
>> the original check that I suggested for whether diff supports
>> the --color option, I am back to my first thought: I prefer the
>> check whether diff supports "--color" is done when dtx_diff detects
>> the "--color" option.
>
> OK, you managed to convince me. Will fix.
Thanks.
>
> Gr{oetje,eeting}s,
>
> Geert
>
^ permalink raw reply
* Re: [PATCH] drivers/amba: add reset control to primecell probe
From: Dinh Nguyen @ 2019-08-02 14:42 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel@vger.kernel.org, Frank Rowand, Kees Cook,
Anton Vorontsov, Colin Cross, Tony Luck, Dinh Nguyen
In-Reply-To: <CAL_Jsq+PRKGwdozr3VECpk2ugrOuWd4CYnRSR7ChyPOKgheYkw@mail.gmail.com>
On 8/2/19 9:37 AM, Rob Herring wrote:
> On Thu, Aug 1, 2019 at 12:44 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>>
>> From: Dinh Nguyen <dinh.nguyen@intel.com>
>>
>> The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
>> default. Until recently, the DMA controller was brought out of reset by the
>> bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals that
>> are not used are held in reset and are left to Linux to bring them out of
>> reset.
>
> You can fix this in the kernel, but any versions before this change
> will remain broken. IMO, the u-boot change should be reverted because
> it is breaking an ABI (though not a good one).
>
Right, there exists in U-Boot to support legacy platforms before this
recent change. This would be for future versions.
>> Add a mechanism for getting the reset property and de-assert the primecell
>> module from reset if found. This is a not a hard fail if the reset property
>> is not present in the device tree node, so the driver will continue to probe.
>
> I think this belongs in the AMBA bus code, not the DT code, as that is
> where we already have clock control code for similar reasons.
>
Ok.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>> ---
>> drivers/of/platform.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
>> index 7801e25e6895..d8945705313d 100644
>> --- a/drivers/of/platform.c
>> +++ b/drivers/of/platform.c
>> @@ -21,6 +21,7 @@
>> #include <linux/of_irq.h>
>> #include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> +#include <linux/reset.h>
>>
>> const struct of_device_id of_default_bus_match_table[] = {
>> { .compatible = "simple-bus", },
>> @@ -229,6 +230,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>> struct amba_device *dev;
>> const void *prop;
>> int i, ret;
>> + struct reset_control *rstc;
>>
>> pr_debug("Creating amba device %pOF\n", node);
>>
>> @@ -270,6 +272,18 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>> goto err_free;
>> }
>>
>> + /*
>> + * reset control of the primecell block is optional
>> + * and will not fail if the reset property is not found.
>> + */
>> + rstc = of_reset_control_get_exclusive(node, "dma");
>
> 'dma' doesn't sound very generic.
>
how about 'primecell' ?
Thanks for the review!
Dinh
^ permalink raw reply
* Re: [PATCH v2 3/3] soc/tegra: regulators: Add regulators coupler for Tegra30
From: Dmitry Osipenko @ 2019-08-02 14:39 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Rob Herring, Thierry Reding, Jonathan Hunter, Mark Brown,
devicetree, linux-tegra, linux-kernel
In-Reply-To: <20190802140512.GD3883@pdeschrijver-desktop.Nvidia.com>
02.08.2019 17:05, Peter De Schrijver пишет:
> On Thu, Jul 25, 2019 at 06:18:32PM +0300, Dmitry Osipenko wrote:
>> Add regulators coupler for Tegra30 SoCs that performs voltage balancing
>> of a coupled regulators and thus provides voltage scaling functionality.
>>
>> There are 2 coupled regulators on all Tegra30 SoCs: CORE and CPU. The
>> coupled regulator voltages shall be in a range of 300mV from each other
>> and CORE voltage shall be higher than the CPU by N mV, where N depends
>> on the CPU voltage.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> drivers/soc/tegra/Kconfig | 4 +
>> drivers/soc/tegra/Makefile | 1 +
>> drivers/soc/tegra/regulators-tegra30.c | 316 +++++++++++++++++++++++++
>> 3 files changed, 321 insertions(+)
>> create mode 100644 drivers/soc/tegra/regulators-tegra30.c
>>
> ...
>
>> +
>> +static int tegra30_core_cpu_limit(int cpu_uV)
>> +{
>> + if (cpu_uV < 800000)
>> + return 950000;
>> +
>> + if (cpu_uV < 900000)
>> + return 1000000;
>> +
>> + if (cpu_uV < 1000000)
>> + return 1100000;
>> +
>> + if (cpu_uV < 1100000)
>> + return 1200000;
>> +
>> + if (cpu_uV < 1250000) {
>> + switch (tegra_sku_info.cpu_speedo_id) {
>> + case 0 ... 1:
> Aren't we supposed to add /* fall through */ here now?
There is no compiler warning if there is nothing in-between of the
case-switches, so annotation isn't really necessary here. Of course it
is possible to add an explicit annotation just to make clear the
fall-through intention.
>> + case 4:
>> + case 7 ... 8:
>> + return 1200000;
>> +
>> + default:
>> + return 1300000;
>> + }
>> + }
>> +
>
> Other than that, this looks ok to me.
Awesome, thank you very much! Explicit ACK will be appreciated as well.
^ permalink raw reply
* Re: [RFC 5/9] dt-bindings: arm: amlogic: amlogic, meson-gx-ao-secure: convert to yaml
From: Neil Armstrong @ 2019-08-02 14:37 UTC (permalink / raw)
To: robh+dt; +Cc: linux-amlogic, linux-arm-kernel, devicetree
In-Reply-To: <20190801135644.12843-6-narmstrong@baylibre.com>
Hi Rob,
Thanks for reviews.
On 01/08/2019 15:56, Neil Armstrong wrote:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../amlogic/amlogic,meson-gx-ao-secure.txt | 28 -------------
> .../amlogic/amlogic,meson-gx-ao-secure.yaml | 42 +++++++++++++++++++
> 2 files changed, 42 insertions(+), 28 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> create mode 100644 Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt b/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> deleted file mode 100644
> index c67d9f48fb91..000000000000
> --- a/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -Amlogic Meson Firmware registers Interface
> -------------------------------------------
> -
> -The Meson SoCs have a register bank with status and data shared with the
> -secure firmware.
> -
> -Required properties:
> - - compatible: For Meson GX SoCs, must be "amlogic,meson-gx-ao-secure", "syscon"
I have a hard time find how to define "syscon" here, if I put syscon in the compatible
it gets matched on other bindings and I get lot of warnings.
How should I model it ?
Thanks,
Neil
> -
> -Properties should indentify components of this register interface :
> -
> -Meson GX SoC Information
> -------------------------
> -A firmware register encodes the SoC type, package and revision information on
> -the Meson GX SoCs.
> -If present, the following property should be added :
> -
> -Optional properties:
> - - amlogic,has-chip-id: If present, the interface gives the current SoC version.
> -
> -Example
> --------
> -
> -ao-secure@140 {
> - compatible = "amlogic,meson-gx-ao-secure", "syscon";
> - reg = <0x0 0x140 0x0 0x140>;
> - amlogic,has-chip-id;
> -};
> diff --git a/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.yaml b/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.yaml
> new file mode 100644
> index 000000000000..cf79287498f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/amlogic/amlogic,meson-gx-ao-secure.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 BayLibre, SAS
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/arm/amlogic/amlogic,meson-gx-ao-secure.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Amlogic Meson Firmware registers Interface
> +
> +maintainers:
> + - Neil Armstrong <narmstrong@baylibre.com>
> +
> +description: |
> + The Meson SoCs have a register bank with status and data shared with the
> + secure firmware.
> +
> +properties:
> + compatible:
> + enum:
> + - amlogic,meson-gx-ao-secure
> +
> + reg:
> + maxItems: 1
> +
> + amlogic,has-chip-id:
> + description: |
> + A firmware register encodes the SoC type, package and revision
> + information on the Meson GX SoCs. If present, the interface gives
> + the current SoC version.
> + type: boolean
> +
> +required:
> + - compatible
> + - reg
> +
> +examples:
> + - |
> + ao-secure@140 {
> + compatible = "amlogic,meson-gx-ao-secure", "syscon";
> + reg = <0x140 0x140>;
> + amlogic,has-chip-id;
> + };
>
^ permalink raw reply
* Re: [PATCH] drivers/amba: add reset control to primecell probe
From: Rob Herring @ 2019-08-02 14:37 UTC (permalink / raw)
To: Dinh Nguyen
Cc: devicetree, linux-kernel@vger.kernel.org, Frank Rowand, Kees Cook,
Anton Vorontsov, Colin Cross, Tony Luck, Dinh Nguyen
In-Reply-To: <20190801184346.7015-1-dinguyen@kernel.org>
On Thu, Aug 1, 2019 at 12:44 PM Dinh Nguyen <dinguyen@kernel.org> wrote:
>
> From: Dinh Nguyen <dinh.nguyen@intel.com>
>
> The primecell controller on some SoCs, i.e. SoCFPGA, is held in reset by
> default. Until recently, the DMA controller was brought out of reset by the
> bootloader(i.e. U-Boot). But a recent change in U-Boot, the peripherals that
> are not used are held in reset and are left to Linux to bring them out of
> reset.
You can fix this in the kernel, but any versions before this change
will remain broken. IMO, the u-boot change should be reverted because
it is breaking an ABI (though not a good one).
> Add a mechanism for getting the reset property and de-assert the primecell
> module from reset if found. This is a not a hard fail if the reset property
> is not present in the device tree node, so the driver will continue to probe.
I think this belongs in the AMBA bus code, not the DT code, as that is
where we already have clock control code for similar reasons.
>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
> drivers/of/platform.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 7801e25e6895..d8945705313d 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -21,6 +21,7 @@
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/reset.h>
>
> const struct of_device_id of_default_bus_match_table[] = {
> { .compatible = "simple-bus", },
> @@ -229,6 +230,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> struct amba_device *dev;
> const void *prop;
> int i, ret;
> + struct reset_control *rstc;
>
> pr_debug("Creating amba device %pOF\n", node);
>
> @@ -270,6 +272,18 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
> goto err_free;
> }
>
> + /*
> + * reset control of the primecell block is optional
> + * and will not fail if the reset property is not found.
> + */
> + rstc = of_reset_control_get_exclusive(node, "dma");
'dma' doesn't sound very generic.
> + if (!IS_ERR(rstc)) {
> + reset_control_deassert(rstc);
> + reset_control_put(rstc);
> + } else {
> + pr_debug("amba: reset control not found\n");
> + }
> +
> ret = amba_device_add(dev, &iomem_resource);
> if (ret) {
> pr_err("amba_device_add() failed (%d) for %pOF\n",
> --
> 2.20.0
>
^ permalink raw reply
* [PATCH] ARM: dts: stm32: move ltdc pinctrl on stm32mp157a dk1 board
From: Yannick Fertré @ 2019-08-02 14:08 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland,
linux-stm32, linux-arm-kernel, devicetree, linux-kernel,
Benjamin Gaignard, Yannick Fertre, Philippe Cornu,
Fabrice Gasnier
The ltdc pinctrl must be in the display controller node and
not in the peripheral node (hdmi bridge).
Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
arch/arm/boot/dts/stm32mp157a-dk1.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/stm32mp157a-dk1.dts b/arch/arm/boot/dts/stm32mp157a-dk1.dts
index f3f0e37..1285cfc 100644
--- a/arch/arm/boot/dts/stm32mp157a-dk1.dts
+++ b/arch/arm/boot/dts/stm32mp157a-dk1.dts
@@ -99,9 +99,6 @@
reset-gpios = <&gpioa 10 GPIO_ACTIVE_LOW>;
interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
interrupt-parent = <&gpiog>;
- pinctrl-names = "default", "sleep";
- pinctrl-0 = <<dc_pins_a>;
- pinctrl-1 = <<dc_pins_sleep_a>;
status = "okay";
ports {
@@ -276,6 +273,9 @@
};
<dc {
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <<dc_pins_a>;
+ pinctrl-1 = <<dc_pins_sleep_a>;
status = "okay";
port {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 3/3] soc/tegra: regulators: Add regulators coupler for Tegra30
From: Peter De Schrijver @ 2019-08-02 14:05 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Rob Herring, Thierry Reding, Jonathan Hunter, Mark Brown,
devicetree, linux-tegra, linux-kernel
In-Reply-To: <20190725151832.9802-4-digetx@gmail.com>
On Thu, Jul 25, 2019 at 06:18:32PM +0300, Dmitry Osipenko wrote:
> Add regulators coupler for Tegra30 SoCs that performs voltage balancing
> of a coupled regulators and thus provides voltage scaling functionality.
>
> There are 2 coupled regulators on all Tegra30 SoCs: CORE and CPU. The
> coupled regulator voltages shall be in a range of 300mV from each other
> and CORE voltage shall be higher than the CPU by N mV, where N depends
> on the CPU voltage.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/soc/tegra/Kconfig | 4 +
> drivers/soc/tegra/Makefile | 1 +
> drivers/soc/tegra/regulators-tegra30.c | 316 +++++++++++++++++++++++++
> 3 files changed, 321 insertions(+)
> create mode 100644 drivers/soc/tegra/regulators-tegra30.c
>
...
> +
> +static int tegra30_core_cpu_limit(int cpu_uV)
> +{
> + if (cpu_uV < 800000)
> + return 950000;
> +
> + if (cpu_uV < 900000)
> + return 1000000;
> +
> + if (cpu_uV < 1000000)
> + return 1100000;
> +
> + if (cpu_uV < 1100000)
> + return 1200000;
> +
> + if (cpu_uV < 1250000) {
> + switch (tegra_sku_info.cpu_speedo_id) {
> + case 0 ... 1:
Aren't we supposed to add /* fall through */ here now?
> + case 4:
> + case 7 ... 8:
> + return 1200000;
> +
> + default:
> + return 1300000;
> + }
> + }
> +
Other than that, this looks ok to me.
Peter.
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: counter: new bindings for TI eQEP
From: William Breathitt Gray @ 2019-08-02 13:58 UTC (permalink / raw)
To: David Lechner
Cc: Rob Herring, open list:IIO SUBSYSTEM AND DRIVERS, linux-omap,
devicetree, Mark Rutland, Jonathan Cameron, Benoît Cousson,
Tony Lindgren, Thierry Reding, linux-kernel@vger.kernel.org,
Linux PWM List
In-Reply-To: <CAL_JsqLA+m5vKZQ1WwWusnVHwX+nnuApiwKXUnmP6ti-PvMZ-g@mail.gmail.com>
On Fri, Aug 02, 2019 at 07:34:42AM -0600, Rob Herring wrote:
> On Fri, Aug 2, 2019 at 1:25 AM William Breathitt Gray
> <vilhelm.gray@gmail.com> wrote:
> >
> > On Sat, Jul 27, 2019 at 08:48:36PM +0100, Jonathan Cameron wrote:
> > > On Mon, 22 Jul 2019 10:45:35 -0500
> > > David Lechner <david@lechnology.com> wrote:
> > >
> > > > This documents device tree binding for the Texas Instruments Enhanced
> > > > Quadrature Encoder Pulse (eQEP) Module found in various TI SoCs.
> > > >
> > > > Signed-off-by: David Lechner <david@lechnology.com>
> > >
> > > Up to William given it is a counter binding, (unless Rob overrules)
> > > but new bindings are generally preferred as yaml.
> > >
> > > Content looks fine to me.
> > >
> > > Thanks,
> > >
> > > Jonathan
> >
> > Rob,
> >
> > Would you prefer these bindings as yaml, or shall I accept them as they
> > are now?
>
> Still up to you at this point, but I certainly prefer them to be DT schema.
>
> Rob
I think YAML would be a good idea, it's simple and expressive enough
while still being human-readable, and if new bindings in other
subsystems are switching to yaml then we may as well.
David, resubmit this as yaml in version 2 and I'll pick it up.
Thanks,
William Breathitt Gray
^ permalink raw reply
* Re: [PATCH 3/4] dt-bindings: iio: adc: Migrate AD7606 documentation to yaml
From: Rob Herring @ 2019-08-02 13:41 UTC (permalink / raw)
To: Beniamin Bia
Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich,
Hartmut Knaack, Peter Meerwald, Greg Kroah-Hartman,
open list:IIO SUBSYSTEM AND DRIVERS, devel,
linux-kernel@vger.kernel.org, Mark Rutland, devicetree,
Paul E. McKenney, Mauro Carvalho Chehab, Linus Walleij,
Nicolas Ferre, biabeniamin
In-Reply-To: <20190802100304.15899-3-beniamin.bia@analog.com>
On Fri, Aug 2, 2019 at 4:03 AM Beniamin Bia <beniamin.bia@analog.com> wrote:
>
> The documentation for ad7606 was migrated to yaml, the new Linux Kernel
> standard.
Did you forget to delete the old file?
It's a DT, not kernel standard really.
>
> Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
> ---
> .../bindings/iio/adc/adi,ad7606.yaml | 134 ++++++++++++++++++
> MAINTAINERS | 2 +-
> 2 files changed, 135 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> new file mode 100644
> index 000000000000..509dbe9c84d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7606.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7606 Simultaneous Sampling ADC
> +
> +maintainers:
> + - Beniamin Bia <beniamin.bia@analog.com>
> + - Stefan Popa <stefan.popa@analog.com>
> +
> +description: |
> + Analog Devices AD7606 Simultaneous Sampling ADC
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7605-4
> + - adi,ad7606-8
> + - adi,ad7606-6
> + - adi,ad7606-4
> + - adi,ad7616
> +
> + reg:
> + maxItems: 1
> +
> + spi-cpha: true
> +
> + avcc-supply:
> + description:
> + Phandle to the Avcc power supply
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + adi,conversion-start-gpios:
> + description:
> + Must be the device tree identifier of the CONVST pin.
> + This logic input is used to initiate conversions on the analog
> + input channels. As the line is active high, it should be marked
> + GPIO_ACTIVE_HIGH.
> + maxItems: 1
> +
> + reset-gpios:
> + description:
> + Must be the device tree identifier of the RESET pin. If specified,
> + it will be asserted during driver probe. As the line is active high,
> + it should be marked GPIO_ACTIVE_HIGH.
> + maxItems: 1
> +
> + standby-gpios:
> + description:
> + Must be the device tree identifier of the STBY pin. This pin is used
> + to place the AD7606 into one of two power-down modes, Standby mode or
> + Shutdown mode. As the line is active low, it should be marked
> + GPIO_ACTIVE_LOW.
> + maxItems: 1
> +
> + adi,first-data-gpios:
> + description:
> + Must be the device tree identifier of the FRSTDATA pin.
> + The FRSTDATA output indicates when the first channel, V1, is
> + being read back on either the parallel, byte or serial interface.
> + As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
> + maxItems: 1
> +
> + adi,range-gpios:
> + description:
> + Must be the device tree identifier of the RANGE pin. The polarity on
> + this pin determines the input range of the analog input channels. If
> + this pin is tied to a logic high, the analog input range is ±10V for
> + all channels. If this pin is tied to a logic low, the analog input range
> + is ±5V for all channels. As the line is active high, it should be marked
> + GPIO_ACTIVE_HIGH.
> + maxItems: 1
> +
> + adi,oversampling-ratio-gpios:
> + description:
> + Must be the device tree identifier of the over-sampling
> + mode pins. As the line is active high, it should be marked
> + GPIO_ACTIVE_HIGH.
> + maxItems: 1
> +
> + adi,sw-mode:
> + description:
> + Software mode of operation, so far available only for ad7616.
> + It is enabled when all three oversampling mode pins are connected to
> + high level. The device is configured by the corresponding registers. If the
> + adi,oversampling-ratio-gpios property is defined, then the driver will set the
> + oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired
> + to VDD.
> + maxItems: 1
> + type: boolean
> +
> +required:
> + - compatible
> + - reg
> + - spi-cpha
> + - avcc-supply
> + - interrupts
> + - adi,conversion-start-gpios
> +
> +examples:
> + - |
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad7606-8";
> + reg = <0>;
> + spi-max-frequency = <1000000>;
> + spi-cpol;
> +
> + avcc-supply = <&adc_vref>;
> +
> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-parent = <&gpio>;
> +
> + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>;
> + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> + adi,oversampling-ratio-gpios = <&gpio 18 GPIO_ACTIVE_HIGH
> + &gpio 23 GPIO_ACTIVE_HIGH
> + &gpio 26 GPIO_ACTIVE_HIGH>;
> + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>;
> + adi,sw-mode;
> + };
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 052d7a8591fb..d2e465772071 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -900,7 +900,7 @@ L: linux-iio@vger.kernel.org
> W: http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/iio/adc/ad7606.c
> -F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.txt
> +F: Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
>
> ANALOG DEVICES INC AD7768-1 DRIVER
> M: Stefan Popa <stefan.popa@analog.com>
> --
> 2.17.1
>
^ 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