* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-11-24 4:40 UTC (permalink / raw)
To: Stephen Boyd
Cc: Kevin Hilman, Vincent Guittot, Rob Herring, Rafael Wysocki,
linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
linux-kernel, Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree@vger.kernel.org, Nayak Rajendra
In-Reply-To: <20161124020322.GI6095@codeaurora.org>
On 23-11-16, 18:03, Stephen Boyd wrote:
> On 11/23, Kevin Hilman wrote:
> > Vincent Guittot <vincent.guittot@linaro.org> writes:
> > > On 23 November 2016 at 16:51, Kevin Hilman <khilman@baylibre.com> wrote:
> > >> Then, at least for this use case, we're talking about voltage, not some
> > >> unspecified units.
>
> In some cases we actually know the voltage of the domain and
> would want to put some voltage mapping in DT. For example, level
> 1 is voltage 2V and level 2 is voltage 2.5V.
But even in these cases we wouldn't be using the voltage values within the
kernel as we will be giving only a performance state to the M3 core, right?
> In other cases we
> don't know the voltage, all we know is the voltage "corner" which
> is a number from 0 to N that is translated into a voltage by the
> firmware but is otherwise unknown what that is outside of the
> firmware. In this case we've lost the units, but otherwise we're
> still interested in requesting some 'level' that the domain be
> operating in.
> > >> But that makes me wonder, this performance state sounds like something
> > >> that is changing dynamically at runtime, so why do you want to describe
> > >> this statically in DT?
Each frequency a device can operate in has the requirement of minimum
performance state of the domain and so we need these values in the DT.
> > >>
> > >> This sounds to me like the job of the genpd. When any device in the
> > >> domain does its pm_runtime_get(), the domain could check the device
> > >> frequency and see if it needs to change the domain voltage in order for
> > >> that device to operate at that frequency.
Also note that the performance index may be required to be changed before
updating the frequency in case we are increasing the frequency which needs a
higher performance index to be set.
> How do we check the device frequency? Does the domain need to
> know about the clocks for all devices that are in the domain and
> what clocks in there are contributing to the voltage requirement?
>
> In out of tree solutions we've 'bucketized' the requirements of
> the devices into an array sized to the number of levels of the
> voltage domain. When a device requires a new level, we increment
> the new level and decrement the old level and then look for the
> largest non-zero index in the array.
For such a design we need to know the index-size in advance and I am not sure if
we should get anything like that from the DT.
> This is the inverse design
> of iterating over all devices in the domain to see what frequency
> they're running at to determine the voltage requirement. I guess
> using PM QoS would be similar here to do the aggregation and then
> tell the domain to go to that level.
>
> > >> When the device goes away
> > >> (using pm_runtime_put()) the domain can check again if it could lower
> > >> the voltage and still meet the requirements of the remaining devices.
This will be done nevertheless.
> > >
> > > That's only part of the job. The device can change its frequency and
> > > as a result ask for a new voltage index while it is already running
> >
> > That's fine. Use clock notifiers, or better use QoS (with notifiers) so
> > that the genpd knows when any of those change.
Yes genpd will be handling it all but it will surely need to know the
performance index for each individual clock rate we support.
The way I have written the code for now is this with another QOS request type
DEV_PM_QOS_PERFORMANCE:
+static int _generic_set_opp_pd(...)
+{
+
...
+ /* Scaling up? Scale voltage before frequency */
+ if (freq > old_freq)
+ dev_pm_qos_update_request(req, perf);
+
+ clk_set_rate(...);
+
+ if (freq < old_freq)
+ dev_pm_qos_update_request(req, perf);
+
+ return 0;
+}
And genpd is registering its notifier for DEV_PM_QOS_PERFORMANCE request type
where it accumulates requests from all the devices and selects the highest one.
--
viresh
^ permalink raw reply
* Re: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-11-24 4:57 UTC (permalink / raw)
To: Stephen Boyd, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Russell King, Linux-Kernel, Mark Brown,
linux-clk, Linux-ARM
In-Reply-To: <87a8cpejn5.wl%kuninori.morimoto.gx@renesas.com>
Hi Stephen, again
> > I've seen bindings that have the 'clocks' property at the top
> > level and the appropriate 'clock-names' property to relate the
> > clocks to a subnode.
> >
> > sound_soc {
> > clocks = <&xxx>, <&xxx>;
> > clock-names = "cpu", "codec";
> > ...
> > cpu {
> > ...
> > };
> > codec {
> > ...
> > };
> > };
> >
> > Then the subnodes call clk_get() with the top level device and
> > the name of their node and things match up. I suppose this
> > binding is finalized though, so we can't really do that?
> >
> > I see that the gpio framework has a similar design called
> > devm_get_gpiod_from_child(), so how about we add a
> > devm_get_clk_from_child() API? That would more closely match the
> > intent here, which is to restrict the clk_get() operation to
> > child nodes of the device passed as the first argument.
> >
> > struct clk *devm_get_clk_from_child(struct device *dev,
> > const char *con_id,
> > struct device_node *child);
Thanks, but, my point is that Linux already have "of_clk_get()",
but we don't have its devm_ version.
The point is that of_clk_get() can get clock from "device_node".
Why having devm_ version become so problem ?
Best regards
---
Kuninori Morimoto
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Sekhar Nori @ 2016-11-24 5:03 UTC (permalink / raw)
To: David Lechner, Kevin Hilman
Cc: Bartosz Golaszewski, Michael Turquette, Rob Herring, Frank Rowand,
Mark Rutland, Peter Ujfalusi, Russell King, linux-devicetree,
David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha,
arm-soc, Laurent Pinchart
In-Reply-To: <f88df859-3c0f-38f2-275d-ce6a9996fb6e-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
On Thursday 24 November 2016 04:18 AM, David Lechner wrote:
> On 11/23/2016 04:32 PM, Kevin Hilman wrote:
>> David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes:
>>
>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote:
>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>:
>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>>>>>
>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>>>>>> controller drivers to da850.dtsi.
>>>>>>
>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>>>> ---
>>>>>> v1 -> v2:
>>>>>> - moved the priority controller node above the cfgchip node
>>>>>> - renamed added nodes to better reflect their purpose
>>>>>>
>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>>>> 1 file changed, 8 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi
>>>>>> b/arch/arm/boot/dts/da850.dtsi
>>>>>> index 1bb1f6d..412eec6 100644
>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>> @@ -210,6 +210,10 @@
>>>>>> };
>>>>>>
>>>>>> };
>>>>>> + prictrl: priority-controller@14110 {
>>>>>> + compatible = "ti,da850-mstpri";
>>>>>> + reg = <0x14110 0x0c>;
>>>>>
>>>>>
>>>>> I think we should add status = "disabled"; here and let boards opt in.
>>>>>
>>>>>> + };
>>>>>> cfgchip: chip-controller@1417c {
>>>>>> compatible = "ti,da830-cfgchip", "syscon",
>>>>>> "simple-mfd";
>>>>>> reg = <0x1417c 0x14>;
>>>>>> @@ -451,4 +455,8 @@
>>>>>> 1 0 0x68000000 0x00008000>;
>>>>>> status = "disabled";
>>>>>> };
>>>>>> + memctrl: memory-controller@b0000000 {
>>>>>> + compatible = "ti,da850-ddr-controller";
>>>>>> + reg = <0xb0000000 0xe8>;
>>>>>
>>>>>
>>>>> same here. status = "disabled";
>>>>>
>>>>>> + };
>>>>>> };
>>>>>>
>>>>
>>>> Hi David,
>>>>
>>>> I did that initially[1][2] and it was rejected by Kevin[3] and
>>>> Laurent[4].
>>>>
>>>> FYI this patch has already been queued by Sekhar.
>>>
>>> Thanks. I did not see those threads.
>>>
>>> FYI to maintainers, having these enabled by default causes error
>>> messages in the kernel log for other boards that are not supported by
>>> the drivers.
>>
>> Then the driver is too noisy and should be cleaned up.
>>
>>> Since there is only one board that is supported and soon
>>> to be 2 that are not, I would rather have this disabled by default to
>>> avoid the error messages.
>>
>> IMO, what exactly are the error messages? Sounds like the driver is
>> being too verbose, and calling things errors that are not really errors.
>
> It is just one line per driver.
>
> dev_err(dev, "no master priorities defined for this board\n");
>
> and
>
> dev_err(dev, "no settings defined for this board\n");
>
>
> Since "ti,da850-lcdk" is the only board supported in these drivers, all
> other boards will see these error messages.
Thats pretty bad. Sorry about that. The original justification for
keeping them enabled all the time was that they are in-SoC modules with
no external dependencies (like IO lines or voltage rails) so they can be
enabled on all boards that use DA850. While that remains true, the
configuration itself is board specific.
I think the error messages are still useful, so instead of silencing
them, I think we should go back to keeping these nodes disabled by
default and enabling only on boards which have support for it in the driver.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/3] dmaengine: sun6i: share the dma driver with sun50i
From: Chen-Yu Tsai @ 2016-11-24 5:04 UTC (permalink / raw)
To: Hao Zhang
Cc: Maxime Ripard, Chen-Yu Tsai, Dan Williams, Vinod Koul,
Mark Rutland, Rob Herring, Catalin Marinas, Will Deacon,
dmaengine-u79uwXL29TY76Z2rM5mHXA, linux-kernel, devicetree,
linux-arm-kernel
In-Reply-To: <1479638740-20520-4-git-send-email-hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi,
On Sun, Nov 20, 2016 at 6:45 PM, Hao Zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Changes the limited buswith to 8 bytes,and add
> the test in sun6i_dma_config function
>
> Accroding to sun6i dma driver, i think ,if the client
> doesn't configure the address width with dmaengine_slave_config
> function, it would use the default width. So we can add the test
> in sun6i_dma_config function called by dmaengine_slave_config,
> and test the configuration whether is support for the device.
>
One thing people haven't really noticed is that starting with
A80, A83T, H3, the DMA channel configuration registers have
been slightly changed when compared to A31/A23/A33. The DMA
burst length field offset was changed by 1.
We need to fix this.
ChenYu
> Signed-off-by: Hao Zhang <hao5781286-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/dma/sun6i-dma.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a235878..f7c90b6 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -250,7 +250,7 @@ static inline s8 convert_burst(u32 maxburst)
> static inline s8 convert_buswidth(enum dma_slave_buswidth addr_width)
> {
> if ((addr_width < DMA_SLAVE_BUSWIDTH_1_BYTE) ||
> - (addr_width > DMA_SLAVE_BUSWIDTH_4_BYTES))
> + (addr_width > DMA_SLAVE_BUSWIDTH_8_BYTES))
> return -EINVAL;
>
> return addr_width >> 1;
> @@ -758,6 +758,18 @@ static int sun6i_dma_config(struct dma_chan *chan,
> {
> struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
>
> + if ((BIT(config->src_addr_width) | chan->device->src_addr_widths) !=
> + chan->device->src_addr_widths) {
> + dev_err(chan2dev(chan), "Invalid DMA configuration\n");
> + return -EINVAL;
> + }
> +
> + if ((BIT(config->dst_addr_width) | chan->device->dst_addr_widths) !=
> + chan->device->dst_addr_widths) {
> + dev_err(chan2dev(chan), "Invalid DMA configuration\n");
> + return -EINVAL;
> + }
> +
> memcpy(&vchan->cfg, config, sizeof(*config));
>
> return 0;
> @@ -1028,11 +1040,23 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
> .nr_max_vchans = 34,
> };
>
> +/*
> + * The A64 has 8 physical channels, a maximum DRQ port id of 27,
> + * and a total of 38 usable source and destination endpoints.
> + */
> +
> +static struct sun6i_dma_config sun50i_a64_dma_cfg = {
> + .nr_max_channels = 8,
> + .nr_max_requests = 27,
> + .nr_max_vchans = 38,
> +};
> +
> static const struct of_device_id sun6i_dma_match[] = {
> { .compatible = "allwinner,sun6i-a31-dma", .data = &sun6i_a31_dma_cfg },
> { .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
> { .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
> { .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
> + { .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> @@ -1112,6 +1136,13 @@ static int sun6i_dma_probe(struct platform_device *pdev)
> BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> sdc->slave.directions = BIT(DMA_DEV_TO_MEM) |
> BIT(DMA_MEM_TO_DEV);
> +
> + if (of_device_is_compatible(pdev->dev.of_node,
> + "allwinner,sun50i-a64-dma")) {
> + sdc->slave.src_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> + sdc->slave.dst_addr_widths |= BIT(DMA_SLAVE_BUSWIDTH_8_BYTES);
> + }
> +
> sdc->slave.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
> sdc->slave.dev = &pdev->dev;
>
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/5] Documentation: pinctrl: palmas: Add ti,palmas-powerhold-override property definition
From: Keerthy @ 2016-11-24 5:28 UTC (permalink / raw)
To: Rob Herring
Cc: tony, lee.jones, linux-omap, linux-kernel, devicetree, linux-gpio,
nm, t-kristo
In-Reply-To: <20161115014352.5d3qwoonlo63mp5p@rob-hp-laptop>
On Tuesday 15 November 2016 07:13 AM, Rob Herring wrote:
> On Thu, Nov 10, 2016 at 10:39:16AM +0530, Keerthy wrote:
>> GPIO7 is configured in POWERHOLD mode which has higher priority
>> over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
>> bit is turned off. This property enables driver to over ride the
>> POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
>> scenarios.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>> Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>
> Acked-by: Rob Herring <robh@kernel.org>
Tony,
Are you planning to pick this one as well?
- Keerthy
>
^ permalink raw reply
* Re: [PATCH 1/2] gpio: axp209: use correct register for GPIO input status
From: Chen-Yu Tsai @ 2016-11-24 5:52 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Quentin Schulz, Linus Walleij, Alexandre Courbot, Rob Herring,
Mark Rutland, Chen-Yu Tsai, linux-gpio@vger.kernel.org,
devicetree, linux-kernel
In-Reply-To: <20161123144532.3089ebfd@free-electrons.com>
On Wed, Nov 23, 2016 at 9:45 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 23 Nov 2016 14:27:48 +0100, Quentin Schulz wrote:
>> The GPIO input status was read from control register
>> (AXP20X_GPIO[210]_CTRL) instead of status register (AXP20X_GPIO20_SS).
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>
> This smells like a bug fix, so perhaps Cc: stable?
Presently there are no in-tree boards that use the GPIOs
as input. And the only user I see is the CHIP, for the headphone
jack detection. Again, not supported in mainline yet.
Not sure if there is value in sending it for stable.
ChenYu
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
From: Sekhar Nori @ 2016-11-24 5:54 UTC (permalink / raw)
To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland,
Kevin Hilman
Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski
In-Reply-To: <06bc8517-8c33-85a1-9d5a-29042c7281db@lechnology.com>
On Wednesday 23 November 2016 09:54 PM, David Lechner wrote:
> On 11/23/2016 05:12 AM, Sekhar Nori wrote:
>> On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
>>> This SoC has a separate pin controller for configuring pullup/pulldown
>>> bias on groups of pins.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>> arch/arm/boot/dts/da850.dtsi | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 8945815..1c0224c 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -210,6 +210,11 @@
>>> };
>>>
>>> };
>>> + pinconf: pin-controller@22c00c {
>>> + compatible = "ti,da850-pupd";
>>> + reg = <0x22c00c 0x8>;
>>> + status = "disabled";
>>> + };
>>
>> Can you please place this below the i2c1 node. I am trying to keep the
>> nodes sorted by unit address. I know thats broken in many places today,
>> but lets add the new ones where they should eventually end up.
>
> I can do this, but it seems that the predominant sorting pattern here is
> to keep subsystems together (e.g. all i2c are together, all uart are
> together, etc.)
Yeah, but that quickly gives away as there are many singleton devices
and everyone tries to add theirs at the end of the list resulting in
merge conflicts.
> Would a separate patch to sort everything by unit address to get this
> cleaned up be acceptable?
Agree with Kevin that it would be churn. If done, it should be last
thing that gets done in a merge window. I would not attempt it in
relatively busy merge windows like this one.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v4 0/2] da8xx: fix section mismatch in new drivers
From: Sekhar Nori @ 2016-11-24 6:01 UTC (permalink / raw)
To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy,
Sudeep Holla
In-Reply-To: <1479908400-10136-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On Wednesday 23 November 2016 07:09 PM, Bartosz Golaszewski wrote:
> Sekhar noticed there's a section mismatch in the da8xx-mstpri and
> da8xx-ddrctl drivers. This is caused by calling
> of_flat_dt_get_machine_name() which has an __init annotation.
>
> This series makes the drivers drop the call and not print the
> machine name in the error message.
Applied both. Thanks!
Regards,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/5] smd: Make packet size a constant
From: Bjorn Andersson @ 2016-11-24 6:14 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: linux-arm-msm, linux-soc, devicetree, linux-mmc, andy.gross,
sboyd, robh, arnd, riteshh
In-Reply-To: <1479863388-23678-3-git-send-email-jeremymc@redhat.com>
On Tue 22 Nov 17:09 PST 2016, Jeremy McNicoll wrote:
> Use a macro to define the maximum size of a RPM message.
>
No thanks.
> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> ---
> drivers/soc/qcom/smd-rpm.c | 2 +-
> include/linux/soc/qcom/smd.h | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/smd-rpm.c b/drivers/soc/qcom/smd-rpm.c
> index 6609d7e..b5a2836 100644
> --- a/drivers/soc/qcom/smd-rpm.c
> +++ b/drivers/soc/qcom/smd-rpm.c
> @@ -114,7 +114,7 @@ int qcom_rpm_smd_write(struct qcom_smd_rpm *rpm,
> size_t size = sizeof(*pkt) + count;
>
> /* SMD packets to the RPM may not exceed 256 bytes */
> - if (WARN_ON(size >= 256))
> + if (WARN_ON(size >= SMD_RPM_MAX_SIZE))
> return -EINVAL;
The only thing you do is to change "oh, the max packet size is 256
bytes" to "hmm, i wonder what SMD_RPM_MAX_SIZE is and if the comment is
still valid".
>
> pkt = kmalloc(size, GFP_KERNEL);
> diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
> index f148e0f..8039015 100644
> --- a/include/linux/soc/qcom/smd.h
> +++ b/include/linux/soc/qcom/smd.h
> @@ -4,6 +4,13 @@
> #include <linux/device.h>
> #include <linux/mod_devicetable.h>
>
> +
> +/*
> + * SMD packets to the RPM may not exceed 256 bytes
> + */
> +#define SMD_RPM_MAX_SIZE 256
> +
And this has nothing to do with SMD, it's a limitation of RPM.
Regards,
Bjorn
^ permalink raw reply
* [PATCH 0/2] ARM: dts: sun6i: Disable display pipeline by default
From: Chen-Yu Tsai @ 2016-11-24 6:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
Hi,
While we now support the internal display pipeline found on sun6i, it
is possible that we are unable to enable the display for some boards,
due to a lack of drivers for the panels or bridges found on them. If
the display pipeline is enabled, the driver will try to enable, and
possibly screw up the simple framebuffer U-boot had configured.
This series disables the display pipeline by default, and re-enables
it for the A31 Hummingbird, which already had its display pipeline
enabled.
The series can go in after 4.10-rc1, as a fix, but should not be delayed
till the next release.
Regards
ChenYu
Chen-Yu Tsai (2):
ARM: dts: sun6i: Disable display pipeline by default
ARM: dts: sun6i: hummingbird: Enable display engine again
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 4 ++++
arch/arm/boot/dts/sun6i-a31.dtsi | 1 +
2 files changed, 5 insertions(+)
--
2.10.2
^ permalink raw reply
* [PATCH 1/2] ARM: dts: sun6i: Disable display pipeline by default
From: Chen-Yu Tsai @ 2016-11-24 6:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161124064339.12615-1-wens-jdAy2FN1RRM@public.gmane.org>
While we now support the internal display pipeline found on sun6i, it
is possible that we are unable to enable the display for some boards,
due to a lack of drivers for the panels or bridges found on them. If
the display pipeline is enabled, the driver will try to enable, and
possibly screw up the simple framebuffer U-boot had configured.
Disable the display pipeline by default.
Fixes: 6d0e5b70be13 ("ARM: dts: sun6i: Add device nodes for first
display pipeline")
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 20a0331ddfb5..4662d3344cd2 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -234,6 +234,7 @@
de: display-engine {
compatible = "allwinner,sun6i-a31-display-engine";
allwinner,pipelines = <&fe0>;
+ status = "disabled";
};
soc@01c00000 {
--
2.10.2
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: sun6i: hummingbird: Enable display engine again
From: Chen-Yu Tsai @ 2016-11-24 6:43 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161124064339.12615-1-wens-jdAy2FN1RRM@public.gmane.org>
Now that we disable the display engine by default, we need to re-enable
it for the Hummingbird A31, which already had its display pipeline
enabled.
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index b168d6df2b30..83643bbd51dc 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -140,6 +140,10 @@
cpu-supply = <®_dcdc3>;
};
+&de {
+ status = "okay";
+};
+
&ehci0 {
status = "okay";
};
--
2.10.2
^ permalink raw reply related
* RE: [PATCH][v2] arm64: Add DTS support for FSL's LS1012A SoC
From: Yao Yuan @ 2016-11-24 7:18 UTC (permalink / raw)
To: shawnguo@kernel.org, robh+dt@kernel.org, mark.rutland@arm.com
Cc: oss@buserror.net, Harninder Rai, Bhaskar U,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <1479320647-24460-1-git-send-email-harninder.rai@nxp.com>
Hi Shawn and All,
Any comment for this LS1012A platform support patch?
Is this a good enough base for consequence dts update patches?
If yes, I'd like to send some dts patches for DSPI and QSPI based on this patch.
Thanks.
Yao
On 11/17/2016 02:24 AM, Harninder Rai wrote:
> LS1012A features an advanced 64-bit ARM v8 CortexA53 processor with 32 KB
> of parity protected L1-I cache, 32 KB of ECC protected L1-D cache, as well as 256
> KB of ECC protected L2 cache.
>
> Features summary
> One 64-bit ARM-v8 Cortex-A53 core with the following capabilities
> - Arranged as a cluster of one core supporting a 256 KB L2 cache with ECC
> protection
> - Speed up to 800 MHz
> - Parity-protected 32 KB L1 instruction cache and 32 KB L1 data cache
> - Neon SIMD engine
> - ARM v8 cryptography extensions
> One 16-bit DDR3L SDRAM memory controller ARM core-link CCI-400 cache
> coherent interconnect Cryptography acceleration (SEC) One Configurable x3
> SerDes One PCI Express Gen2 controller, supporting x1 operation One serial
> ATA (SATA Gen 3.0) controller One USB 3.0/2.0 controller with integrated PHY
>
> Following levels of DTSI/DTS files have been created for the LS1012A
> SoC family:
>
> - fsl-ls1012a.dtsi:
> DTS-Include file for FSL LS1012A SoC.
>
> - fsl-ls1012a-frdm.dts:
> DTS file for FSL LS1012A FRDM board.
>
> - fsl-ls1012a-qds.dts:
> DTS file for FSL LS1012A QDS board.
>
> - fsl-ls1012a-rdb.dts:
> DTS file for FSL LS1012A RDB board.
>
> Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
> Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
> ---
> Changes in v2: Incorporated Shawn's comments
> - Brief introduction of the SoC in commit message
> - Alphabetic ordering of labeled nodes
> - Better naming to be used for regulator node
> - Make timer node's comments more readable
> - Sort nodes with unit-address in order of the address
>
> arch/arm64/boot/dts/freescale/Makefile | 3 +
> arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts | 115 ++++++++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts | 128 +++++++++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts | 59 +++++
> arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 245
> +++++++++++++++++++++
> 5 files changed, 550 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
>
^ permalink raw reply
* Re: [RFC PATCH] ARM: dts: Add support for Turris Omnia
From: Uwe Kleine-König @ 2016-11-24 8:37 UTC (permalink / raw)
To: Andrew Lunn, Tomas Hlavacek
Cc: Rob Herring, Mark Rutland, Russell King, Jason Cooper,
Gregory Clement, Sebastian Hesselbarth, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20161123003505.GL2691@lunn.ch>
[-- Attachment #1.1: Type: text/plain, Size: 1525 bytes --]
On 11/23/2016 01:35 AM, Andrew Lunn wrote:
>> +++ b/arch/arm/boot/dts/armada-385-turris-omnia.dts
>> @@ -0,0 +1,279 @@
>> +/*
>> + * Device Tree file for the Turris Omnia
>> + * Schematic available at https://www.turris.cz/doc/_media/rtrom01-schema.pdf
>
> Cool that there is a link to the schematics. But please could you put
> it lower down. It is more likely to be seen if it comes after the
> copyright and license section.
I added to the top because that's where I would look. But checking other
dts files it seems indeed to be more common after the copyright stuff.
I'd suggest to even start a new comment (i.e.
* last blabla of copyright
*/
/*
* Schematic available at ...
) to be more "loud".
@Tomas: I think it doesn't make sense when we alternate sending patches
without prior arrangement. Do you already work on a v5? If not I can do
that to fix the last few comments. Not sure when a submission is too
late to enter v4.10, but I think the window isn't that big any more.
> No leds? No buttons via gpio-keys?
The leds are controlled by a Cortex-M0 and without intervention blink
according to a hardware function (network, power, pci). IMHO that's ok
for an initial setup.
And there are no buttons that are routed to the Armada CPU. Just a reset
button (well, ok, this one is routed to the Armada CPU, but you cannot
make this a gpio-key :-) and the other button is used to control the
brightness of the LEDs and is only routed to the M0.
Best regards
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH V4 0/2] pinctrl: tegra: Add support for IO pad control
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
swarren-3lzwWm7+Weoh9ZMKESR00Q,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
gnurou-Re5JQEeQqe8AvxtiuMwx3w, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
joe-6d6DIl74uiNBDgjK7y7TUQ
Cc: yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Laxman Dewangan
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO power rail sources. When IO
interface are not used then IO pads can be configure in low power
state to reduce the power from that IO pads.
This series add the support of configuration of IO pad via pinctrl
framework. The io pad driver uses the tegra PMC interface.
---
This driver was sent earlier for review along with soc/tegra pmc
changes. During review, decided to first conclude in soc/tegra pmc
patches and then review this.
Thierry applied the pmc patches in the private tree
https://github.com/thierryreding/linux/tree/tegra186
and he wanted to have the patches for user of the new APIs so that
it can be pushed to mainline.
Sending the pinctrl driver. This needs Ack/reviewed from pinctrl subsystem
i.e. Linus Welleij to apply in the Thierry's T186 branch along with
PMC patches.
---
Changes from V1:
- use the regulator framework to get the IO voltage instead of table from
DT. The regulator handle is provided from DT.
Changes from V2:
- Nit fixes and variable/allocation optimisation as per review comment from
V2.
Changes from V3:
Use devm_regulator_get() instead of devm_regulator_get_optional().
Laxman Dewangan (2):
pinctrl: tegra: Add DT binding for io pads control
pinctrl: tegra: Add driver to configure voltage and power of io pads
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 126 +++++
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 530 +++++++++++++++++++++
4 files changed, 669 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V4 1/2] pinctrl: tegra: Add DT binding for io pads control
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij, robh+dt, mark.rutland, swarren, thierry.reding,
gnurou, jonathanh, joe
Cc: yamada.masahiro, linux-gpio, devicetree, linux-tegra,
linux-kernel, Laxman Dewangan
In-Reply-To: <1479976734-30498-1-git-send-email-ldewangan@nvidia.com>
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
sources. When IO interfaces are not used then IO pads can be
configure in low power state to reduce the power consumption from
that IO pads.
On Tegra124, the voltage level of IO power rail source is auto
detected by hardware(SoC) and hence it is only require to configure
in low power mode if IO pads are not used.
On T210 onwards, the auto-detection of voltage level from IO power
rail is removed from SoC and hence SW need to configure the PMC
register explicitly to set proper voltage in IO pads based on
IO rail power source voltage.
Add DT binding document for detailing the DT properties for
configuring IO pads voltage levels and its power state.
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
New in series based on pinctrl driver requirement.
Changes from V2:
Updated the statement to say 1.8V and 3.3V as nominal voltage.
Corrected DT example by adding -supply and taken care of V1 review
from Rob.
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 126 +++++++++++++++++++++
1 file changed, 126 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
new file mode 100644
index 0000000..a88c484
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
@@ -0,0 +1,126 @@
+NVIDIA Tegra PMC IO pad controller
+
+NVIDIA Tegra124 and later SoCs support the multi-voltage level and low power
+state of some of its IO pads. When IO interface are not used then IO pads can
+be configure in low power state to reduce the power from that IO pads. The IO
+pads can work in the nominal IO voltage of 1.8V and 3.3V from power rail
+sources.
+
+On Tegra124, the voltage of IO power rail source is auto detected by SoC and
+hence it is only require to configure in low power mode if IO pads are not
+used.
+
+On T210 onwards, the HW based auto-detection for IO voltage is removed and
+hence SW need to configure the PMC register explicitly, to set proper voltage
+in IO pads, based on IO rail power source voltage.
+
+The voltage configurations and low power state of IO pads should be done in
+boot if it is not going to change otherwise dynamically based on IO rail
+voltage on that IO pads and usage of IO pads
+
+The DT property of the IO pads must be under the node of pmc i.e.
+pmc@7000e400 for Tegra124 onwards.
+
+Please refer to <pinctrl-bindings.txt> in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+Tegra's pin configuration nodes act as a container for an arbitrary number of
+subnodes. Each of these subnodes represents some desired configuration for an
+IO pads, or a list of IO pads. This configuration can include the voltage and
+power enable/disable control
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content. Each subnode only affects those
+parameters that are explicitly listed. Unspecified is represented as an absent
+property,
+
+See the TRM to determine which properties and values apply to each IO pads.
+
+Required subnode-properties:
+==========================
+- pins : An array of strings. Each string contains the name of an IO pads. Valid
+ values for these names are listed below.
+
+Optional subnode-properties:
+==========================
+Following properties are supported from generic pin configuration explained
+in <dt-bindings/pinctrl/pinctrl-binding.txt>.
+low-power-enable: enable low power mode.
+low-power-disable: disable low power mode.
+
+Valid values for pin for T124 are:
+ audio, bb, cam, comp, csia, csib, csie, dsi, dsib, dsic, dsid, hdmi,
+ hsic, hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2,
+ pex-ctrl, sdmmc1, sdmmc3, sdmmc4, sys-ddc, uart, usb0, usb1, usb2,
+ usb-bias
+
+Valid values for pin for T210 are:
+ audio, audio-hv, cam, csia, csib, csic, csid, csie, csif,
+ dbg, debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2,
+ gpio, hdmi, hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2,
+ pex-ctrl, sdmmc1, sdmmc3, spi, spi-hv, uart, usb-bias, usb0,
+ usb1, usb2, usb3.
+
+To find out the IO rail voltage for setting the voltage of IO pad by SW,
+the regulator supply handle must provided from the DT and it is explained
+in the regulator DT binding document
+ <devicetree/bindings/regulator/regulator.txt>.
+For example, for GPIO rail the supply name is vddio-gpio and regulator
+handle is supplied from DT as
+ vddio-gpio-supply = <®ulator_xyz>;
+
+For T210, following IO pads support the 1.8V/3.3V and the corresponding
+IO voltage pin names are as follows:
+ audio -> vddio-audio
+ audio-hv -> vddio-audio-hv
+ cam ->vddio-cam
+ dbg -> vddio-dbg
+ dmic -> vddio-dmic
+ gpio -> vddio-gpio
+ pex-ctrl -> vddio-pex-ctrl
+ sdmmc1 -> vddio-sdmmc1
+ sdmmc3 -> vddio-sdmmc3
+ spi -> vddio-spi
+ spi-hv -> vddio-spi-hv
+ uart -> vddio-uart
+
+Example:
+ i2c@7000d000 {
+ pmic@3c {
+ regulators {
+ vddio_sdmmc1: ldo2 {
+ /* Regulator entries for LDO2 */
+ };
+
+ vdd_cam: ldo3 {
+ /* Regulator entries for LDO3 */
+ };
+ };
+ };
+ };
+
+ pmc@7000e400 {
+ vddio-cam-supply = <&vdd_cam>;
+ vddio-sdmmc1-supply = <&vddio_sdmmc1>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&tegra_io_pad_volt_default>;
+ tegra_io_pad_volt_default: common {
+ audio-hv {
+ pins = "audio-hv";
+ low-power-disable;
+ };
+
+ gpio {
+ pins = "gpio";
+ low-power-disable;
+ };
+
+ audio {
+ pins = "audio", "dmic", "sdmmc3";
+ low-power-enable;
+ };
+ };
+
+ };
--
2.1.4
^ permalink raw reply related
* [PATCH V4 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Laxman Dewangan @ 2016-11-24 8:38 UTC (permalink / raw)
To: linus.walleij, robh+dt, mark.rutland, swarren, thierry.reding,
gnurou, jonathanh, joe
Cc: yamada.masahiro, linux-gpio, devicetree, linux-tegra,
linux-kernel, Laxman Dewangan
In-Reply-To: <1479976734-30498-1-git-send-email-ldewangan@nvidia.com>
NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
sources. When IO interfaces are not used then IO pads can be
configure in low power state to reduce the power consumption from
that IO pads.
On Tegra124, the voltage level of IO power rail source is auto
detected by hardware(SoC) and hence it is only require to configure
in low power mode if IO pads are not used.
On T210 onwards, the auto-detection of voltage level from IO power
rail is removed from SoC and hence SW need to configure the PMC
register explicitly to set proper voltage in IO pads based on
IO rail power source voltage.
This driver adds the IO pad driver to configure the power state and
IO pad voltage based on the usage and power tree via pincontrol
framework. The configuration can be static and dynamic.
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
Changes from V1:
- Dropped the custom properties to set pad voltage and use regulator.
- Added support for regulator to get vottage in boot and configure IO
pad voltage.
- Add support for callback to handle regulator notification and configure
IO pad voltage based on voltage change.
Changes from V2:
Mostly nit changes per Jon's feedback i.e. use macros for voltage, added
comment on macros, reduce the structure and variable name size, optimise
number of variables, and allocate memory for regulator info when it needed.
Changes from V3:
Use the devm_regulator_get() instead of devm_regulator_get_optional().
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 530 +++++++++++++++++++++++++++
3 files changed, 543 insertions(+)
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
diff --git a/drivers/pinctrl/tegra/Kconfig b/drivers/pinctrl/tegra/Kconfig
index 24e20cc..6004e5c 100644
--- a/drivers/pinctrl/tegra/Kconfig
+++ b/drivers/pinctrl/tegra/Kconfig
@@ -23,6 +23,18 @@ config PINCTRL_TEGRA210
bool
select PINCTRL_TEGRA
+config PINCTRL_TEGRA_IO_PAD
+ bool "Tegra IO pad Control Driver"
+ depends on ARCH_TEGRA && REGULATOR
+ select PINCONF
+ select PINMUX
+ help
+ NVIDIA Tegra124/210 SoC has IO pads which supports multi-voltage
+ level of interfacing and deep power down mode of IO pads. The
+ voltage of IO pads are SW configurable based on IO rail of that
+ pads on T210. This driver provides the interface to change IO pad
+ voltage and power state via pincontrol interface.
+
config PINCTRL_TEGRA_XUSB
def_bool y if ARCH_TEGRA
select GENERIC_PHY
diff --git a/drivers/pinctrl/tegra/Makefile b/drivers/pinctrl/tegra/Makefile
index d9ea2be..3ebaaa2 100644
--- a/drivers/pinctrl/tegra/Makefile
+++ b/drivers/pinctrl/tegra/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_PINCTRL_TEGRA30) += pinctrl-tegra30.o
obj-$(CONFIG_PINCTRL_TEGRA114) += pinctrl-tegra114.o
obj-$(CONFIG_PINCTRL_TEGRA124) += pinctrl-tegra124.o
obj-$(CONFIG_PINCTRL_TEGRA210) += pinctrl-tegra210.o
+obj-$(CONFIG_PINCTRL_TEGRA_IO_PAD) += pinctrl-tegra-io-pad.o
obj-$(CONFIG_PINCTRL_TEGRA_XUSB) += pinctrl-tegra-xusb.o
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
new file mode 100644
index 0000000..aab02d0
--- /dev/null
+++ b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
@@ -0,0 +1,530 @@
+/*
+ * pinctrl-tegra-io-pad: IO PAD driver for configuration of IO rail and deep
+ * Power Down mode via pinctrl framework.
+ *
+ * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
+ *
+ * Author: Laxman Dewangan <ldewangan@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <soc/tegra/pmc.h>
+
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinctrl-utils.h"
+
+#define TEGRA_IO_RAIL_1800000UV 1800000
+#define TEGRA_IO_RAIL_3300000UV 3300000
+
+/* Covert IO voltage to IO pad voltage enum */
+#define tegra_io_uv_to_io_pads_uv(io_uv) \
+ (((io_uv) == TEGRA_IO_RAIL_1800000UV) ? \
+ TEGRA_IO_PAD_1800000UV : TEGRA_IO_PAD_3300000UV)
+
+#define tegra_io_voltage_is_valid(io_uv) \
+ ({ typeof(io_uv) io_uv_ = (io_uv); \
+ ((io_uv_ == TEGRA_IO_RAIL_1800000UV) || \
+ (io_uv_ == TEGRA_IO_RAIL_3300000UV)); })
+
+struct tegra_io_pads_cfg {
+ const char *name;
+ const unsigned int pins[1];
+ const char *vsupply;
+ enum tegra_io_pad id;
+ bool supports_low_power;
+};
+
+struct tegra_io_pads_soc_data {
+ const struct tegra_io_pads_cfg *cfg;
+ int num_cfg;
+ const struct pinctrl_pin_desc *desc;
+ int num_desc;
+};
+
+struct tegra_io_pads_info {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ const struct tegra_io_pads_soc_data *soc_data;
+};
+
+struct tegra_io_pads_regulator_info {
+ struct tegra_io_pads_info *tiopi;
+ const struct tegra_io_pads_cfg *cfg;
+ struct regulator *regulator;
+ struct notifier_block regulator_nb;
+};
+
+static int tegra_io_pads_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->num_cfg;
+}
+
+static const char *tegra_io_pads_pinctrl_get_group_name(
+ struct pinctrl_dev *pctldev, unsigned int group)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->cfg[group].name;
+}
+
+static int tegra_io_pads_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = tiopi->soc_data->cfg[group].pins;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops tegra_io_pads_pinctrl_ops = {
+ .get_groups_count = tegra_io_pads_pinctrl_get_groups_count,
+ .get_group_name = tegra_io_pads_pinctrl_get_group_name,
+ .get_group_pins = tegra_io_pads_pinctrl_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+ .dt_free_map = pinctrl_utils_free_map,
+};
+
+static int tegra_io_pads_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ int param = pinconf_to_config_param(*config);
+ const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
+ int arg = 0;
+ int ret;
+
+ switch (param) {
+ case PIN_CONFIG_LOW_POWER_MODE:
+ if (!cfg->supports_low_power) {
+ dev_err(tiopi->dev,
+ "IO pad %s does not support low power\n",
+ cfg->name);
+ return -EINVAL;
+ }
+
+ ret = tegra_io_pad_power_get_status(cfg->id);
+ if (ret < 0)
+ return ret;
+ arg = !ret;
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n", param);
+ return -EINVAL;
+ }
+
+ *config = pinconf_to_config_packed(param, (u16)arg);
+
+ return 0;
+}
+
+static int tegra_io_pads_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ const struct tegra_io_pads_cfg *cfg = &tiopi->soc_data->cfg[pin];
+ int i;
+
+ for (i = 0; i < num_configs; i++) {
+ int ret;
+ int param = pinconf_to_config_param(configs[i]);
+ u16 param_val = pinconf_to_config_argument(configs[i]);
+
+ switch (param) {
+ case PIN_CONFIG_LOW_POWER_MODE:
+ if (!cfg->supports_low_power) {
+ dev_err(tiopi->dev,
+ "IO pad %s does not support low power\n",
+ cfg->name);
+ return -EINVAL;
+ }
+ if (param_val)
+ ret = tegra_io_pad_power_disable(cfg->id);
+ else
+ ret = tegra_io_pad_power_enable(cfg->id);
+ if (ret < 0) {
+ dev_err(tiopi->dev,
+ "Failed to set DPD %d of io-pad %s: %d\n",
+ param_val, cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n",
+ param);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static const struct pinconf_ops tegra_io_pads_pinconf_ops = {
+ .pin_config_get = tegra_io_pads_pinconf_get,
+ .pin_config_set = tegra_io_pads_pinconf_set,
+};
+
+static struct pinctrl_desc tegra_io_pads_pinctrl_desc = {
+ .name = "pinctrl-tegra-io-pads",
+ .pctlops = &tegra_io_pads_pinctrl_ops,
+ .confops = &tegra_io_pads_pinconf_ops,
+};
+
+static int tegra_io_pads_rail_change_notify_cb(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct tegra_io_pads_regulator_info *rinfo;
+ struct pre_voltage_change_data *vdata;
+ unsigned long int io_volt_uv;
+ enum tegra_io_pad_voltage pad_volt;
+ int ret;
+
+ rinfo = container_of(nb, struct tegra_io_pads_regulator_info,
+ regulator_nb);
+
+ switch (event) {
+ case REGULATOR_EVENT_PRE_VOLTAGE_CHANGE:
+ vdata = data;
+
+ if (!tegra_io_voltage_is_valid(vdata->old_uV) ||
+ !tegra_io_voltage_is_valid(vdata->min_uV)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu:%lu\n",
+ rinfo->cfg->name, vdata->old_uV, vdata->min_uV);
+ return -EINVAL;
+ }
+
+ /**
+ * Change IO pad voltage before changing IO voltage when it
+ * changes from 1.8V to 3.3V
+ */
+ if (vdata->min_uV == TEGRA_IO_RAIL_1800000UV)
+ break;
+
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
+ TEGRA_IO_PAD_3300000UV);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ vdata->min_uV, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ case REGULATOR_EVENT_VOLTAGE_CHANGE:
+ io_volt_uv = (unsigned long)data;
+ ret = tegra_io_pad_get_voltage(rinfo->cfg->id);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to get IO pad voltage: %d\n", ret);
+ return ret;
+ }
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu\n",
+ rinfo->cfg->name, io_volt_uv);
+ return -EINVAL;
+ }
+
+ /*
+ * If IO pad configuration matching with IO rail voltage then
+ * do nothing.
+ */
+ if (((io_volt_uv == TEGRA_IO_RAIL_1800000UV) &&
+ (ret == TEGRA_IO_PAD_1800000UV)) ||
+ ((io_volt_uv == TEGRA_IO_RAIL_3300000UV) &&
+ (ret == TEGRA_IO_PAD_3300000UV)))
+ break;
+
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id,
+ TEGRA_IO_PAD_1800000UV);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ vdata->min_uV, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ case REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE:
+ io_volt_uv = (unsigned long)data;
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_err(rinfo->tiopi->dev,
+ "IO rail %s voltage is not 1.8/3.3V: %lu\n",
+ rinfo->cfg->name, io_volt_uv);
+ return -EINVAL;
+ }
+
+ pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
+ ret = tegra_io_pad_set_voltage(rinfo->cfg->id, pad_volt);
+ if (ret < 0) {
+ dev_err(rinfo->tiopi->dev,
+ "Failed to set voltage %lu of pad %s: %d\n",
+ io_volt_uv, rinfo->cfg->name, ret);
+ return ret;
+ }
+ break;
+
+ default:
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static int tegra_io_pads_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ const struct tegra_io_pads_soc_data *soc_data =
+ (const struct tegra_io_pads_soc_data *)id->driver_data;
+ struct tegra_io_pads_info *tiopi;
+ int ret, i;
+
+ if (!pdev->dev.parent->of_node) {
+ dev_err(dev, "PMC should be register from DT\n");
+ return -ENODEV;
+ }
+
+ tiopi = devm_kzalloc(dev, sizeof(*tiopi), GFP_KERNEL);
+ if (!tiopi)
+ return -ENOMEM;
+
+ tiopi->dev = &pdev->dev;
+ pdev->dev.of_node = pdev->dev.parent->of_node;
+ tiopi->soc_data = soc_data;
+
+ for (i = 0; i < soc_data->num_cfg; ++i) {
+ struct tegra_io_pads_regulator_info *rinfo;
+ enum tegra_io_pad_voltage pad_volt;
+ int io_volt_uv;
+
+ if (!soc_data->cfg[i].vsupply)
+ continue;
+
+ rinfo = devm_kzalloc(dev, sizeof(*rinfo), GFP_KERNEL);
+ if (!rinfo)
+ return -ENOMEM;
+
+ rinfo->tiopi = tiopi;
+ rinfo->cfg = &soc_data->cfg[i];
+
+ rinfo->regulator = devm_regulator_get(dev,
+ soc_data->cfg[i].vsupply);
+ if (IS_ERR(rinfo->regulator)) {
+ ret = PTR_ERR(rinfo->regulator);
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ continue;
+ }
+
+ io_volt_uv = regulator_get_voltage(rinfo->regulator);
+ if (io_volt_uv < 0) {
+ dev_warn(dev, "Failed to get voltage for rail %s: %d\n",
+ soc_data->cfg[i].vsupply, io_volt_uv);
+ continue;
+ }
+
+ if (!tegra_io_voltage_is_valid(io_volt_uv)) {
+ dev_warn(dev, "IO rail %s voltage is not 1.8/3.3V: %d\n",
+ soc_data->cfg[i].vsupply, io_volt_uv);
+ continue;
+ }
+
+ pad_volt = tegra_io_uv_to_io_pads_uv(io_volt_uv);
+ ret = tegra_io_pad_set_voltage(soc_data->cfg[i].id, pad_volt);
+ if (ret < 0) {
+ dev_err(dev, "Failed to set voltage %d of pad %s: %d\n",
+ io_volt_uv, soc_data->cfg[i].name, ret);
+ return ret;
+ }
+
+ rinfo->regulator_nb.notifier_call =
+ tegra_io_pads_rail_change_notify_cb;
+ ret = devm_regulator_register_notifier(rinfo->regulator,
+ &rinfo->regulator_nb);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register regulator %s notifier: %d\n",
+ soc_data->cfg[i].name, ret);
+ return ret;
+ }
+ }
+
+ tegra_io_pads_pinctrl_desc.pins = tiopi->soc_data->desc;
+ tegra_io_pads_pinctrl_desc.npins = tiopi->soc_data->num_desc;
+ platform_set_drvdata(pdev, tiopi);
+
+ tiopi->pctl = devm_pinctrl_register(dev, &tegra_io_pads_pinctrl_desc,
+ tiopi);
+ if (IS_ERR(tiopi->pctl)) {
+ ret = PTR_ERR(tiopi->pctl);
+ dev_err(dev, "Failed to register io-pad pinctrl driver: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+#define TEGRA124_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, NULL), \
+ _entry_(1, "bb", BB, true, NULL), \
+ _entry_(2, "cam", CAM, true, NULL), \
+ _entry_(3, "comp", COMP, true, NULL), \
+ _entry_(4, "csia", CSIA, true, NULL), \
+ _entry_(5, "csib", CSIB, true, NULL), \
+ _entry_(6, "csie", CSIE, true, NULL), \
+ _entry_(7, "dsi", DSI, true, NULL), \
+ _entry_(8, "dsib", DSIB, true, NULL), \
+ _entry_(9, "dsic", DSIC, true, NULL), \
+ _entry_(10, "dsid", DSID, true, NULL), \
+ _entry_(11, "hdmi", HDMI, true, NULL), \
+ _entry_(12, "hsic", HSIC, true, NULL), \
+ _entry_(13, "hv", HV, true, NULL), \
+ _entry_(14, "lvds", LVDS, true, NULL), \
+ _entry_(15, "mipi-bias", MIPI_BIAS, true, NULL), \
+ _entry_(16, "nand", NAND, true, NULL), \
+ _entry_(17, "pex-bias", PEX_BIAS, true, NULL), \
+ _entry_(18, "pex-clk1", PEX_CLK1, true, NULL), \
+ _entry_(19, "pex-clk2", PEX_CLK2, true, NULL), \
+ _entry_(20, "pex-ctrl", PEX_CNTRL, true, NULL), \
+ _entry_(21, "sdmmc1", SDMMC1, true, NULL), \
+ _entry_(22, "sdmmc3", SDMMC3, true, NULL), \
+ _entry_(23, "sdmmc4", SDMMC4, true, NULL), \
+ _entry_(24, "sys-ddc", SYS_DDC, true, NULL), \
+ _entry_(25, "uart", UART, true, NULL), \
+ _entry_(26, "usb0", USB0, true, NULL), \
+ _entry_(27, "usb1", USB1, true, NULL), \
+ _entry_(28, "usb2", USB2, true, NULL), \
+ _entry_(29, "usb-bias", USB_BIAS, true, NULL)
+
+#define TEGRA210_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, "vddio-audio"), \
+ _entry_(1, "audio-hv", AUDIO_HV, true, "vddio-audio-hv"), \
+ _entry_(2, "cam", CAM, true, "vddio-cam"), \
+ _entry_(3, "csia", CSIA, true, NULL), \
+ _entry_(4, "csib", CSIB, true, NULL), \
+ _entry_(5, "csic", CSIC, true, NULL), \
+ _entry_(6, "csid", CSID, true, NULL), \
+ _entry_(7, "csie", CSIE, true, NULL), \
+ _entry_(8, "csif", CSIF, true, NULL), \
+ _entry_(9, "dbg", DBG, true, "vddio-dbg"), \
+ _entry_(10, "debug-nonao", DEBUG_NONAO, true, NULL), \
+ _entry_(11, "dmic", DMIC, true, "vddio-dmic"), \
+ _entry_(12, "dp", DP, true, NULL), \
+ _entry_(13, "dsi", DSI, true, NULL), \
+ _entry_(14, "dsib", DSIB, true, NULL), \
+ _entry_(15, "dsic", DSIC, true, NULL), \
+ _entry_(16, "dsid", DSID, true, NULL), \
+ _entry_(17, "emmc", SDMMC4, true, NULL), \
+ _entry_(18, "emmc2", EMMC2, true, NULL), \
+ _entry_(19, "gpio", GPIO, true, "vddio-gpio"), \
+ _entry_(20, "hdmi", HDMI, true, NULL), \
+ _entry_(21, "hsic", HSIC, true, NULL), \
+ _entry_(22, "lvds", LVDS, true, NULL), \
+ _entry_(23, "mipi-bias", MIPI_BIAS, true, NULL), \
+ _entry_(24, "pex-bias", PEX_BIAS, true, NULL), \
+ _entry_(25, "pex-clk1", PEX_CLK1, true, NULL), \
+ _entry_(26, "pex-clk2", PEX_CLK2, true, NULL), \
+ _entry_(27, "pex-ctrl", PEX_CNTRL, false, "vddio-pex-ctrl"), \
+ _entry_(28, "sdmmc1", SDMMC1, true, "vddio-sdmmc1"), \
+ _entry_(29, "sdmmc3", SDMMC3, true, "vddio-sdmmc3"), \
+ _entry_(30, "spi", SPI, true, "vddio-spi"), \
+ _entry_(31, "spi-hv", SPI_HV, true, "vddio-spi-hv"), \
+ _entry_(32, "uart", UART, true, "vddio-uart"), \
+ _entry_(33, "usb0", USB0, true, NULL), \
+ _entry_(34, "usb1", USB1, true, NULL), \
+ _entry_(35, "usb2", USB2, true, NULL), \
+ _entry_(36, "usb3", USB3, true, NULL), \
+ _entry_(37, "usb-bias", USB_BIAS, true, NULL)
+
+#define TEGRA_IO_PAD_INFO(_pin, _name, _id, _lpstate, _vsupply) \
+ { \
+ .name = _name, \
+ .pins = {(_pin)}, \
+ .id = TEGRA_IO_PAD_##_id, \
+ .vsupply = (_vsupply), \
+ .supports_low_power = (_lpstate), \
+ }
+
+static const struct tegra_io_pads_cfg tegra124_io_pads_cfg_info[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+static const struct tegra_io_pads_cfg tegra210_io_pads_cfg_info[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+#define TEGRA_IO_PAD_DESC(_pin, _name, _id, _lpstate, _vsupply) \
+ PINCTRL_PIN(_pin, _name)
+
+static const struct pinctrl_pin_desc tegra124_io_pads_pinctrl_desc[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct pinctrl_pin_desc tegra210_io_pads_pinctrl_desc[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct tegra_io_pads_soc_data tegra124_io_pad_soc_data = {
+ .desc = tegra124_io_pads_pinctrl_desc,
+ .num_desc = ARRAY_SIZE(tegra124_io_pads_pinctrl_desc),
+ .cfg = tegra124_io_pads_cfg_info,
+ .num_cfg = ARRAY_SIZE(tegra124_io_pads_cfg_info),
+};
+
+static const struct tegra_io_pads_soc_data tegra210_io_pad_soc_data = {
+ .desc = tegra210_io_pads_pinctrl_desc,
+ .num_desc = ARRAY_SIZE(tegra210_io_pads_pinctrl_desc),
+ .cfg = tegra210_io_pads_cfg_info,
+ .num_cfg = ARRAY_SIZE(tegra210_io_pads_cfg_info),
+};
+
+static const struct platform_device_id tegra_io_pads_dev_id[] = {
+ {
+ .name = "pinctrl-t124-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra124_io_pad_soc_data,
+ }, {
+ .name = "pinctrl-t210-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra210_io_pad_soc_data,
+ }, {
+ },
+};
+MODULE_DEVICE_TABLE(platform, tegra_io_pads_dev_id);
+
+static struct platform_driver tegra_io_pads_pinctrl_driver = {
+ .driver = {
+ .name = "pinctrl-tegra-io-pad",
+ },
+ .probe = tegra_io_pads_pinctrl_probe,
+ .id_table = tegra_io_pads_dev_id,
+};
+
+module_platform_driver(tegra_io_pads_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA TEGRA IO pad Control Driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2 1/5] Documentation: pinctrl: palmas: Add ti,palmas-powerhold-override property definition
From: Lee Jones @ 2016-11-24 8:45 UTC (permalink / raw)
To: Keerthy
Cc: Rob Herring, tony, linux-omap, linux-kernel, devicetree,
linux-gpio, nm, t-kristo
In-Reply-To: <23bf98ee-bcdc-e100-54ce-6f4b1e13ffcf@ti.com>
On Thu, 24 Nov 2016, Keerthy wrote:
>
>
> On Tuesday 15 November 2016 07:13 AM, Rob Herring wrote:
> > On Thu, Nov 10, 2016 at 10:39:16AM +0530, Keerthy wrote:
> > > GPIO7 is configured in POWERHOLD mode which has higher priority
> > > over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
> > > bit is turned off. This property enables driver to over ride the
> > > POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
> > > scenarios.
> > >
> > > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > > ---
> > > Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> >
> > Acked-by: Rob Herring <robh@kernel.org>
>
> Tony,
>
> Are you planning to pick this one as well?
This should be taken by LinusW.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Bartosz Golaszewski @ 2016-11-24 8:48 UTC (permalink / raw)
To: Sekhar Nori
Cc: David Lechner, Kevin Hilman, Michael Turquette, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King,
linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen,
Jyri Sarha, arm-soc, Laurent Pinchart
In-Reply-To: <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org>
2016-11-24 6:03 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>:
> On Thursday 24 November 2016 04:18 AM, David Lechner wrote:
>> On 11/23/2016 04:32 PM, Kevin Hilman wrote:
>>> David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes:
>>>
>>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote:
>>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>:
>>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>>>>>>
>>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>>>>>>> controller drivers to da850.dtsi.
>>>>>>>
>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>>>>> ---
>>>>>>> v1 -> v2:
>>>>>>> - moved the priority controller node above the cfgchip node
>>>>>>> - renamed added nodes to better reflect their purpose
>>>>>>>
>>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>>>>>> 1 file changed, 8 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi
>>>>>>> b/arch/arm/boot/dts/da850.dtsi
>>>>>>> index 1bb1f6d..412eec6 100644
>>>>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>>>>> @@ -210,6 +210,10 @@
>>>>>>> };
>>>>>>>
>>>>>>> };
>>>>>>> + prictrl: priority-controller@14110 {
>>>>>>> + compatible = "ti,da850-mstpri";
>>>>>>> + reg = <0x14110 0x0c>;
>>>>>>
>>>>>>
>>>>>> I think we should add status = "disabled"; here and let boards opt in.
>>>>>>
>>>>>>> + };
>>>>>>> cfgchip: chip-controller@1417c {
>>>>>>> compatible = "ti,da830-cfgchip", "syscon",
>>>>>>> "simple-mfd";
>>>>>>> reg = <0x1417c 0x14>;
>>>>>>> @@ -451,4 +455,8 @@
>>>>>>> 1 0 0x68000000 0x00008000>;
>>>>>>> status = "disabled";
>>>>>>> };
>>>>>>> + memctrl: memory-controller@b0000000 {
>>>>>>> + compatible = "ti,da850-ddr-controller";
>>>>>>> + reg = <0xb0000000 0xe8>;
>>>>>>
>>>>>>
>>>>>> same here. status = "disabled";
>>>>>>
>>>>>>> + };
>>>>>>> };
>>>>>>>
>>>>>
>>>>> Hi David,
>>>>>
>>>>> I did that initially[1][2] and it was rejected by Kevin[3] and
>>>>> Laurent[4].
>>>>>
>>>>> FYI this patch has already been queued by Sekhar.
>>>>
>>>> Thanks. I did not see those threads.
>>>>
>>>> FYI to maintainers, having these enabled by default causes error
>>>> messages in the kernel log for other boards that are not supported by
>>>> the drivers.
>>>
>>> Then the driver is too noisy and should be cleaned up.
>>>
>>>> Since there is only one board that is supported and soon
>>>> to be 2 that are not, I would rather have this disabled by default to
>>>> avoid the error messages.
>>>
>>> IMO, what exactly are the error messages? Sounds like the driver is
>>> being too verbose, and calling things errors that are not really errors.
>>
>> It is just one line per driver.
>>
>> dev_err(dev, "no master priorities defined for this board\n");
>>
>> and
>>
>> dev_err(dev, "no settings defined for this board\n");
>>
>>
>> Since "ti,da850-lcdk" is the only board supported in these drivers, all
>> other boards will see these error messages.
>
> Thats pretty bad. Sorry about that. The original justification for
> keeping them enabled all the time was that they are in-SoC modules with
> no external dependencies (like IO lines or voltage rails) so they can be
> enabled on all boards that use DA850. While that remains true, the
> configuration itself is board specific.
>
> I think the error messages are still useful, so instead of silencing
> them, I think we should go back to keeping these nodes disabled by
> default and enabling only on boards which have support for it in the driver.
>
> Thanks,
> Sekhar
I'll send a patch.
Thanks,
Bartosz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/7] add binding for stm32 multifunctions timer driver
From: Lee Jones @ 2016-11-24 8:52 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List, Thierry Reding,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks6rnago88JC0C5Uj6JpGZGuwyoOj-W8+r7Oj4s8_GuXyA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Rob,
Would you mind casting an eye on this please?
On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> 2016-11-23 10:21 GMT+01:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> > On Wed, 23 Nov 2016, Benjamin Gaignard wrote:
> >
> >> 2016-11-22 17:52 GMT+01:00 Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> >> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >> >
> >> >> Add bindings information for stm32 timer MFD
> >> >>
> >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> >> >> ---
> >> >> .../devicetree/bindings/mfd/stm32-timer.txt | 53 ++++++++++++++++++++++
> >> >> 1 file changed, 53 insertions(+)
> >> >> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> new file mode 100644
> >> >> index 0000000..3cefce1
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> >> @@ -0,0 +1,53 @@
> >> >> +STM32 multifunctions timer driver
> >> >
> >> > "STM32 Multi-Function Timer/PWM device bindings"
> >> >
> >> > Doesn't this shared device have a better name?
> >>
> >> In SoC documentation those hardware blocks are named "advanced-control
> >> timers", "general purpose timers" or "basic timers"
> >> "stm32-timer" name is already used for clock source driver, that why I
> >> have prefix it with mfd
> >
> > MFD is a Linuxisum and has no place in hardware description.
> >
> > Please used one of the names you mentioned above.
>
> I will go for "st,stm32-advanced-timer"
>
> >
> > Hopefully the one that best fits.
> >
> >> >> +stm32 timer MFD allow to handle at the same time pwm and IIO timer devices
> >> >
> >> > No need for this sentence.
> >> >
> >> OK
> >>
> >> >> +Required parameters:
> >> >> +- compatible: must be one of the follow value:
> >> >> + "st,stm32-mfd-timer1"
> >> >> + "st,stm32-mfd-timer2"
> >> >> + "st,stm32-mfd-timer3"
> >> >> + "st,stm32-mfd-timer4"
> >> >> + "st,stm32-mfd-timer5"
> >> >> + "st,stm32-mfd-timer6"
> >> >> + "st,stm32-mfd-timer7"
> >> >> + "st,stm32-mfd-timer8"
> >> >> + "st,stm32-mfd-timer9"
> >> >> + "st,stm32-mfd-timer10"
> >> >> + "st,stm32-mfd-timer11"
> >> >> + "st,stm32-mfd-timer12"
> >> >> + "st,stm32-mfd-timer13"
> >> >> + "st,stm32-mfd-timer14"
> >> >
> >> > We don't normally number devices.
> >> >
> >> > What's stopping you from simply doing:
> >> >
> >> > pwm1: pwm1@40010000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm2: pwm1@40020000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> > pwm3: pwm1@40030000 {
> >> > compatible = "st,stm32-pwm";
> >> > };
> >> >
> >>
> >> Because each instance of the hardware is slightly different: number of
> >> pwm channels, triggers capabilities, etc ..
> >> so I need to distinguish them.
> >> Since it look to be a problem I will follow your suggestion and add a
> >> property this driver to be able to identify each instance.
> >> Do you think that "id" parameter (integer for 1 to 14) is acceptable ?
> >
> > Unfortunately not. IDs aren't allowed in DT.
> >
> > What about "pwm-chans" and "trigger"?
> >
> > pwm-chans : Number of available channels available
>
> For pwm I need those 4 properties:
> st,pwm-number: the number of PWM devices
st,pwm-num-chan is already documented.
Please use that instead of creating new properties.
> st,complementary: if exist have complementary ouput
> st,32bit-counter: if exist have 32 bits counter
> st,breakinput-polarity: if set enable break input feature.
>
> Is it acceptable from pwm maintainer point of view ?
>
> > trigger : Boolean value specifying whether a timer is present
>
> Following our discussion on IRC I will try to code for your proposal:
>
> advanced-timer@40010000 {
> compatible = "st,stm32-advanced-timer";
> reg = <0x40010000 0x400>;
> clocks = <&rcc 0 160>;
> clock-names = "clk_int";
>
> pwm@0 {
> compatible = "st,stm32-pwm";
> st,pwm-number= <4>;
> st,complementary;
> st,breakinput;
> };
>
> timer@0 {
> reg = <1>;
> compatible = "st,stm32-iio-timer";
> interrupts = <27>;
> triggers = <5 2 3 4>;
> };
> };
>
> triggers parameter will be used to know which trigger are valid for
> the IIO device
Except for "st,pwm-number" as mentioned above, this looks good to me.
Rob, would what do you think?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] mfd: cpcap: Add minimal support
From: Lee Jones @ 2016-11-24 8:59 UTC (permalink / raw)
To: Tony Lindgren
Cc: Samuel Ortiz, linux-kernel, linux-omap, devicetree, Marcel Partap,
Mark Rutland, Michael Scott, Rob Herring
In-Reply-To: <20161123152652.GB4082@atomide.com>
On Wed, 23 Nov 2016, Tony Lindgren wrote:
> * Lee Jones <lee.jones@linaro.org> [161121 03:43]:
> > On Fri, 18 Nov 2016, Tony Lindgren wrote:
> > > --- a/drivers/mfd/Makefile
> > > +++ b/drivers/mfd/Makefile
> > > @@ -97,6 +97,7 @@ obj-$(CONFIG_MFD_MC13XXX_I2C) += mc13xxx-i2c.o
> > > obj-$(CONFIG_MFD_CORE) += mfd-core.o
> > >
> > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o
> > > +obj-$(CONFIG_MFD_CPCAP) += cpcap.o
> >
> > Who is the manufacturer?
>
> Hmm that I don't know. There seems to be both ST and TI versions
> of this chip manufactured for Motorola. So my guess is that it
> should be Motorola unless there's some similar catalog part
> available from ST used by others. If anybody has more info
> on this please let me know :)
If this IP is shared amongst vendors, it usually means it was designed
by someone else? Synopsis perhaps?
> > > + cpcap->vendor = (val >> 6) & 0x0007;
> > > + cpcap->revision = ((val >> 3) & 0x0007) | ((val << 3) & 0x0038);
> >
> > Lots of magic numbers here. I suggest you define them.
>
> I'll check if some earlier code has these defined. Otherwise I'll
> just add a comment on the lack of available documentation.
*sad face*
Does that mean you don't even know what they're for?
> > > + error = cpcap_init_irq_bank(cpcap, 0, 0, 16);
> >
> > 'ret' is more traditional.
>
> FYI error seems to be preferred over ret as it's meaning is
> clear, git grep "error =" drivers/input for example.
> I can of course change it if you prefer ret over error.
I'd prefer to stick to the conventions of *this* subsystem.
... and the most common convention used kernel wide:
$ git grep "ret =" | wc -l
117976
$ git grep "err =" | wc -l
56708
$ git grep "error =" | wc -l
14427
> > > + error = cpcap_init_irq_bank(cpcap, 2, 32, 64);
> > > + if (error)
> > > + return error;
> >
> > I don't think I've seen this method of adding bulk IRQ chips before.
> > Isn't there a cleaner or generic way to do this?
>
> I'll check.
>
> ...
> > > +#define CPCAP_REG_LDEB 0x1270 /* LMR Debounce Settings */
> > > +#define CPCAP_REG_LGDET 0x1274 /* LMR GCAI Detach Detect */
> > > +#define CPCAP_REG_LMISC 0x1278 /* LMR Misc Bits */
> > > +#define CPCAP_REG_LMACE 0x127c /* LMR Mace IC Support */
> > > +
> > > +#define CPCAP_REG_TEST 0x7c00 /* Test */
> > > +
> > > +#define CPCAP_REG_ST_TEST1 0x7d08 /* ST Test1 */
> > > +
> > > +#define CPCAP_REG_ST_TEST2 0x7d18 /* ST Test2 */
> >
> > It would be nice to line up the entire file. #OCD
>
> Hmm care to clarify what you mean here? I think it's lined up with
I'm missing context now you've <snip>ed.
These look straight, however is the whole file lined up (as much as
*practically* possible)?
> tabs to line up. I left empty lines where the registers are not
> contiguous. What does #OCD mean, Obsessive Compulsive Disorder over
> header files maybe? :)
Yes, that's what it means.
/me likes straight lines. :)
> Anywys thanks for the review, the rest of the comments I will just
> fix and repost.
Welcome.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ulf Hansson @ 2016-11-24 9:05 UTC (permalink / raw)
To: Gregory CLEMENT, Rob Herring
Cc: Ziji Hu, Adrian Hunter, linux-mmc@vger.kernel.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, devicetree@vger.kernel.org,
Thomas Petazzoni, linux-arm-kernel@lists.infradead.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu
In-Reply-To: <87d1hno2d7.fsf@free-electrons.com>
On 22 November 2016 at 18:23, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Rob,
>
> On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
>
> [...]
>
>>>> +
>>>> +- reg:
>>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>>> +
>>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>>> + PHY PAD Voltage Control register.
>>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>>> + in below.
>>>> + Please also check property marvell,pad-type in below.
>>>> +
>>>> +Optional Properties:
>>>> +- marvell,xenon-slotno:
>>>
>>> Multiple slots should be represented as child nodes IMO. I think some
>>> other bindings already do this.
>>>
>>
>> All the slots are entirely independent.
>> I prefer to consider it as multiple independent SDHCs placed in
>> a single IP, instead of that a IP contains multiple child slots.
>
> It was indeed what I tried to show in my answer for the 1st version:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
>
> Maybe you missed it.
>
> You also mentioned other bindings using child nodes, but for this one
> we have one controller with only one set of register with multiple slots
> (Atmel is an example). Here each slot have it own set of register.
>
> Actually giving the fact that each slot is controlled by a different set
> of register I wonder why the hardware can't also deduce the slot number
> from the address register. For me it looks like an hardware bug but we
> have to deal with it.
>
> Do you still think we needchild node here?
Using child-nodes for slots like what's done in the atmel case, is
currently broken. I would recommend to avoid using child-nodes for
slots, if possible.
To give you some more background, currently the mmc core treats child
nodes as embedded non-removable cards or SDIO funcs. However, we can
change to make child-nodes also allowed to describe slots, but it
requires a specific compatible for "slots" and of course then we also
need to update the DT parsing of the child-nodes in the mmc core.
Documentation/devicetree/bindings/mmc/mmc.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt
>
>>
>> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
>> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
>> In my very own opinion, it is inconvenient and unnecessary.
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Arnd Bergmann @ 2016-11-24 9:11 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Rob Herring, Ziji Hu, Adrian Hunter,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Vi
In-Reply-To: <CAPDyKFpoifsKkse7Fc-bbZAoa=QGT=9QOQ-4D=f60ptx0hzZsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thursday, November 24, 2016 10:05:45 AM CET Ulf Hansson wrote:
> > You also mentioned other bindings using child nodes, but for this one
> > we have one controller with only one set of register with multiple slots
> > (Atmel is an example). Here each slot have it own set of register.
> >
> > Actually giving the fact that each slot is controlled by a different set
> > of register I wonder why the hardware can't also deduce the slot number
> > from the address register. For me it looks like an hardware bug but we
> > have to deal with it.
> >
> > Do you still think we needchild node here?
>
> Using child-nodes for slots like what's done in the atmel case, is
> currently broken. I would recommend to avoid using child-nodes for
> slots, if possible.
>
> To give you some more background, currently the mmc core treats child
> nodes as embedded non-removable cards or SDIO funcs. However, we can
> change to make child-nodes also allowed to describe slots, but it
> requires a specific compatible for "slots" and of course then we also
> need to update the DT parsing of the child-nodes in the mmc core.
>
> Documentation/devicetree/bindings/mmc/mmc.txt
> Documentation/devicetree/bindings/mmc/mmc-card.txt
I don't see anything wrong with having child nodes for the slots
even with the current binding, under one condition:
The mmc.txt binding above must refer only to the child node, while
the parent node conceptually becomes a plain bus or MFD that
happens to encapsulate multiple MMC host controllers, and possibly
provides some shared registers to them.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-11-24 9:12 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Gabriele Paoloni, mark.rutland@arm.com, catalin.marinas@arm.com,
linux-pci@vger.kernel.org, liviu.dudau@arm.com, Linuxarm,
lorenzo.pieralisi@arm.com, xuwei (O), Jason Gunthorpe,
T homas Petazzoni, linux-serial@vger.kernel.org,
benh@kernel.crashing.org, devicetree@vger.kernel.org,
minyard@acm.org, will.deacon@arm.com, John Garry
In-Reply-To: <4675465.4Qhqy6WU4X@wuerfel>
Hi, Arnd,
Thanks you very much!
To understand your idea more clear, I have some questions on your patch sketch.
Please check it below.
On 2016/11/24 7:23, Arnd Bergmann wrote:
> On Wednesday, November 23, 2016 6:07:11 PM CET Arnd Bergmann wrote:
>> On Wednesday, November 23, 2016 3:22:33 PM CET Gabriele Paoloni wrote:
>>> From: Arnd Bergmann [mailto:arnd@arndb.de]
>>>> On Friday, November 18, 2016 5:03:11 PM CET Gabriele Paoloni wrote:
>>
>> Please don't proliferate the use of
>> pci_pio_to_address/pci_address_to_pio here, computing the physical
>> address from the logical address is trivial, you just need to
>> subtract the start of the range that you already use when matching
>> the port number range.
>>
>> The only thing we need here is to make of_address_to_resource()
>> return the correct logical port number that was registered for
>> a given host device when asked to translate an address that
>> does not have a CPU address associated with it.
>
> Ok, I admit this was a little harder than I expected, but see below
> for a rough outline of how I think it can be done.
>
> This makes it possible to translate bus specific I/O port numbers
> from device nodes into Linux port numbers, and gives a way to register
> them. We could take this further and completely remove pci_pio_to_address
> and pci_address_to_pio if we make the I/O port translation always
> go through the io_range list, looking up up the hostbridge by fwnode,
> but we don't have to do that now.
>
> The patch is completely untested and probably buggy, it just seemed
> easier to put out a prototype than to keep going in circles with the
> discussion.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index bf601d4df8cf..6cadf0501bb0 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -730,7 +730,8 @@ static void acpi_pci_root_validate_resources(struct device *dev,
> }
> }
>
> -static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> +static void acpi_pci_root_remap_iospace(struct fwnode_handle *node,
> + struct resource_entry *entry)
> {
> #ifdef PCI_IOBASE
> struct resource *res = entry->res;
> @@ -739,11 +740,7 @@ static void acpi_pci_root_remap_iospace(struct resource_entry *entry)
> resource_size_t length = resource_size(res);
> unsigned long port;
>
> - if (pci_register_io_range(cpu_addr, length))
> - goto err;
> -
> - port = pci_address_to_pio(cpu_addr);
> - if (port == (unsigned long)-1)
> + if (pci_register_io_range(node, cpu_addr, length, &port))
> goto err;
>
> res->start = port;
> @@ -781,7 +778,8 @@ int acpi_pci_probe_root_resources(struct acpi_pci_root_info *info)
> else {
> resource_list_for_each_entry_safe(entry, tmp, list) {
> if (entry->res->flags & IORESOURCE_IO)
> - acpi_pci_root_remap_iospace(entry);
> + acpi_pci_root_remap_iospace(&device->fwnode,
> + entry);
>
> if (entry->res->flags & IORESOURCE_DISABLED)
> resource_list_destroy_entry(entry);
I think those changes in pci_root.c is only to match the new definition of
pci_register_io_range() and work for PCI I/O. It doesn't make sense for LPC, is
it right?
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index a50025a3777f..df96955a43f8 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -760,8 +760,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
> set_bit(NBD_RUNNING, &nbd->runtime_flags);
> blk_mq_update_nr_hw_queues(&nbd->tag_set, nbd->num_connections);
> args = kcalloc(num_connections, sizeof(*args), GFP_KERNEL);
> - if (!args)
> + if (!args) {
> + error = -ENOMEM;
> goto out_err;
> + }
> nbd->task_recv = current;
> mutex_unlock(&nbd->config_lock);
>
I think change here is none of the business.:)
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..5decaba96eed 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -2,6 +2,7 @@
> #define pr_fmt(fmt) "OF: " fmt
>
> #include <linux/device.h>
> +#include <linux/fwnode.h>
> #include <linux/io.h>
> #include <linux/ioport.h>
> #include <linux/module.h>
> @@ -323,14 +324,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
>
> if (res->flags & IORESOURCE_IO) {
> unsigned long port;
> - err = pci_register_io_range(range->cpu_addr, range->size);
> + err = pci_register_io_range(&np->fwnode, range->cpu_addr, range->size, &port);
> if (err)
> goto invalid_range;
> - port = pci_address_to_pio(range->cpu_addr);
> - if (port == (unsigned long)-1) {
> - err = -EINVAL;
> - goto invalid_range;
> - }
> res->start = port;
> } else {
> if ((sizeof(resource_size_t) < 8) &&
> @@ -479,7 +475,7 @@ static int of_empty_ranges_quirk(struct device_node *np)
> return false;
> }
>
> -static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> +static u64 of_translate_one(struct device_node *parent, struct of_bus *bus,
> struct of_bus *pbus, __be32 *addr,
> int na, int ns, int pna, const char *rprop)
> {
> @@ -507,7 +503,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> ranges = of_get_property(parent, rprop, &rlen);
> if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> pr_debug("no ranges; cannot translate\n");
> - return 1;
> + return OF_BAD_ADDR;
> }
> if (ranges == NULL || rlen == 0) {
> offset = of_read_number(addr, na);
> @@ -528,7 +524,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> }
> if (offset == OF_BAD_ADDR) {
> pr_debug("not found !\n");
> - return 1;
> + return offset;
> }
> memcpy(addr, ranges + na, 4 * pna);
>
> @@ -537,7 +533,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> pr_debug("with offset: %llx\n", (unsigned long long)offset);
>
> /* Translate it into parent bus space */
> - return pbus->translate(addr, offset, pna);
> + if (pbus->translate(addr, offset, pna))
> + return OF_BAD_ADDR;
> +
> + return offset;
> }
>
> /*
> @@ -549,9 +548,14 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> * that translation is impossible (that is we are not dealing with a value
> * that can be mapped to a cpu physical address). This is not really specified
> * that way, but this is traditionally the way IBM at least do things
> + *
> + * Whenever the translation fails, the *host pointer will be set to the
> + * device that lacks a tranlation, and the return code is relative to
> + * that node.
> */
> static u64 __of_translate_address(struct device_node *dev,
> - const __be32 *in_addr, const char *rprop)
> + const __be32 *in_addr, const char *rprop,
> + struct device_node **host)
> {
> struct device_node *parent = NULL;
> struct of_bus *bus, *pbus;
> @@ -564,6 +568,7 @@ static u64 __of_translate_address(struct device_node *dev,
> /* Increase refcount at current level */
> of_node_get(dev);
>
> + *host = NULL;
> /* Get parent & match bus type */
> parent = of_get_parent(dev);
> if (parent == NULL)
> @@ -600,8 +605,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus = of_match_bus(parent);
> pbus->count_cells(dev, &pna, &pns);
> if (!OF_CHECK_COUNTS(pna, pns)) {
> - pr_err("Bad cell count for %s\n",
> - of_node_full_name(dev));
> + pr_debug("Bad cell count for %s\n",
> + of_node_full_name(dev));
> + *host = of_node_get(parent);
> break;
> }
I don't think here is the right place to fill *host. I think you want to return
the parent where the of_translate_one() failed for the 'ranges' property
missing. So, I think this seems better:
if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) {
*host = of_node_get(dev);
break;
}
>
> @@ -609,7 +615,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus->name, pna, pns, of_node_full_name(parent));
>
> /* Apply bus translation */
> - if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop))
> + result = of_translate_one(dev, bus, pbus, addr, na, ns,
> + pna, rprop);
> + if (result == OF_BAD_ADDR)
> break;
>
> /* Complete the move up one level */
> @@ -628,13 +636,32 @@ static u64 __of_translate_address(struct device_node *dev,
>
> u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_address);
>
> u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
> {
> - return __of_translate_address(dev, in_addr, "dma-ranges");
> + struct device_node *host;
> + u64 ret;
> +
> + ret = __of_translate_address(dev, in_addr, "dma-ranges", &host);
> +
> + if (host) {
> + of_node_put(host);
> + return OF_BAD_ADDR;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL(of_translate_dma_address);
>
> @@ -676,29 +703,48 @@ const __be32 *of_get_address(struct device_node *dev, int index, u64 *size,
> }
> EXPORT_SYMBOL(of_get_address);
>
> +extern unsigned long extio_translate(struct fwnode_handle *node, unsigned long offset);
> +
> +u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr)
> +{
> + u64 taddr;
> + unsigned long port;
> + struct device_node *host;
> +
> + taddr = __of_translate_address(dev, in_addr, "ranges", &host);
> + if (host) {
> + /* host specific port access */
> + port = extio_translate(&host->fwnode, taddr);
> + of_node_put(host);
> + } else {
> + /* memory mapped I/O range */
> + port = pci_address_to_pio(taddr);
> + if (port == (unsigned long)-1)
> + return OF_BAD_ADDR;
> + }
> +
> + return port;
> +}
> +
> static int __of_address_to_resource(struct device_node *dev,
> const __be32 *addrp, u64 size, unsigned int flags,
> const char *name, struct resource *r)
> {
> u64 taddr;
>
> - if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
> + if (flags & IORESOURCE_MEM)
> + taddr = of_translate_address(dev, addrp);
> + else if (flags & IORESOURCE_IO)
> + taddr = of_translate_ioport(dev, addrp);
> + else
> return -EINVAL;
> - taddr = of_translate_address(dev, addrp);
> +
> if (taddr == OF_BAD_ADDR)
> return -EINVAL;
> memset(r, 0, sizeof(struct resource));
> - if (flags & IORESOURCE_IO) {
> - unsigned long port;
> - port = pci_address_to_pio(taddr);
> - if (port == (unsigned long)-1)
> - return -EINVAL;
> - r->start = port;
> - r->end = port + size - 1;
> - } else {
> - r->start = taddr;
> - r->end = taddr + size - 1;
> - }
> +
> + r->start = taddr;
> + r->end = taddr + size - 1;
> r->flags = flags;
> r->name = name ? name : dev->full_name;
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index eda6a7cf0e54..320ab9fbf6af 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3249,6 +3249,7 @@ EXPORT_SYMBOL(pci_request_regions_exclusive);
> #ifdef PCI_IOBASE
> struct io_range {
> struct list_head list;
> + struct fwnode_handle *node;
> phys_addr_t start;
> resource_size_t size;
> };
> @@ -3257,11 +3258,14 @@ static LIST_HEAD(io_range_list);
> static DEFINE_SPINLOCK(io_range_lock);
> #endif
>
> +#define IO_RANGE_IOEXT (resource_size_t)(-1ull)
> +
> /*
> * Record the PCI IO range (expressed as CPU physical address + size).
> * Return a negative value if an error has occured, zero otherwise
> */
> -int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> +int __weak pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port)
> {
> int err = 0;
>
> @@ -3272,7 +3276,12 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> /* check if the range hasn't been previously recorded */
> spin_lock(&io_range_lock);
> list_for_each_entry(range, &io_range_list, list) {
> - if (addr >= range->start && addr + size <= range->start + size) {
> + if (node == range->node)
> + goto end_register;
> +
I don't think it is safe to only check the node had been registered. For
PCI/PCIE, there is only one I/O windows in bridge, it seems ok. But for non-pci
devices, such as ISA/LPC, I wonder it is possible there are several disjoint I/O
'ranges' entries...
What parameters are necessary for linux PIO allocation?
1) For those bus devices which have no MMIO( that is to say, indirectIO is
using), I think 'addr' is not needed, but 'size' is mandatory;
I am thinking for our LPC, as there is no cpu address, we should not input
'addr' for the io range register. With 'size' as parameter, we implement a new
io range register function where can assign an unique linux PIO for this
register calling. The output linux PIO can allocate from a sub-range of whole
I/O space of [0, IO_SPACE_LIMIT]. This sub-range is specific for indirectIO, I
want to define a new macro, such as EXTIO_LIMIT, to represent the upper limit of
indirect IO space.
#if defined(CONFIG_PCI) && defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT PCIBIOS_MIN_IO
#elif defined(CONFIG_INDIRECT_PIO)
#define EXTIO_LIMIT 0x1000
#else
#define EXTIO_LIMIT 0x00
#end
We should do some checkings to ensure EXTIO_LIMIT < IO_SPACE_LIMIT.
Then when someone call pci_register_io_range() or a new function for the linux
PIO register, we can allocate linux PIO from [0, EXTIO_LIMIT) for indirectIO
bus, from [EXTIO_LIMIT, IO_SPACE_LIMIT] for MMIO;
But there are issues confused me yet. For example, how to know the IO size for
the indirectIO bus? You known, there is no 'ranges' property for those buses....
2) For PCI MMIO, I think 'addr' is needed
As for the current pci_register_io_range()/pci_address_to_pio(), I have two doubts:
2.1) If there are multiple PCI host bridges which support I/O transaction, I
wonder whether the first host bridge can access the downstream devices with bus
I/O address in [0, PCIBIOS_MIN_IO)
for the first host bridge, pci_address_to_pio() will return a linux PIO range
start from 0.
But when calling __pci_assign_resource() to allocate the linux PIO for PCI/PCIE
devices/buses which are just children of first host bus, it can not allocate
linux PIO less than PCIBIOS_MIN_IO, which means kernel can not called in/out()
with port less than PCIBIOS_MIN_IO. But we had ioremap [PCI_IOBASE + 0,
PCI_IOBASE + size) to [pci_ioadd, pci_ioadd + size) before.
static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
struct resource *res = dev->resource + resno;
resource_size_t min;
int ret;
min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
and in the later function:
static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
resource_size_t size, resource_size_t align,
resource_size_t min, unsigned long type_mask,
resource_size_t (*alignf)(void *,
....
pci_bus_for_each_resource(bus, r, i) {
resource_size_t min_used = min;
....
if (avail.start)
min_used = avail.start;
max = avail.end;
/* Ok, try it out.. */
ret = allocate_resource(r, res, size, min_used, max,
align, alignf, alignf_data);
After allocate_resource(), a IO resource is allocated, but whose 'start' is not
less than min_used.( since avail.start is 0, min_used will keep the 'min'
without change to avail.start; Should be PCIBIOS_MIN_IO).
2.2) Is it possible the return linux PIO isn't page-aligned?
When calling pci_remap_iospace(const struct resource *res, phys_addr_t
phys_addr), if res->start is not page-aligned, it seems that
ioremap_page_range() will meet some issues for duplication iorempa for same
virtual page.
of-course, if we always configure the I/O ranges size as page-aligned, it will
be OK.
I found PowerPC will ensure the 'vaddress' and the 'size' are page-aligned
before ioremap, do we need to improve the current handling in
pci_register_io_range/pci_address_to_pio?
Thanks,
Zhichang
> + if (addr != IO_RANGE_IOEXT &&
> + addr >= range->start &&
> + addr + size <= range->start + size) {
> /* range already registered, bail out */
> goto end_register;
> }
> @@ -3298,6 +3307,7 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
> goto end_register;
> }
>
> + range->node = node;
> range->start = addr;
> range->size = size;
>
> @@ -3305,11 +3315,26 @@ int __weak pci_register_io_range(phys_addr_t addr, resource_size_t size)
>
> end_register:
> spin_unlock(&io_range_lock);
> +
> + *port = allocated_size;
> +#else
> + /*
> + * powerpc and microblaze have their own registration,
> + * just look up the value here
> + */
> + *port = pci_address_to_pio(addr);
> #endif
>
> return err;
> }
>
> +#ifdef CONFIG_IOEXT
> +int ioext_register_io_range
> +{
> + return pci_register_io_range(node, IO_RANGE_IOEXT, size, port);
> +}
> +#endif
> +
> phys_addr_t pci_pio_to_address(unsigned long pio)
> {
> phys_addr_t address = (phys_addr_t)OF_BAD_ADDR;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6bd94a803e8f..b7a8fa3da3ca 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1192,7 +1192,8 @@ int __must_check pci_bus_alloc_resource(struct pci_bus *bus,
> void *alignf_data);
>
>
> -int pci_register_io_range(phys_addr_t addr, resource_size_t size);
> +int pci_register_io_range(struct fwnode_handle *node, phys_addr_t addr,
> + resource_size_t size, unsigned long *port);
> unsigned long pci_address_to_pio(phys_addr_t addr);
> phys_addr_t pci_pio_to_address(unsigned long pio);
> int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
>
>
> .
>
^ permalink raw reply
* Re: [RFC PATCH 2/5] dmaengine: allow sun6i-dma for more SoCs
From: Andre Przywara @ 2016-11-24 9:16 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Maxime Ripard, Icenowy Zheng, linux-sunxi, linux-arm-kernel,
Mark Rutland, Rob Herring, devicetree
In-Reply-To: <CAGb2v67M8DrPaf8GzSPEjekgV6cLcXXzO3tVUc9kjUDcM3BE_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi,
On 24/11/16 04:16, Chen-Yu Tsai wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 9:17 AM, Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org> wrote:
>> The sun6i DMA driver is used in the Allwinner A64 and H5 SoC, which
>> have arm64 capable cores. Add the generic sunxi config symbol to allow
>> the driver to be selected by arm64 Kconfigs, which don't feature
>> SoC specific MACH_xxxx configs.
>>
>> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
>> ---
>> drivers/dma/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index af63a6b..003c284 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -157,7 +157,7 @@ config DMA_SUN4I
>>
>> config DMA_SUN6I
>> tristate "Allwinner A31 SoCs DMA support"
>> - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
>> + depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST || ARCH_SUNXI
>
> AFAIK ARCH_SUNXI encompasses/supersedes MACH_SUN*I.
> (And I don't have to add MACH_SUN9I later :) )
Sure, admittedly it was just a quick hack to get things going.
Actually I don't know why we had a *depend* on those MACH_s before. I
think technically it does not depend on a certain SoC (having the
COMPILE_TEST in there hints on that). So what about:
depends on ARCH_SUNXI || COMPILE_TEST
and maybe:
default y if MACH_SUN6I || MACH_SUN8I
Though I see that both multi_v7_defconfig and sunxi_defconfig explicitly
set this, so this wouldn't be needed?
Cheers,
Andre.
^ 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