Re: [PATCH V3 1/3] dts: change Marvell prefix to 'marvell'
On Fri, Jul 12, 2013 at 11:10 PM, Daniel Drake wrote: > On Thu, Jul 11, 2013 at 5:54 PM, Haojian Zhuang > wrote: >>> Well, Daniel Drake spoke up for OLPC. Does that count? >> >> We don't know they used DT on Marvell MMP2/MMP3. So they don't have DTS file >> in kernel, we could use both old name & new name in driver. > > You are listed as one of the MMP maintainers in the MAINTAINERS file > and I have sent you several patches in the few 3 weeks which make > OLPC's usage of MMP + DT pretty obvious. As a maintainer I believe you > are supposed to review the patches too. hint hint ;) > These patches couldn't be applied. Since we're moving irq drivers from arch directories to irq directories. When the irq patches are applied, you can rebase your patches. Regards Haojian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Fixed PHY Device Tree usage?
Le 13/07/2013 00:44, Grant Likely a écrit :
On Wed, Jul 10, 2013 at 6:23 PM, Florian Fainelli wrote:
Hello Thomas,
2013/7/10 Thomas Petazzoni :
Dear Florian Fainelli,
On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:
[snip]
};
phy1: ethernet-phy@1 {
... all the properties you listed ...
... maybe the "id" property is not needed
because of the phandle ...
};
};
soc {
ethernet@0 {
phy = <&phy0>;
...
};
ethernet@1 {
phy = <&phy1>;
...
};
};
or do you have in mind another representation?
Not really this is more or less what I had in mind. I am wondering
whether we should really declare the "mdio-fixed" node, or if we
should not rather make the following:
- declare all PHY nodes in the system as sub nodes of their belonging
real hardware MDIO bus node
- flag specific PHY nodes as "fixed" with a "fixed-link" boolean for instance
- if we see that flag, make that specific PHY node bind to the
fixed-phy driver instead
So the fixed PHY driver is going to travel through *all* nodes of the
DT, and whenever some random node has a "fixed" property, it's going to
say it corresponds to a fixed PHY? That doesn't seem like a good idea.
Why not? Since we are already have to scan the entire MDIO bus we are
attached to, when we encounter such a PHY node with the special
"fixed" properties, we just call fixed_phy_add() with the right
parameters and voila. Which is also the reason why I was suggesting to
put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
we have this logic only in one place.
Hi Florian,
I think this discussion is going in the wrong direction. The concept
of a dummy phy is really a Linux kernel internal detail. Creating some
kind of dummy MDIO bus node does not describe the hardware. There is
already support in the kernel for Ethernet MACs connected directly to
a switch or other device. It is far better to describe how the MAC
needs to be configured than to invent a non-existent phy. Search for
"fixed-link" in the kernel tree to see how it is used.
Errm, fixed-link is deprecated according to the comment which parses it.
In fact, the code parsing this special property does not parse all the
integers representing the fixed-link. But fair enough, if that is the
way to go, then let's stick with it.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Fixed PHY Device Tree usage?
On Wed, Jul 10, 2013 at 6:23 PM, Florian Fainelli wrote:
> Hello Thomas,
>
> 2013/7/10 Thomas Petazzoni :
>> Dear Florian Fainelli,
>>
>> On Wed, 10 Jul 2013 17:29:44 +0100, Florian Fainelli wrote:
> [snip]
>
>>
>>> > };
>>> >
>>> > phy1: ethernet-phy@1 {
>>> > ... all the properties you listed ...
>>> > ... maybe the "id" property is not needed
>>> > because of the phandle ...
>>> > };
>>> > };
>>> >
>>> > soc {
>>> > ethernet@0 {
>>> > phy = <&phy0>;
>>> > ...
>>> > };
>>> >
>>> > ethernet@1 {
>>> > phy = <&phy1>;
>>> > ...
>>> > };
>>> > };
>>> >
>>> > or do you have in mind another representation?
>>>
>>> Not really this is more or less what I had in mind. I am wondering
>>> whether we should really declare the "mdio-fixed" node, or if we
>>> should not rather make the following:
>>>
>>> - declare all PHY nodes in the system as sub nodes of their belonging
>>> real hardware MDIO bus node
>>> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for
>>> instance
>>> - if we see that flag, make that specific PHY node bind to the
>>> fixed-phy driver instead
>>
>> So the fixed PHY driver is going to travel through *all* nodes of the
>> DT, and whenever some random node has a "fixed" property, it's going to
>> say it corresponds to a fixed PHY? That doesn't seem like a good idea.
>
> Why not? Since we are already have to scan the entire MDIO bus we are
> attached to, when we encounter such a PHY node with the special
> "fixed" properties, we just call fixed_phy_add() with the right
> parameters and voila. Which is also the reason why I was suggesting to
> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
> we have this logic only in one place.
Hi Florian,
I think this discussion is going in the wrong direction. The concept
of a dummy phy is really a Linux kernel internal detail. Creating some
kind of dummy MDIO bus node does not describe the hardware. There is
already support in the kernel for Ethernet MACs connected directly to
a switch or other device. It is far better to describe how the MAC
needs to be configured than to invent a non-existent phy. Search for
"fixed-link" in the kernel tree to see how it is used.
g.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Sharing *.dtsi between Linux architectures?
On Fri, Jul 12, 2013 at 01:58:35PM -0600, Stephen Warren wrote: > Is there a (possibly just proposed) mechanism in place to allow *.dts > from multiple Linux architectures to share common *.dtsi files? Don't forget that the long term goal is to move these files out of the kernel source, which means that they'll be a separately managed project, possibly with a different file structure. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] gps6507x.txt: Remove executable bits
On 07/12/2013 12:31 PM, Joe Perches wrote: > Documentation shouldn't be executable. > > Signed-off-by: Joe Perches Acked-by: Rob Herring > --- > diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt > b/Documentation/devicetree/bindings/mfd/tps6507x.txt > old mode 100755 > new mode 100644 > > ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Sharing *.dtsi between Linux architectures?
On Jul 13, 2013, at 4:17 AM, Rob Herring wrote: > On 07/12/2013 02:58 PM, Stephen Warren wrote: >> Is there a (possibly just proposed) mechanism in place to allow *.dts >> from multiple Linux architectures to share common *.dtsi files? > > Nothing proposed yet. There was some discussion at Connect (which I > missed part of). We're certainly going to start to have that issue > between arm and arm64 as well as probably FSL powerpc and arm. > > I would like to move all dts files out of arch. I think we should think > about how we construct a separate dts repository and then move the > kernel structure in that direction (it's still believed there is too > much interdependency to have separate repo yet). I don't think cpu > architecture is the right top level structure for dts files. Probably > something by vendor and/or SOC family is more appropriate. Then you have > to figure out how to handle board vs. chip vendors. > >> As an example, consider two SoCs that are identical except for the CPU >> complex. One uses an ARMv7 CPU (DTs in arch/arm/boot/dts/) and the other >> uses some ARMv8 CPU (DTs in arch/am64/boot/dts/). It'd be useful to >> define all the SoC components in some common .dtsi file to avoid >> duplication, and have both arch/arm/boot/dts/tegraXXX.dtsi and >> arch/arm64/boot/dts/tegraYYY.dtsi include that and add the relevant >> CPU-related nodes. >> >> I could imagine creating one of the following paths for this purpose: >> >> arch/common/dts/ >> include/dt-common/ >> include/dtsi/ >> >> ... or perhaps re-using the existing: >> >> include/dt-bindings/ >> >> ... although my original intent for that last location was just to house >> header files that define constants that are part of binding definitions, >> rather than actual structural content. > > I think we should stick with defines there. I think it's to do it how move the dt simple in dt directory at the root of the kernel tree this will simplify the move to out of tree and include/dt-bindings/ to dt/include/dt-bindings/ and just handle the include via a -i in the Makefile Best Regards, J. > > Rob > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Sharing *.dtsi between Linux architectures?
On 07/12/2013 02:58 PM, Stephen Warren wrote: > Is there a (possibly just proposed) mechanism in place to allow *.dts > from multiple Linux architectures to share common *.dtsi files? Nothing proposed yet. There was some discussion at Connect (which I missed part of). We're certainly going to start to have that issue between arm and arm64 as well as probably FSL powerpc and arm. I would like to move all dts files out of arch. I think we should think about how we construct a separate dts repository and then move the kernel structure in that direction (it's still believed there is too much interdependency to have separate repo yet). I don't think cpu architecture is the right top level structure for dts files. Probably something by vendor and/or SOC family is more appropriate. Then you have to figure out how to handle board vs. chip vendors. > As an example, consider two SoCs that are identical except for the CPU > complex. One uses an ARMv7 CPU (DTs in arch/arm/boot/dts/) and the other > uses some ARMv8 CPU (DTs in arch/am64/boot/dts/). It'd be useful to > define all the SoC components in some common .dtsi file to avoid > duplication, and have both arch/arm/boot/dts/tegraXXX.dtsi and > arch/arm64/boot/dts/tegraYYY.dtsi include that and add the relevant > CPU-related nodes. > > I could imagine creating one of the following paths for this purpose: > > arch/common/dts/ > include/dt-common/ > include/dtsi/ > > ... or perhaps re-using the existing: > > include/dt-bindings/ > > ... although my original intent for that last location was just to house > header files that define constants that are part of binding definitions, > rather than actual structural content. I think we should stick with defines there. Rob ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Sharing *.dtsi between Linux architectures?
Is there a (possibly just proposed) mechanism in place to allow *.dts from multiple Linux architectures to share common *.dtsi files? As an example, consider two SoCs that are identical except for the CPU complex. One uses an ARMv7 CPU (DTs in arch/arm/boot/dts/) and the other uses some ARMv8 CPU (DTs in arch/am64/boot/dts/). It'd be useful to define all the SoC components in some common .dtsi file to avoid duplication, and have both arch/arm/boot/dts/tegraXXX.dtsi and arch/arm64/boot/dts/tegraYYY.dtsi include that and add the relevant CPU-related nodes. I could imagine creating one of the following paths for this purpose: arch/common/dts/ include/dt-common/ include/dtsi/ ... or perhaps re-using the existing: include/dt-bindings/ ... although my original intent for that last location was just to house header files that define constants that are part of binding definitions, rather than actual structural content. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 2/2] iio: twl6030-gpadc: TWL6030, TWL6032 GPADC driver
A couple of comments inline.
On 07/12/2013 09:18 AM, Oleksandr Kozaruk wrote:
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index ab0767e6..87d699e 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -157,4 +157,12 @@ config VIPERBOARD_ADC
Say yes here to access the ADC part of the Nano River
Technologies Viperboard.
+config TWL6030_GPADC
+ tristate "TWL6030 GPADC (General Purpose A/D Convertor) Support"
+ depends on TWL4030_CORE
+ default n
+ help
+ Say yes here if you want support for the TWL6030 General Purpose
+ A/D Convertor.
+
Keep it in alphabetical order
endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 0a825be..8b05633 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
+obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
Same here.
diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c
new file mode 100644
index 000..6ceb789
--- /dev/null
+++ b/drivers/iio/adc/twl6030-gpadc.c
@@ -0,0 +1,1019 @@
[...]
+static u8 twl6032_channel_to_reg(int channel)
+{
+ return TWL6032_GPADC_GPCH0_LSB;
There is more than one channel, isn't there?
[...]
> +static int twl6030_gpadc_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct twl6030_gpadc_data *gpadc = iio_priv(indio_dev);
> + int ret = -EINVAL;
> +
> + mutex_lock(&gpadc->lock);
> +
> + ret = gpadc->pdata->start_conversion(chan->channel);
> + if (ret) {
> + dev_err(gpadc->dev, "failed to start conversion\n");
> + goto err;
> + }
> + /* wait for conversion to complete */
> + wait_for_completion_interruptible_timeout(&gpadc->irq_complete,
> + msecs_to_jiffies(5000));
wait_for_completion_interruptible_timeout() can return either a negative error
code if it was interrupted, 0 if it timed out, or a positive value if it was
successfully completed. You need to handle all three cases. Have a look at
other existing drivers to see how to do this properly.
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = twl6030_gpadc_get_raw(gpadc, chan->channel, val);
> + ret = ret ? -EIO : IIO_VAL_INT;
> + break;
> +
> + case IIO_CHAN_INFO_PROCESSED:
> + ret = twl6030_gpadc_get_processed(gpadc, chan->channel, val);
> + ret = ret ? -EIO : IIO_VAL_INT;
> + break;
> +
> + default:
> + break;
> + }
> +err:
> + mutex_unlock(&gpadc->lock);
> +
> + return ret;
> +}
+}
+
[...]
+static int twl6030_gpadc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct twl6030_gpadc_data *gpadc;
+ const struct twl6030_gpadc_platform_data *pdata;
+ const struct of_device_id *match;
+ struct iio_dev *indio_dev;
+ int irq;
+ int ret = 0;
+
+ match = of_match_device(of_match_ptr(of_twl6030_match_tbl), dev);
+ pdata = match ? match->data : dev->platform_data;
+
+ if (!pdata)
+ return -EINVAL;
+
+ indio_dev = iio_device_alloc(sizeof(struct twl6030_gpadc_data));
sizeof(*gpadc)
+ if (!indio_dev) {
+ dev_err(dev, "failed allocating iio device\n");
+ ret = -ENOMEM;
+ }
+
+ gpadc = iio_priv(indio_dev);
+
+ gpadc->twl6030_cal_tbl = devm_kzalloc(dev,
+ sizeof(struct twl6030_chnl_calib) *
+ pdata->nchannels, GFP_KERNEL);
+ if (!gpadc->twl6030_cal_tbl)
+ goto err;
+
+ gpadc->dev = dev;
+ gpadc->pdata = pdata;
+
+ platform_set_drvdata(pdev, indio_dev);
+ mutex_init(&gpadc->lock);
+ init_completion(&gpadc->irq_complete);
+
+ ret = pdata->calibrate(gpadc);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to read calibration registers\n");
+ goto err;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "failed to get irq\n");
+ goto err;
+ }
+
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ twl6030_gpadc_irq_handler,
+ IRQF_ONESHOT, "twl6030_gpadc", gpadc);
You access memory in the interrupt handler which is freed before the interrupt
handler is freed.
+ if (ret) {
+ dev_dbg(&pdev->dev, "could not request irq\n");
+ goto err;
+ }
+
+ ret = twl6030_gpadc_enable_irq(TWL6030_GPADC_RT_SW1_EOC_MASK);
+ if (ret < 0) {
+ dev
Re: [PATCH v3 2/2] iio: twl6030-gpadc: TWL6030, TWL6032 GPADC driver
On 07/12/2013 08:18 AM, Oleksandr Kozaruk wrote: > The GPADC is general purpose ADC found on TWL6030, > and TWL6032 PMIC, known also as Phoenix and PhoenixLite. > > The TWL6030 and TWL6032 have GPADC with 17 and 19 > channels respectively. Some channels have current > source and are used for measuring voltage drop > on resistive load for detecting battery ID resistance, > or measuring voltage drop on NTC resistors for external > temperature measurements. Some channels measure voltage, > (i.e. battery voltage), and have voltage dividers, > thus, capable to scale voltage. Some channels are dedicated > for measuring die temperature. > > Some channels are calibrated in 2 points, having > offsets from ideal values kept in trim registers. This > is used to correct measurements. > > The differences between GPADC in TWL6030 and TWL6032: > - 10 bit vs 12 bit ADC; > - 17 vs 19 channels; > - channels have different purpose(i. e. battery voltage > channel 8 vs channel 18); > - trim values are interpreted differently. > > The driver exports function returning converted value for > requested channels. > > Based on the driver patched from Balaji TK, Graeme Gregory, Ambresh K, > Girish S Ghongdemath. Couple of things: 1) It looks from the driver that a lot of the channels are not measuring voltages but rather temperature or currents etc. If so then their types in the channel mask should be appropriately set. Also if some of the channels are entirely internal test networks, what is the benefit of exposing them to userspace as readable channels? If channels are merely 'suggested' as being used for temperatures etc then it is fine to keep them as voltages. 2) You have my sympathy when it comes to the way those trim values are packed into the registers. That is truely hideous ;) Various trivial little bits but all in all a nice clean driver. Thanks, Jonathan > > Signed-off-by: Balaji T K > Graeme Gregory > Signed-off-by: Oleksandr Kozaruk > --- > drivers/iio/adc/Kconfig |8 + > drivers/iio/adc/Makefile|1 + > drivers/iio/adc/twl6030-gpadc.c | 1019 > +++ > 3 files changed, 1028 insertions(+) > create mode 100644 drivers/iio/adc/twl6030-gpadc.c > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > index ab0767e6..87d699e 100644 > --- a/drivers/iio/adc/Kconfig > +++ b/drivers/iio/adc/Kconfig > @@ -157,4 +157,12 @@ config VIPERBOARD_ADC > Say yes here to access the ADC part of the Nano River > Technologies Viperboard. > > +config TWL6030_GPADC > + tristate "TWL6030 GPADC (General Purpose A/D Convertor) Support" > + depends on TWL4030_CORE > + default n > + help > + Say yes here if you want support for the TWL6030 General Purpose > + A/D Convertor. Perhaps a little more detail for such a complex device? > + > endmenu > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > index 0a825be..8b05633 100644 > --- a/drivers/iio/adc/Makefile > +++ b/drivers/iio/adc/Makefile > @@ -17,3 +17,4 @@ obj-$(CONFIG_MAX1363) += max1363.o > obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o > obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o > obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o > +obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o > diff --git a/drivers/iio/adc/twl6030-gpadc.c b/drivers/iio/adc/twl6030-gpadc.c > new file mode 100644 > index 000..6ceb789 > --- /dev/null > +++ b/drivers/iio/adc/twl6030-gpadc.c > @@ -0,0 +1,1019 @@ > +/* > + * TWL6030 GPADC module driver > + * > + * Copyright (C) 2009-2013 Texas Instruments Inc. > + * Nishant Kamat > + * Balaji T K > + * Graeme Gregory > + * Girish S Ghongdemath > + * Ambresh K > + * Oleksandr Kozaruk + * > + * Based on twl4030-madc.c > + * Copyright (C) 2008 Nokia Corporation > + * Mikko Ylinen > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > + * 02110-1301 USA > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRIVER_NAME "twl6030_gpadc" > + > +#define TWL6030_GPADC_MAX_CHANNELS 17 > +#define TWL6032_GPADC_MAX_CHANNELS 19 > +/* Define this as the biggest of all chips using this driver */ > +#define GPADC_MAX_CHANNELS TWL6032_GPADC_MAX_CHANNELS > + > +#define TWL6030_GPADC_CTRL 0x00/*
Re: DT binding review for Armada display subsystem
On Fri, Jul 12, 2013 at 12:39 PM, Jean-Francois Moine wrote:
> - I think it is better to keep the names 'lcd' for the memory to dumb panel
> sub-devices and 'dcon' for the dumb panel to LCD/VGA sub-device, as
> named in the spec.
I agree it is worth keeping the spec-defined names, if they do not
cause excessive confusion when mixed with the spec-defined names for
MMP2/MMP3. I'll check the spec you pointed out, thanks.
> - the phandles to the clocks does not tell how the clock may be set by
> the driver (it is an array index in the 510).
In the other threads on clock selection, we decided that that exact
information on how to select a clock (i.e. register bits/values) must
be in the driver, not in the DT. What was suggested there is a
well-documented scheme for clock naming, so the driver knows which
clock is which. That is defined in the proposed DT binding though the
"valid clock names" part. For an example driver implementation of this
you can see my patch "armada_drm clock selection - try 2".
> - I don't see the use of the phandles in the leaves (dcon and tda998x).
That is defined by the video interfaces common binding. I'm not
immediately aware of the use, but the ability to easily traverse the
graph in both directions seems like a requirement that could easily
emerge from somewhere.
> Well, here is how I feel the DTs:
>
> - for the Cubox (one LCD, the DCON is not used, TDA998x for HDMI/DVI
> output):
That is the same as my proposal except you have decided to use direct
phandle properties instead of using the common video interfaces
binding (which is just a slightly more detailed way of describing such
links). In the "best practices" thread, the suggestion was raised
multiple times of doing what v4l2 does, so thats why I decided to
explore this here.
> - for some tablet based on the Armada 510 with a LCD and a VGA connector:
>
> video {
> compatible = "marvell,armada-video";
> marvell,video-devices = <&lcd0>, <&lcd1>, <&dcon>
> };
This proposal is slightly different because it does not describe the
relationship between the LCD controllers and the DCON. Focusing
specifically on LCD1, which would be connected to a DAC via a phandle
property in your proposal. The interconnectivity between the
components represented as a graph (and in the v4l2 way, which includes
my proposal) would be:
LCD1 --- DCON --- DAC
However your phandle properties would "suggest" that LCD1 is directly
connected to the DAC. The driver would have to have special knowledge
that the DCON sits right in the middle. Maybe that is not an issue, as
it is obviously OK for the driver to have *some* knowledge about how
the hardware works :)
I don't think we have a full consensus on whether we want to go the
"v4l2 way" here. But I figured that would be a good thing to propose
in a first iteration to have a clearer idea of what the result would
look like. It sounds like you are not overly keen on it, I would be
interested to hear exactly why.
Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
On 07/12/2013 11:24 AM, Thierry Reding wrote:
> On Fri, Jul 12, 2013 at 08:40:07AM -0600, Stephen Warren wrote:
>> On 07/12/2013 04:41 AM, Laurent Pinchart wrote:
>>> Hi Stephen,
> [...]
>>> What about splitting it in three patches that
>>>
>>> - add the include/dt-bindings/pwm/pwm.h header, and update
>>> include/linux/pwm.h and
>>> Documentation/devicetree/bindings/pwm/pwm.txt
>>>
>>> - update the rest of the documentation
>>>
>>> - update the .dts files
>>
>> I think that sounds reasonable.
>
> Shouldn't the addition of include/dt-bindings/pwm/pwm.h be separate
> from its inclusion in include/linux/pwm.h so that it can be moved
> more easily (cherry-picked) to a separate repository?
I'm fine with that being another separate patch. However, I doubt
cherry-picking is an issue here; when the separate DT repo is created,
it seems likely that someone will simply copy the latest files from
the latest Linux kernel in order to populate the tree. cherry-picking
probably won't work because:
a) I doubt that the DT binding/header additions have always been kept
separate from kernel code changes in all of Linux's history.
b) I wouldn't be remotely surprised if the layout of the new repo was
entirely different to the kernel source tree layout, so direct
cherry-pick wouldn't work.
c) Not having a common git history would make adding a kernel remote
into the DT repo rather odd.
(b) and (c) would at leat require some kind of git filter operation
rather than cherry-pick, and this issue could be solve in that filter
definition.
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
>
> enum pwm_polarity {
>
> - PWM_POLARITY_NORMAL, - PWM_POLARITY_INVERSED, +
> PWM_POLARITY_NORMAL = 0, +PWM_POLARITY_INVERSED = 1,
>
> };
Rather than manually editing that to ensure the enum matches
the DT bindings header, the whole point of making a separate
directory was that drivers could include
the binding header files directly to avoid having to
duplicate the constant definitions. Can't
include and remove that enum?
>>>
>>> We could do that, but we would then need to modify all drivers
>>> to replace enum_pwm_polarity with unsigned int. Thierry, what's
>>> your opinion on this ?
>>
>> Or perhaps we could keep the enums around, but force the values
>> to match the DT constants:
>>
>> enum pwm_polarity { PWM_POLARITY_NORMAL = PWM_POLARITY_NORMAL,
>> PWM_POLARITY_INVERTED = PWM_POLARITY_INVERTED, };
>>
>> (although obviously you'd need to avoid the enum and DT constants
>> having the same name).
>
> I think I've seen stuff like the following done in a few header
> files to keep compatibility between enums and defines.
>
> enum foo { BAR, #define BAR BAR BAZ, #define BAZ BAZ };
>
> Which, as I understand it, won't work in this case because DTC can
> only cope with plain cpp files?
Yeah, dtc doesn't understand "enum", so you can't include an enum
definition in a DT file. You have to share simple #define headers
between DT and kernel code.
>> Although this brings up one point: let's say we support ACPI/..
>> bindings in the future. The enum possibly can't match the binding
>> values from every different kind of binding definition (DT, ACPI,
>> ...) so perhaps rather than changing the enum definition in
>> , what we should be doing is mapping between the
>> different name-spaces in whatever of_xlate function exists for
>> the PWM flags cell. That would be more flexible.
>
> I'm not quite sure what exactly you are suggesting here. Can you
> elaborate?
Suppose ACPI (or whatever else) starts representing PWM devices.
Suppose the author isn't aware that DT exists, represents PWM devices
and/or has already defined some PWM-related flags. So, ACPI picks bit
5 in some data value to represent inverted, rather than bit 0. Then,
there is no way that all of [ (a) DT binding PWM flags (b) ACPI PWM
flags (c) Linux's enum foo ] can use the same values. Hence, some
mapping is required between them.
The typical way to do this is to define an "of_xlate" function which
converts from DT cell values to Linux-internal representation.
Presumably if we added an ACPI parser, there'd be some equivalent for
that.
So, what I'm arguing for is that of_pwm_simple_xlate() (or any other
custom xlate function) should include both and
, and contain explicit code to convert between the two
name-spaces of flags definitions. Since those two name-spaces
currently are 100% identical, presumably if the code is written in the
right way, the compiler will actually just optimize it all away...
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] gps6507x.txt: Remove executable bits
Documentation shouldn't be executable. Signed-off-by: Joe Perches --- diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt old mode 100755 new mode 100644 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2] pci: exynos: split into two parts such as Synopsys part and Exynos part
On Fri, Jul 12, 2013 at 03:31:23PM +0530, Kishon Vijay Abraham I wrote: > Hi, > > On Thursday 11 July 2013 11:19 AM, Jingoo Han wrote: [...] > > /* set the number of lines as 4 */ > > - readl_rc(pp, dbi_base + PCIE_PORT_LINK_CONTROL, &val); > > + dw_pcie_readl_rc(pp, dbi_base + PCIE_PORT_LINK_CONTROL, &val); > > val &= ~PORT_LINK_MODE_MASK; > > val |= PORT_LINK_MODE_4_LANES; > > - writel_rc(pp, val, dbi_base + PCIE_PORT_LINK_CONTROL); > > + dw_pcie_writel_rc(pp, val, dbi_base + PCIE_PORT_LINK_CONTROL); > > I guess here we need to make this configurable. In Jacinto6 this can be either > single lane or double lane. Maybe we should have a dt property to specify the > number of lanes? On Tegra we use nvidia,num-lanes to specify the lane count for each port. Perhaps standardizing on a generic num-lanes property would make sense? Cc'ing devicetree-discuss mailing list, maybe somebody can provide some guidance. Thierry signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
On Fri, Jul 12, 2013 at 08:40:07AM -0600, Stephen Warren wrote:
> On 07/12/2013 04:41 AM, Laurent Pinchart wrote:
> > Hi Stephen,
[...]
> > What about splitting it in three patches that
> >
> > - add the include/dt-bindings/pwm/pwm.h header, and update
> > include/linux/pwm.h
> > and Documentation/devicetree/bindings/pwm/pwm.txt
> >
> > - update the rest of the documentation
> >
> > - update the .dts files
>
> I think that sounds reasonable.
Shouldn't the addition of include/dt-bindings/pwm/pwm.h be separate from
its inclusion in include/linux/pwm.h so that it can be moved more easily
(cherry-picked) to a separate repository?
> >>> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> >>>
> >>> enum pwm_polarity {
> >>>
> >>> - PWM_POLARITY_NORMAL,
> >>> - PWM_POLARITY_INVERSED,
> >>> + PWM_POLARITY_NORMAL = 0,
> >>> + PWM_POLARITY_INVERSED = 1,
> >>>
> >>> };
> >>
> >> Rather than manually editing that to ensure the enum matches the DT
> >> bindings
> >> header, the whole point of making a separate directory
> >> was
> >> that drivers could include the binding header files directly to avoid
> >> having
> >> to duplicate the constant definitions. Can't include >> bindings/pwm.h> and remove that enum?
> >
> > We could do that, but we would then need to modify all drivers to replace
> > enum_pwm_polarity with unsigned int. Thierry, what's your opinion on this ?
>
> Or perhaps we could keep the enums around, but force the values to match
> the DT constants:
>
> enum pwm_polarity {
> PWM_POLARITY_NORMAL = PWM_POLARITY_NORMAL,
> PWM_POLARITY_INVERTED = PWM_POLARITY_INVERTED,
> };
>
> (although obviously you'd need to avoid the enum and DT constants having
> the same name).
I think I've seen stuff like the following done in a few header files to
keep compatibility between enums and defines.
enum foo {
BAR,
#define BAR BAR
BAZ,
#define BAZ BAZ
};
Which, as I understand it, won't work in this case because DTC can only
cope with plain cpp files?
> Although this brings up one point: let's say we support ACPI/.. bindings
> in the future. The enum possibly can't match the binding values from
> every different kind of binding definition (DT, ACPI, ...) so perhaps
> rather than changing the enum definition in , what we
> should be doing is mapping between the different name-spaces in whatever
> of_xlate function exists for the PWM flags cell. That would be more
> flexible.
I'm not quite sure what exactly you are suggesting here. Can you
elaborate?
Thierry
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] iio: add APDS9300 ambilent light sensor driver
On 07/10/2013 03:08 PM, Oleksandr Kravchenko wrote:
> From: Oleksandr Kravchenko
>
> This patch adds IIO driver for APDS9300 ambilent light sensor (ALS).
s/ambilent/ambient/
> http://www.avagotech.com/docs/AV02-1077EN
>
> The driver allows to read raw data from ADC registers or calculate
> lux value. It also can handle threshold inrerrupt.
s/inrerrupt/interrupt/
The patch looks very good in general, a couple of comment inline.
>
> Signed-off-by: Oleksandr Kravchenko
> ---
> .../devicetree/bindings/iio/light/apds9300.txt | 22 +
> drivers/iio/light/Kconfig | 10 +
> drivers/iio/light/Makefile |1 +
> drivers/iio/light/apds9300.c | 528
>
> 4 files changed, 561 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/light/apds9300.txt
> create mode 100644 drivers/iio/light/apds9300.c
>
> diff --git a/Documentation/devicetree/bindings/iio/light/apds9300.txt
> b/Documentation/devicetree/bindings/iio/light/apds9300.txt
> new file mode 100644
> index 000..d6f66c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/apds9300.txt
> @@ -0,0 +1,22 @@
> +* Avago APDS9300 ambient light sensor
> +
> +http://www.avagotech.com/docs/AV02-1077EN
> +
> +Required properties:
> +
> + - compatible : should be "avago,apds9300"
You should also add the avago vendor prefix to
Documentation/devicetree/bindings/vendor-prefixes.txt. Preferably in a
separate patch.
> + - reg : the I2C address of the sensor
> +
> +Optional properties:
> +
> + - interrupt-parent : should be the phandle for the interrupt controller
> + - interrupts : interrupt mapping for GPIO IRQ
> +
> +Example:
> +
> +apds9300@39 {
> + compatible = "avago,apds9300";
> + reg = <0x39>;
> + interrupt-parent = <&gpio2>;
> + interrupts = <29 8>;
> +};
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 5ef1a39..08a6742 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -52,6 +52,16 @@ config VCNL4000
>To compile this driver as a module, choose M here: the
>module will be called vcnl4000.
>
> +config APDS9300
> + tristate "APDS9300 ambient light sensor"
> + depends on I2C
> + help
> + Say Y here if you want to build a driver for the Avago APDS9300
> + ambient light sensor.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called apds9300.
> +
Keeps this in alphabetical order
> config HID_SENSOR_ALS
> depends on HID_SENSOR_HUB
> select IIO_BUFFER
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 040d9c7..da58e12 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_ADJD_S311) += adjd_s311.o
> obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
> obj-$(CONFIG_SENSORS_TSL2563)+= tsl2563.o
> obj-$(CONFIG_VCNL4000) += vcnl4000.o
> +obj-$(CONFIG_APDS9300) += apds9300.o
Same here
> obj-$(CONFIG_HID_SENSOR_ALS) += hid-sensor-als.o
> diff --git a/drivers/iio/light/apds9300.c b/drivers/iio/light/apds9300.c
> new file mode 100644
> index 000..2275ecc
> --- /dev/null
> +++ b/drivers/iio/light/apds9300.c
> @@ -0,0 +1,528 @@
> +/*
> + * apds9300.c - IIO driver for Avago APDS9300 ambient light sensor
> + *
> + * Copyright 2013 Oleksandr Kravchenko
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
No device driver should ever need to include irq.h
> +#include
> +#include
> +#include
> +
[...]
> +
> +static unsigned long als_calculate_lux(u16 ch0, u16 ch1)
> +{
> + unsigned long lux, tmp;
> + u64 tmp64;
> +
> + /* avoid division by zero */
> + if (ch0 == 0)
> + return 0;
> +
> + tmp = ch1 * 100 / ch0;
> + if (tmp <= 52) {
> + /*
> + * Variable tmp64 need to avoid overflow of this part of lux
> + * calculation formula.
> + */
If you want to avoid the overflow you have to do the math as 64bit math. As
it is right now it will do 32bit math and only store the result in a 64 bit
variable.
> + tmp64 = ch0 * lux_ratio[tmp] * 5930 / 1000;
> + lux = 3150 * ch0 - (unsigned long)tmp64;
> + }
> + else if (tmp <= 65)
> + lux = 2290 * ch0 - 2910 * ch1;
> + else if (tmp <= 80)
> + lux = 1570 * ch0 - 1800 * ch1;
> + else if (tmp <= 130)
> + lux = 338 * ch0 - 260 * ch1;
> + else
> + lux = 0;
> +
> + return lux / 10;
> +}
> +
[...]
> +static int als_get_adc_val(struct als_data *data, int adc_number)
>
Re: [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup
On 07/12/2013 05:26 PM, Gerhard Sittig wrote: [...] > Lars, there is a checkpatch warning about a line longer than 80 > characters in the common xlate part -- I didn't dare to change your > submission, and the part included here is verbatim from your patchwork > 2331091 (original submission) and 2555701 (resend), is there a followup > which you can provide or point us to? Well the line is 81 characters long, so I wouldn't worry to much about that. But we should probably remove the extern from all the function declarations in dma-of.h, since the extern is kind of implicit for functions declarations. - Lars ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
DT binding review for Armada display subsystem
Hi,
Based on the outcomes of the "Best practice device tree design for display
subsystems" discussion I have drafted a DT binding. Comments much appreciated.
At a high level, it uses a "super node" as something for the driver to bind
to, needed because there is no clear one device that controls all the
others, and also to distinguish between the Armada 510 possible use cases
of having one video card with two outputs, or two independent video cards.
It uses node-to-node linking beyond that point, V4L2 style.
I have some questions still:
1. What names does the Armada 510 datasheet give to the components? Below
I renamed the "LCD controllers" (which don't control LCDs on any of the
current target hardware) to "display controllers". For what we were
previously referring to as DCON, I renamed to "output selector". I would
like to match the docs.
Actually the output selector is not mentioned as per Sebastian's
suggestion, but I believe it would fit in the below design once the first
user comes along (e.g. it would slot in the graph between dcon0 and tda99x).
2. On that topic, what names does the Armada 510 datasheet give to the
available clocks? Lets make them match the docs.
3. What is the "remote" property in the video interfaces binding? Doesn't
seem to be documented. But I think I copied its usage style below.
Marvell Armada Display Subsystem
Design considerations
-
The display device that emerges from this subsystem is quite heavily
componentized and the formation of the composite device will vary by SoC and
by board. The DT binding is designed with this flexibility in mind.
For example, there are 2 display controllers on the Armada 510.
They could legitametely be used to form two independent video cards, or
they could be combined into a single video card with two outputs, depending
on board wiring and use case.
As there is no clear component that controls the other components, especially
when we wish to combine the two display controllers into one virtual device, we
define a top-level "super node" that describes the basic composition of the
overall display device. That node uses phandles to define the start of the
graph of display components.
In the bindings for the individual display components, phandle properties
are used to represent direct, fixed links to other components. We use the
port/endpoint abstraction from bindings/media/video-interfaces.txt to
represent these links.
display super node
--
Required properties:
- compatible: "marvell,armada-display"
- marvell,video-devices : List of phandles to the display controllers that
form this composite device.
Optional properties;
- reg: Memory address and size of a RAM allocation designated as graphics
memory
- linux,video-memory-size: If reg is not provided, this property defines
how much memory to cut out of regular available RAM to be used as graphics
memory.
display-controller node
---
Required properties:
- compatible: "marvell,armada-510-display", "marvell,mmp2-display" or
"marvell,mmp3-display"
- reg: Register space for this display controller
- clocks: List of clocks available to this device. From common clock binding.
- clock-names: Names of the given clocks (see below)
- ports: From video interface binding
Valid clock names for Armada 510: extclk0 extclk1 axi pll
Valid clock names for MMP2: lcd1 lcd2 axi hdmi
Valid clock names for MMP3: lcd1 lcd2 axi hdmi dsi lvds
Example:
display {
  compatible = "marvell,armada-display";
   reg = <0 0x3f00 0x100>; /* video-mem hole */
   marvell,video-devices = <&dcon0>;
  };
dcon0: display-controller@2 {
compatible = "marvell,armada-510-display-controller";
   reg = <0x2 0x1000>;
   interrupts = <47>;
   clocks = <&ext_clk0>;
   clock-names = "extclk0";
port {
dcon0_1: endpoint {
remote = <&tda998x>;
};
};
};
&i2c0 {
  tda998x: hdmi-transmitter@60 {
   compatible = "nxp,tda19988";
    reg = <0x60>;
port {
tda998x_1: endpoint {
remote-endpoint = <&dcon0_1>;
};
};
   };
};
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 1/3] dts: change Marvell prefix to 'marvell'
On Fri, Jul 12, 2013 at 10:05:45AM -0600, Daniel Drake wrote:
> On Fri, Jul 12, 2013 at 9:57 AM, Jason Cooper wrote:
> > This also means we should do a patch for stable v3.5+ appending the
> > "mrvl,..." string to the drivers that had it removed improperly, as
> > Daniel discovered. Daniel, since you are probably most familiar (and
> > most able to test ;-) ), would you mind putting that patch together?
>
> I will look at that. There was another case of a quiet mrvl->marvell
> conversion that bit us around a year ago as well, so I will dig that
> up too for consideration.
Ok, thanks.
> > So we don't have to keep it around forever, we could do as Arnd has
> > suggested in the past (for an entirely different problem):
> >
> > /* assuming it goes in for v3.12 */
> > if (WARN_ON(of_device_is_compatible(dev, "mrvl,..."))) {
> > dev_info(&dev->dev, "compatible string \"mrvl,...\" being
> > removed in v3.15\n");
> > BUG_ON(LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0));
> > }
> >
> > * I can't find where Arnd's suggestion was, so this hack is completely
> > my own.
> >
> > Keep in mind, the above hack is just a suggestion, it makes my skin
> > crawl just looking at it... I'm open to other ideas. Or, not doing it
> > at all.
>
> A new OpenFirmware or dtb file would want to keep the old mrvl
> compatible string around in order to have compatibility with old
> kernels. So I think that hack would have to be extended to "if device
> is compatible with mrvl, but not marvell, then warn". And that seems
> so ugly that my vote would be to avoid it. Especially while we don't
> know of existing users who have a requirement of stability.
whew! Ok, drop that idea.
thx,
Jason.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 1/3] dts: change Marvell prefix to 'marvell'
On Fri, Jul 12, 2013 at 9:57 AM, Jason Cooper wrote:
> This also means we should do a patch for stable v3.5+ appending the
> "mrvl,..." string to the drivers that had it removed improperly, as
> Daniel discovered. Daniel, since you are probably most familiar (and
> most able to test ;-) ), would you mind putting that patch together?
I will look at that. There was another case of a quiet mrvl->marvell
conversion that bit us around a year ago as well, so I will dig that
up too for consideration.
> So we don't have to keep it around forever, we could do as Arnd has
> suggested in the past (for an entirely different problem):
>
> /* assuming it goes in for v3.12 */
> if (WARN_ON(of_device_is_compatible(dev, "mrvl,..."))) {
> dev_info(&dev->dev, "compatible string \"mrvl,...\" being
> removed in v3.15\n");
> BUG_ON(LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0));
> }
>
> * I can't find where Arnd's suggestion was, so this hack is completely
> my own.
>
> Keep in mind, the above hack is just a suggestion, it makes my skin
> crawl just looking at it... I'm open to other ideas. Or, not doing it
> at all.
A new OpenFirmware or dtb file would want to keep the old mrvl
compatible string around in order to have compatibility with old
kernels. So I think that hack would have to be extended to "if device
is compatible with mrvl, but not marvell, then warn". And that seems
so ugly that my vote would be to avoid it. Especially while we don't
know of existing users who have a requirement of stability.
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 1/3] dts: change Marvell prefix to 'marvell'
On Fri, Jul 12, 2013 at 09:10:49AM -0600, Daniel Drake wrote:
> On Thu, Jul 11, 2013 at 5:54 PM, Haojian Zhuang
> wrote:
> >> Well, Daniel Drake spoke up for OLPC. Does that count?
> >
> > We don't know they used DT on Marvell MMP2/MMP3. So they don't have DTS file
> > in kernel, we could use both old name & new name in driver.
>
> You are listed as one of the MMP maintainers in the MAINTAINERS file
> and I have sent you several patches in the few 3 weeks which make
> OLPC's usage of MMP + DT pretty obvious. As a maintainer I believe you
> are supposed to review the patches too. hint hint ;)
>
> My request to avoid breaking compatibility actually comes as a
> two-prong request.
>
> I would prefer to see these compatible properties stay the same as it
> seems like changing them has little purpose/benefit - and there *will*
> become a later point where changing them causes major breakage.
I agree with both of you :) It needs to stabilise quickly, but since
the first inception of Marvell compatible strings was in the mv643xx_eth
driver for powerpc (long before ARM thought it was cool), and they used
"marvell,...", I think we should stick with that.
Which means doing as Haojian now realizes, and having both strings in
the relevant drivers.
This also means we should do a patch for stable v3.5+ appending the
"mrvl,..." string to the drivers that had it removed improperly, as
Daniel discovered. Daniel, since you are probably most familiar (and
most able to test ;-) ), would you mind putting that patch together?
So we don't have to keep it around forever, we could do as Arnd has
suggested in the past (for an entirely different problem):
/* assuming it goes in for v3.12 */
if (WARN_ON(of_device_is_compatible(dev, "mrvl,..."))) {
dev_info(&dev->dev, "compatible string \"mrvl,...\" being
removed in v3.15\n");
BUG_ON(LINUX_VERSION_CODE >= KERNEL_VERSION(3,15,0));
}
* I can't find where Arnd's suggestion was, so this hack is completely
my own.
Keep in mind, the above hack is just a suggestion, it makes my skin
crawl just looking at it... I'm open to other ideas. Or, not doing it
at all.
thx,
Jason.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 8/8] HACK mmc: mxcmmc: enable clocks for the MPC512x
Q&D HACK to enable SD card support without correct COMMON_CLK support,
best viewed with 'git diff -w -b', NOT acceptable for mainline (NAKed)
Signed-off-by: Gerhard Sittig
---
drivers/mmc/host/mxcmmc.c | 41 +++--
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index d503635..b9d21cc 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1121,20 +1121,29 @@ static int mxcmci_probe(struct platform_device *pdev)
host->res = r;
host->irq = irq;
- host->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
- if (IS_ERR(host->clk_ipg)) {
- ret = PTR_ERR(host->clk_ipg);
- goto out_iounmap;
- }
+ if (!is_mpc512x_mmc(host)) {
+ host->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
+ if (IS_ERR(host->clk_ipg)) {
+ ret = PTR_ERR(host->clk_ipg);
+ goto out_iounmap;
+ }
- host->clk_per = devm_clk_get(&pdev->dev, "per");
- if (IS_ERR(host->clk_per)) {
- ret = PTR_ERR(host->clk_per);
- goto out_iounmap;
+ host->clk_per = devm_clk_get(&pdev->dev, "per");
+ if (IS_ERR(host->clk_per)) {
+ ret = PTR_ERR(host->clk_per);
+ goto out_iounmap;
+ }
+ } else {
+ host->clk_per = devm_clk_get(&pdev->dev, "sdhc_clk");
+ if (IS_ERR(host->clk_per)) {
+ ret = PTR_ERR(host->clk_per);
+ goto out_iounmap;
+ }
}
clk_prepare_enable(host->clk_per);
- clk_prepare_enable(host->clk_ipg);
+ if (host->clk_ipg)
+ clk_prepare_enable(host->clk_ipg);
mxcmci_softreset(host);
@@ -1204,7 +1213,8 @@ out_free_dma:
dma_release_channel(host->dma);
out_clk_put:
clk_disable_unprepare(host->clk_per);
- clk_disable_unprepare(host->clk_ipg);
+ if (host->clk_ipg)
+ clk_disable_unprepare(host->clk_ipg);
out_iounmap:
iounmap(host->base);
out_free:
@@ -1236,7 +1246,8 @@ static int mxcmci_remove(struct platform_device *pdev)
dma_release_channel(host->dma);
clk_disable_unprepare(host->clk_per);
- clk_disable_unprepare(host->clk_ipg);
+ if (host->clk_ipg)
+ clk_disable_unprepare(host->clk_ipg);
release_mem_region(host->res->start, resource_size(host->res));
@@ -1255,7 +1266,8 @@ static int mxcmci_suspend(struct device *dev)
if (mmc)
ret = mmc_suspend_host(mmc);
clk_disable_unprepare(host->clk_per);
- clk_disable_unprepare(host->clk_ipg);
+ if (host->clk_ipg)
+ clk_disable_unprepare(host->clk_ipg);
return ret;
}
@@ -1267,7 +1279,8 @@ static int mxcmci_resume(struct device *dev)
int ret = 0;
clk_prepare_enable(host->clk_per);
- clk_prepare_enable(host->clk_ipg);
+ if (host->clk_ipg)
+ clk_prepare_enable(host->clk_ipg);
if (mmc)
ret = mmc_resume_host(mmc);
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 7/8] dma: mpc512x: register for device tree channel lookup
register the controller for device tree based lookup of DMA channels
(non-fatal for backwards compatibility with older device trees), provide
the '#dma-cells' property in the shared mpc5121.dtsi file, and introduce
a bindings document for the MPC512x DMA controller
Signed-off-by: Gerhard Sittig
---
.../devicetree/bindings/dma/mpc512x-dma.txt| 55
arch/powerpc/boot/dts/mpc5121.dtsi |1 +
drivers/dma/mpc512x_dma.c | 21 ++--
3 files changed, 74 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 000..a4867d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,55 @@
+* Freescale MPC512x DMA Controller
+
+The DMA controller in the Freescale MPC512x SoC can move blocks of
+memory contents between memory and peripherals or memory to memory.
+
+Refer to the "Generic DMA Controller and DMA request bindings" description
+in the dma.txt file for a more detailled discussion of the binding. The
+MPC512x DMA engine binding follows the common scheme, but doesn't provide
+support for the optional channels and requests counters (those values are
+derived from the detected hardware features) and has a fixed client
+specifier length of 1 integer cell (the value is the DMA channel, since
+the DMA controller uses a fixed assignment of request lines per channel).
+
+
+DMA controller node properties:
+
+Required properties:
+- compatible: should be "fsl,mpc5121-dma"
+- reg: address and size of the DMA controller's register set
+- interrupts: interrupt spec for the DMA controller
+
+Optional properties:
+- #dma-cells: must be <1>, describes the number of integer cells
+ needed to specify the 'dmas' property in client nodes,
+ strongly recommended since common client helper code
+ uses this property
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = "fsl,mpc5121-dma";
+ reg = <0x14000 0x1800>;
+ interrupts = <65 0x8>;
+ #dma-cells = <1>;
+ };
+
+
+Client node properties:
+
+Required properties:
+- dmas:list of DMA specifiers, consisting each of a
handle
+ for the DMA controller and integer cells to specify
+ the channel used within the DMA controller
+- dma-names: list of identifier strings for the DMA specifiers,
+ client device driver code uses these strings to
+ have DMA channels looked up at the controller
+
+Example:
+
+ sdhc@1500 {
+ compatible = "fsl,mpc5121-sdhc";
+ /* ... */
+ dmas = <&dma0 30>;
+ dma-names = "rx-tx";
+ };
diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi
b/arch/powerpc/boot/dts/mpc5121.dtsi
index 384e692..dae99b7 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -394,6 +394,7 @@
compatible = "fsl,mpc5121-dma";
reg = <0x14000 0x1800>;
interrupts = <65 0x8>;
+ #dma-cells = <1>;
};
};
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 8f6d545..3d79e3a 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
@@ -939,11 +940,23 @@ static int mpc_dma_probe(struct platform_device *op)
/* Register DMA engine */
dev_set_drvdata(dev, mdma);
retval = dma_async_device_register(dma);
- if (retval) {
- devm_free_irq(dev, mdma->irq, mdma);
- irq_dispose_mapping(mdma->irq);
+ if (retval)
+ goto out_irq;
+
+ /* register with OF helpers for DMA lookups (nonfatal) */
+ if (dev->of_node) {
+ retval = of_dma_controller_register(dev->of_node,
+ of_dma_xlate_by_chan_id,
+ mdma);
+ if (retval)
+ dev_warn(dev, "could not register for OF lookup\n");
}
+ return 0;
+
+out_irq:
+ devm_free_irq(dev, mdma->irq, mdma);
+ irq_dispose_mapping(mdma->irq);
return retval;
}
@@ -952,6 +965,8 @@ static int mpc_dma_remove(struct platform_device *op)
struct device *dev = &op->dev;
struct mpc_dma *mdma = dev_get_drvdata(dev);
+ if (dev->of_node)
+ of_dma_controller_free(dev->of_node);
dma_async_device_unregister(&mdma->dma);
devm_free_irq(dev, mdma->irq, mdma);
[PATCH RFC 6/8] dma: of: Add common xlate function for matching by channel id
From: Lars-Peter Clausen
From: Lars-Peter Clausen
This patch adds a new common OF dma xlate callback function which will match a
channel by it's id. The binding expects one integer argument which it will use
to
lookup the channel by the id.
Unlike of_dma_simple_xlate this function is able to handle a system with
multiple DMA controllers. When registering the of dma provider with
of_dma_controller_register a pointer to the dma_device struct which is
associated with the dt node needs to passed as the data parameter. The filter
function will use this pointer to match only channels which belong to the
specified DMA controller.
Signed-off-by: Lars-Peter Clausen
---
drivers/dma/of-dma.c | 47 +++
include/linux/of_dma.h |4
2 files changed, 51 insertions(+)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index 7aa0864..d5d528e 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -229,3 +229,50 @@ struct dma_chan *of_dma_simple_xlate(struct
of_phandle_args *dma_spec,
&dma_spec->args[0]);
}
EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+struct of_dma_filter_by_chan_id_args {
+ struct dma_device *dev;
+ unsigned int chan_id;
+};
+
+static bool of_dma_filter_by_chan_id(struct dma_chan *chan, void *params)
+{
+ struct of_dma_filter_by_chan_id_args *args = params;
+
+ return chan->device == args->dev && chan->chan_id == args->chan_id;
+}
+
+/**
+ * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id
+ * @dma_spec: pointer to DMA specifier as found in the device tree
+ * @of_dma:pointer to DMA controller data
+ *
+ * This function can be used as the of xlate callback for DMA driver which
wants
+ * to match the channel based on the channel id. When using this xlate function
+ * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
+ * The data parameter of of_dma_controller_register must be a pointer to the
+ * dma_device struct the function should match upon.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+struct of_dma *ofdma)
+{
+ struct of_dma_filter_by_chan_id_args args;
+ dma_cap_mask_t cap;
+
+ args.dev = ofdma->of_dma_data;
+ if (!args.dev)
+ return NULL;
+
+ if (dma_spec->args_count != 1)
+ return NULL;
+
+ dma_cap_zero(cap);
+ dma_cap_set(DMA_SLAVE, cap);
+
+ args.chan_id = dma_spec->args[0];
+
+ return dma_request_channel(cap, of_dma_filter_by_chan_id, &args);
+}
+EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index 364dda7..b7cf614 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -42,6 +42,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct
device_node *np,
const char *name);
extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma);
+extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args
*dma_spec,
+ struct of_dma *ofdma);
#else
static inline int of_dma_controller_register(struct device_node *np,
struct dma_chan *(*of_dma_xlate)
@@ -67,6 +69,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct
of_phandle_args *dma_s
return NULL;
}
+#define of_dma_xlate_by_chan_id NULL
+
#endif
#endif /* __LINUX_OF_DMA_H */
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels
for the DMA controller of the MPC512x SoC, DMA channels directly
correspond to specific peripherals, since requester lines directly map
to channels while no additional flexibility or mapping is involved
introduce a dt-bindings header file for MPC512x DMA channels, and make
the shared DT specs in the mpc5121.dtsi as well as the DMA engine driver
use those names instead of numbers
Signed-off-by: Gerhard Sittig
---
arch/powerpc/boot/dts/mpc5121.dtsi|6 +-
drivers/dma/mpc512x_dma.c |6 --
include/dt-bindings/dma/mpc512x-dma.h | 21 +
3 files changed, 30 insertions(+), 3 deletions(-)
create mode 100644 include/dt-bindings/dma/mpc512x-dma.h
diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi
b/arch/powerpc/boot/dts/mpc5121.dtsi
index bd14c00..384e692 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -9,6 +9,8 @@
* option) any later version.
*/
+#include
+
/dts-v1/;
/ {
@@ -152,7 +154,7 @@
compatible = "fsl,mpc5121-sdhc";
reg = <0x1500 0x100>;
interrupts = <8 0x8>;
- dmas = <&dma0 30>;
+ dmas = <&dma0 MPC512x_DMACHAN_SDHC>;
dma-names = "rx-tx";
};
@@ -262,6 +264,8 @@
lpc@1 {
compatible = "fsl,mpc5121-lpc";
reg = <0x1 0x200>;
+ dmas = <&dma0 MPC512x_DMACHAN_SCLPC>;
+ dma-names = "rx-tx";
};
pata@10200 {
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 0053ff8..8f6d545 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -29,6 +29,8 @@
* file called COPYING.
*/
+#include
+
#include
#include
#include
@@ -257,7 +259,7 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
prev->tcd->dlast_sga = mdesc->tcd_paddr;
prev->tcd->e_sg = 1;
/* only start explicitly on MDDRC channel */
- if (cid == 32)
+ if (cid == MPC512x_DMACHAN_MDDRC)
mdesc->tcd->start = 1;
prev = mdesc;
@@ -276,7 +278,7 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
/* peripherals involved, use external request */
out_8(&mdma->regs->dmaserq, cid);
break;
- case 32:
+ case MPC512x_DMACHAN_MDDRC:
/* memory transfer, software provided start signal */
out_8(&mdma->regs->dmassrt, cid);
break;
diff --git a/include/dt-bindings/dma/mpc512x-dma.h
b/include/dt-bindings/dma/mpc512x-dma.h
new file mode 100644
index 000..56b06d1
--- /dev/null
+++ b/include/dt-bindings/dma/mpc512x-dma.h
@@ -0,0 +1,21 @@
+/*
+ * This header file provides symbolic specifiers for DMA channels
+ * within the MPC512x SoC's DMA controller. Since requester lines
+ * directly map to channel numbers and no additional flexibility
+ * is involved, DMA channels can be considered directly associated
+ * with individual peripherals.
+ *
+ * This header file gets shared among DT bindings which provide
+ * hardware specs, and driver code which implements supporting logic.
+ */
+
+#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H
+#define _DT_BINDINGS_DMA_MPC512x_DMA_H
+
+#define MPC512x_DMACHAN_SCLPC 26
+#define MPC512x_DMACHAN_SDHC 30
+#define MPC512x_DMACHAN_MDDRC 32
+
+#define MPC512x_DMACHAN_MAX64
+
+#endif
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 4/8] dts: mpc512x: prepare for preprocessor support
prepare C preprocessor support when processing MPC512x DTS files
- switch from DTS syntax to CPP syntax for include specs
- create a symlink such that DTS processing can reference includes
Signed-off-by: Gerhard Sittig
---
arch/powerpc/boot/dts/ac14xx.dts |2 +-
arch/powerpc/boot/dts/include/dt-bindings |1 +
arch/powerpc/boot/dts/mpc5121ads.dts |2 +-
arch/powerpc/boot/dts/pdm360ng.dts|2 +-
4 files changed, 4 insertions(+), 3 deletions(-)
create mode 12 arch/powerpc/boot/dts/include/dt-bindings
diff --git a/arch/powerpc/boot/dts/ac14xx.dts b/arch/powerpc/boot/dts/ac14xx.dts
index a27a460..a543c40 100644
--- a/arch/powerpc/boot/dts/ac14xx.dts
+++ b/arch/powerpc/boot/dts/ac14xx.dts
@@ -10,7 +10,7 @@
*/
-/include/ "mpc5121.dtsi"
+#include
/ {
model = "ac14xx";
diff --git a/arch/powerpc/boot/dts/include/dt-bindings
b/arch/powerpc/boot/dts/include/dt-bindings
new file mode 12
index 000..08c00e4
--- /dev/null
+++ b/arch/powerpc/boot/dts/include/dt-bindings
@@ -0,0 +1 @@
+../../../../../include/dt-bindings
\ No newline at end of file
diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts
b/arch/powerpc/boot/dts/mpc5121ads.dts
index 7d3cb79..c228a0a 100644
--- a/arch/powerpc/boot/dts/mpc5121ads.dts
+++ b/arch/powerpc/boot/dts/mpc5121ads.dts
@@ -9,7 +9,7 @@
* option) any later version.
*/
-/include/ "mpc5121.dtsi"
+#include
/ {
model = "mpc5121ads";
diff --git a/arch/powerpc/boot/dts/pdm360ng.dts
b/arch/powerpc/boot/dts/pdm360ng.dts
index 7433740..871c16d 100644
--- a/arch/powerpc/boot/dts/pdm360ng.dts
+++ b/arch/powerpc/boot/dts/pdm360ng.dts
@@ -13,7 +13,7 @@
* option) any later version.
*/
-/include/ "mpc5121.dtsi"
+#include
/ {
model = "pdm360ng";
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 3/8] dma: mpc512x: support 'terminate all' control request
implement the TERMINATE_ALL request in the device_control() callback
of the DMA engine driver for the MPC512x DMA controller
reword variable initialization to better follow the code path and to
avoid artificial diffs later on (this style change vanishes when this
patch gets squashed with the device_control() routine's introduction)
Signed-off-by: Gerhard Sittig
---
drivers/dma/mpc512x_dma.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index df10a48..0053ff8 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -748,11 +748,22 @@ static struct dma_async_tx_descriptor
*mpc_dma_prep_slave_sg(
static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
unsigned long arg)
{
- struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
- struct dma_slave_config *cfg = (void *)arg;
+ struct mpc_dma_chan *mchan;
+ struct mpc_dma *mdma;
+ struct dma_slave_config *cfg;
+ mchan = dma_chan_to_mpc_dma_chan(chan);
switch (cmd) {
+ case DMA_TERMINATE_ALL:
+ /* disable channel requests */
+ mdma = dma_chan_to_mpc_dma(chan);
+ out_8(&mdma->regs->dmacerq, chan->chan_id);
+ list_splice_tail_init(&mchan->prepared, &mchan->free);
+ list_splice_tail_init(&mchan->queued, &mchan->free);
+ list_splice_tail_init(&mchan->active, &mchan->free);
+ return 0;
case DMA_SLAVE_CONFIG:
+ cfg = (void *)arg;
if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
return -EINVAL;
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 2/8] dma: mpc512x: fix start condition in execute()
adjust the conditions how submitted DMA jobs get started: memory transfers
need to get initiated by an explicit software request, all transfers which
involve peripherals need to reference the external requester line
Signed-off-by: Gerhard Sittig
---
drivers/dma/mpc512x_dma.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index f90b717..df10a48 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -272,10 +272,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
mdma->tcd[cid].e_sg = 1;
switch (cid) {
- case 26:
+ default:
+ /* peripherals involved, use external request */
out_8(&mdma->regs->dmaserq, cid);
break;
case 32:
+ /* memory transfer, software provided start signal */
out_8(&mdma->regs->dmassrt, cid);
break;
}
--
1.7.10.4
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory
From: Alexander Popov
From: Alexander Popov
Data transfers between memory and i/o memory require more delicate TCD
(Transfer Control Descriptor) configuration and DMA channel service requests
via hardware.
dma_device.device_control callback function is needed to configure
DMA channel to work with i/o memory.
Signed-off-by: Alexander Popov
---
drivers/dma/mpc512x_dma.c | 147 ++---
1 file changed, 140 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2d95673..f90b717 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -2,6 +2,7 @@
* Copyright (C) Freescale Semicondutor, Inc. 2007, 2008.
* Copyright (C) Semihalf 2009
* Copyright (C) Ilya Yanok, Emcraft Systems 2010
+ * Copyright (C) Alexander Popov, Promcontroller 2013
*
* Written by Piotr Ziecik . Hardware description
* (defines, structures and comments) was taken from MPC5121 DMA driver
@@ -28,11 +29,6 @@
* file called COPYING.
*/
-/*
- * This is initial version of MPC5121 DMA driver. Only memory to memory
- * transfers are supported (tested using dmatest module).
- */
-
#include
#include
#include
@@ -190,9 +186,13 @@ struct mpc_dma_chan {
struct list_headcompleted;
struct mpc_dma_tcd *tcd;
dma_addr_t tcd_paddr;
+ u32 tcd_nunits;
/* Lock for this structure */
spinlock_t lock;
+
+ /* Channel's peripheral fifo address */
+ dma_addr_t per_paddr;
};
struct mpc_dma {
@@ -256,7 +256,9 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
prev->tcd->dlast_sga = mdesc->tcd_paddr;
prev->tcd->e_sg = 1;
- mdesc->tcd->start = 1;
+ /* only start explicitly on MDDRC channel */
+ if (cid == 32)
+ mdesc->tcd->start = 1;
prev = mdesc;
}
@@ -268,7 +270,15 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
if (first != prev)
mdma->tcd[cid].e_sg = 1;
- out_8(&mdma->regs->dmassrt, cid);
+
+ switch (cid) {
+ case 26:
+ out_8(&mdma->regs->dmaserq, cid);
+ break;
+ case 32:
+ out_8(&mdma->regs->dmassrt, cid);
+ break;
+ }
}
/* Handle interrupt on one half of DMA controller (32 channels) */
@@ -641,6 +651,126 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t
dst, dma_addr_t src,
return &mdesc->desc;
}
+static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg(
+ struct dma_chan *chan, struct scatterlist *sgl,
+ unsigned int sg_len, enum dma_transfer_direction direction,
+ unsigned long flags, void *context)
+{
+ struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
+ struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
+ struct mpc_dma_desc *mdesc = NULL;
+ struct mpc_dma_tcd *tcd;
+ unsigned long iflags;
+ struct scatterlist *sg;
+ size_t len;
+ int iter, i;
+
+ if (!list_empty(&mchan->active))
+ return NULL;
+
+ for_each_sg(sgl, sg, sg_len, i) {
+ spin_lock_irqsave(&mchan->lock, iflags);
+
+ mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc,
+ node);
+ if (!mdesc) {
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+ /* try to free completed descriptors */
+ mpc_dma_process_completed(mdma);
+ return NULL;
+ }
+
+ list_del(&mdesc->node);
+
+ spin_unlock_irqrestore(&mchan->lock, iflags);
+
+ mdesc->error = 0;
+ tcd = mdesc->tcd;
+
+ /* Prepare Transfer Control Descriptor for this transaction */
+ memset(tcd, 0, sizeof(struct mpc_dma_tcd));
+
+ if (!IS_ALIGNED(sg_dma_address(sg), 4))
+ return NULL;
+
+ if (direction == DMA_DEV_TO_MEM) {
+ tcd->saddr = mchan->per_paddr;
+ tcd->daddr = sg_dma_address(sg);
+ tcd->soff = 0;
+ tcd->doff = 4;
+ } else if (direction == DMA_MEM_TO_DEV) {
+ tcd->saddr = sg_dma_address(sg);
+ tcd->daddr = mchan->per_paddr;
+ tcd->soff = 4;
+ tcd->doff = 0;
+ } else {
+ return NULL;
+ }
+ tcd->ssize = MPC_DMA_TSIZE_4;
+ tcd->dsize = MPC_DMA_TSIZE_4;
+
+ len = sg_dma_len(sg);
+
+ if (mchan->tcd_nunits)
+
[PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup
blurb is below the stats Alexander Popov (1): powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory Gerhard Sittig (6): dma: mpc512x: fix start condition in execute() dma: mpc512x: support 'terminate all' control request dts: mpc512x: prepare for preprocessor support dma: mpc512x: use symbolic specifiers for DMA channels dma: mpc512x: register for device tree channel lookup HACK mmc: mxcmmc: enable clocks for the MPC512x Lars-Peter Clausen (1): dma: of: Add common xlate function for matching by channel id .../devicetree/bindings/dma/mpc512x-dma.txt| 55 ++ arch/powerpc/boot/dts/ac14xx.dts |2 +- arch/powerpc/boot/dts/include/dt-bindings |1 + arch/powerpc/boot/dts/mpc5121.dtsi |7 +- arch/powerpc/boot/dts/mpc5121ads.dts |2 +- arch/powerpc/boot/dts/pdm360ng.dts |2 +- drivers/dma/mpc512x_dma.c | 181 +++- drivers/dma/of-dma.c | 47 + drivers/mmc/host/mxcmmc.c | 41 +++-- include/dt-bindings/dma/mpc512x-dma.h | 21 +++ include/linux/of_dma.h |4 + 11 files changed, 336 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt create mode 12 arch/powerpc/boot/dts/include/dt-bindings create mode 100644 include/dt-bindings/dma/mpc512x-dma.h this series - introduces slave s/g support (that's support for DMA transfers which involve peripherals in contrast to mem-to-mem transfers) - adds device tree based lookup support for DMA channels - combines floating patches and related feedback which already covered several aspects of what the suggested LPB driver needs, to demonstrate how integration might be done Alexander, I'd suggest that you pickup this series and - squash part 2 (start condition) and optionally part 3 (terminate all) into part 1 (your slave s/g introduction) - drop part 8 (SD card demo) as it was already NAKed and will be obsolete with correct COMMON_CLK support, but was useful during test to have one more DMA client on the MPC512x platform - put your SCLPC support on top of the then OF capable DMA driver - please leave the preprocessor support for DTS files separate from the DMA related DTS part, as it will be part of another series as well (clocks) or might get picked up separately for its being useful or even a dependency in other ways (GPIOs, interrupts) Lars, there is a checkpatch warning about a line longer than 80 characters in the common xlate part -- I didn't dare to change your submission, and the part included here is verbatim from your patchwork 2331091 (original submission) and 2555701 (resend), is there a followup which you can provide or point us to? Arnd, this version addresses your concerns in Anatolij's earlier submissions: - the DMA controller really maps peripheral requesters 1:1 to DMA channels, there's no other mapping or flexibility involved - OF registration in the DMA controller's driver now is non-fatal for compatibility with older device trees - the device tree binding is documented (even if it's in line with the generic DMA binding, being explicit is good, I guess) Vinod, would you take these DMA related patches when Alexander's updated version has passed review? That's what I understood from Lars so far. the series is based on v3.10, each step in the series was build-tested, the series' result including the mxcmmc hack was run-tested with the commands below, moving 170MB of data and using DMA during transfers, the numbers translate to 1.6MB/s throughput for the SD card and some 62.5KB per DMA IRQ on average (does this help verify plausibility?) $ find /media/mmcblk0p1/ -type f | xargs time wc ... 532086 3032411 173164032 total real1m 47.41s user0m 8.08s sys 0m 1.69s $ grep -i dma /proc/interrupts 65: 2716 IPIC Level mpc512x_dma $ grep -i mmc /proc/interrupts 20: 5489 IPIC Level mxc-mmc -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V3 1/3] dts: change Marvell prefix to 'marvell'
On Thu, Jul 11, 2013 at 5:54 PM, Haojian Zhuang wrote: >> Well, Daniel Drake spoke up for OLPC. Does that count? > > We don't know they used DT on Marvell MMP2/MMP3. So they don't have DTS file > in kernel, we could use both old name & new name in driver. You are listed as one of the MMP maintainers in the MAINTAINERS file and I have sent you several patches in the few 3 weeks which make OLPC's usage of MMP + DT pretty obvious. As a maintainer I believe you are supposed to review the patches too. hint hint ;) My request to avoid breaking compatibility actually comes as a two-prong request. I would prefer to see these compatible properties stay the same as it seems like changing them has little purpose/benefit - and there *will* become a later point where changing them causes major breakage. At the same time I see that there have been recent efforts to remove MMP2 platform code and make it entirely DT-driven, which could also generate some compatibility concerns. However, such movements are much appreciated and I think they will become increasingly necessary as we bring up the devices on the MMP3 SoC to a fuller extent, so please continue :) I would not want to discourage you from breaking compatibility when *that* type of work needs to be done. So: breaking compatibility is actually OK from my standpoint, but only for now (while we stabilise), and I would advise/appreciate that it only be done in cases where there is a clear purpose and benefit. Daniel ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH v4 03/18] Documentation: devicetree: arm: cpus/cpu nodes bindings updates
On Fri, May 17, 2013 at 10:20 AM, Lorenzo Pieralisi wrote: > In order to extend the current cpu nodes bindings to newer CPUs > inclusive of AArch64 and to update support for older ARM CPUs this > patch updates device tree documentation for the cpu nodes bindings. Sorry for the long delay on this, but I'm still not happy with the binding here. > Main changes: > - adds 64-bit bindings > - define usage of #address-cells > - define 32/64 dts compatibility settings > - defines behaviour on pre and post v7 uniprocessor systems > - adds ARM 11MPcore specific reg property definition > > Signed-off-by: Lorenzo Pieralisi > --- > Documentation/devicetree/bindings/arm/cpus.txt | 459 > ++--- > 1 file changed, 412 insertions(+), 47 deletions(-) [snip] > + # On ARM v8 64-bit systems, where the reg property > + size can be 1 or 2 cells (as defined by cpus node's > + #address-cells property), this property is > + required and matches: > + > + - On systems running the OS in AArch32: The DTS cannot change based on 32-bit or 64-bit OS. > + > + * If the cpus node's #address-cells value is 2: > + > + The first reg cell must be set to 0. > + > + The second reg cell bits [23:0] must be set to > + bits [23:0] of MPIDR_EL1. > + > + All other bits in the reg cells must be set to > 0. > + > + * If the cpus node's #address-cells value is 1: > + > + Bits [23:0] in the reg cell must be set to > + bits [23:0] in MPIDR_EL1. > + > + All other bits in the reg cell must be 0. > + > + - On systems running the OS in AArch64: > + > + * If the cpus node's #address-cells value is 2: > + > + The first reg cell bits [7:0] must be set to > + bits [39:32] of MPIDR_EL1. > + > + The second reg cell bits [23:0] must be set to > + bits [23:0] of MPIDR_EL1. > + > + All other bits in the reg cells must be set to > 0. > + > + * If the cpus node's #address-cells value is 1: > + > + MPIDR_EL1[63:32] is 0 on all processors in the > + system. Your logic is backwards here. If the MPIDR_EL1[63:32] is 0, then #address-cells can be 1. You could say the upper bits are ignored and treated as 0. However, you should simplify all this and just mandate that #address-cells must be 2 for ARMv8 or more generally must match the size of the MPIDR. If we want to boot a 32-bit kernel, then the kernel will have to adapt to support this. > + > + The reg cell bits [23:0] must be set to > + bits [23:0] of MPIDR_EL1. > + > + All other bits in the reg cell must be set to 0. > + > + - compatible: > + Usage: required > + Value type: > + Definition: should be one of: > + "arm,arm710t" > + "arm,arm720t" > + "arm,arm740t" > + "arm,arm7ej-s" > + "arm,arm7tdmi" > + "arm,arm7tdmi-s" > + "arm,arm9es" > + "arm,arm9ej-s" > + "arm,arm920t" > + "arm,arm922t" > + "arm,arm925" > + "arm,arm926e-s" > + "arm,arm926ej-s" > + "arm,arm940t" > + "arm,arm946e-s" > + "arm,arm966e-s" > + "arm,arm968e-s" > + "arm,arm9tdmi" > + "arm,arm1020e" > + "arm,arm1020t" > + "arm,arm1022e" > + "arm,arm1026ej-s" > + "arm,arm1136j-s" > + "arm,arm1136jf-s" > + "arm,arm1156t2-s" > + "arm,arm1156t2f-s" > + "arm,arm1176jzf" > + "arm,arm1176jz-s" > + "arm,arm1176jzf-s" > + "arm,arm11mpcore" > + "arm,cortex-a5" > + "arm,cortex-a7" > + "arm,cortex-a8" > + "arm,cortex-a9" > + "arm,cortex-a15" > + "arm,
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
On 07/12/2013 05:01 AM, Laurent Pinchart wrote: > Hi, > > On Thursday 11 July 2013 14:06:44 Stephen Warren wrote: >> On 07/11/2013 01:32 PM, Thierry Reding wrote: >>> On Thu, Jul 11, 2013 at 11:50:48AM -0600, Stephen Warren wrote: On 07/11/2013 09:36 AM, Thierry Reding wrote: > On Thu, Jul 11, 2013 at 04:37:48PM +0200, Laurent Pinchart > wrote: [...] > >> diff --git >> a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >> b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >> index de0eaed..be09be4 100644 --- >> a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >> +++ b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >> @@ -4,9 +4,9 @@ Required properties: - compatible: should be >> "atmel,tcb-pwm" - #pwm-cells: Should be 3. The first cell >> specifies the per-chip index of the PWM to use, the second >> cell is the period in nanoseconds and - bit 0 in the third >> cell is used to encode the polarity of PWM output. - Set bit >> 0 of the third cell in PWM specifier to 1 for inverse >> polarity & - set to 0 for normal polarity. + the third cell >> is used to encode the polarity of PWM output. Set the + >> PWM_POLARITY_NORMAL flag for normal polarity or the >> PWM_POLARITY_INVERSED + flag for inverted polarity. PWM >> flags are defined in . - tc-block: The >> Timer Counter block to use as a PWM chip. > >> Example: > I'd prefer for the original text to stay in place and the reference to > the dt-bindings/pwm/pwm.h file to go below that block. I disagree here. The whole point of creating header files for the constants in binding definitions was so that you wouldn't have to duplicate all the values into the binding definitions. Rather, you'd simply say "see ". >>> >>> But that's not something that this patch solves. >> >> Well, if the comments I made on the patch re: that should >> simply #include instead of duplicating the >> constants, then yet this patch will solve that. There will be a single place >> where the constants are defined. > > As explained in another reply, this would require replacing the enum with an > unsigned int. I can write a patch if we agree on this. > >>> And it could be solved even in the absence of the header file defining the >>> symbolic constants. If all the standard flags that dt-bindings/pwm/pwm.txt >>> now specifies were to be listed in pwm.txt (they actually are) then >>> referring to that document as the canonical source works equally well. >> >> If that's all the happens, then there will still be duplication >> between pwm.txt and . > > I've explicitly mentioned the flags in individual DT bindings to ease adding > new flags in the future. At the moment the defined flags are either all > supported or not used at all by drivers. If we later add a new flag supported > by a subset of drivers only the driver bindings should list supported flags > for each driver. > > I'm fine with removing the explicit mentions of individual flags right now > and > adding it back when needed if you think that's better. I think the values for any common system-wide flags should be defined once in some system-wide place, and the values for any HW-specific values should be defined only in the documentation for that specific HW. You could try and avoid conflicts by either: a) Allocating system-wide flags from bit 0 up, and HW-specific flags from bit 31 down. or: b) Using 1 cell for standard flags, and a separate cell for any HW-specific flags. Drivers can quite easily adapt to adding extra cells to #pwm-cells, thus making adding a HW-specific cell later backwards-compatible. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
On 07/12/2013 04:41 AM, Laurent Pinchart wrote:
> Hi Stephen,
>
> On Thursday 11 July 2013 11:40:37 Stephen Warren wrote:
>> On 07/11/2013 08:37 AM, Laurent Pinchart wrote:
>>> Define PWM_POLARITY_NORMAL and PWM_POLARITY_INVERTED macros in
>>> include/dt-bindings/pwm/pwm.h to be used by device tree sources.
>>>
>>> Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt | 6 +++---
>>> Documentation/devicetree/bindings/pwm/pwm-samsung.txt | 5 +++--
>>> Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt | 5 +++--
>>> Documentation/devicetree/bindings/pwm/pwm.txt | 8 +---
>>> Documentation/devicetree/bindings/pwm/vt8500-pwm.txt| 5 +++--
>>> arch/arm/boot/dts/am335x-evm.dts| 3 ++-
>>> arch/arm/boot/dts/am335x-evmsk.dts | 3 ++-
>>> arch/arm/boot/dts/wm8850-w70v2.dts | 3 ++-
>>> include/dt-bindings/pwm/pwm.h | 15
>>> include/linux/pwm.h | 4 ++--
>>
>> I think this needs to be separate patches; at least the new pwm.h should
>> be introduced separately to the board-specific *.dts edits, and perhaps
>> further split up?
>
> What about splitting it in three patches that
>
> - add the include/dt-bindings/pwm/pwm.h header, and update
> include/linux/pwm.h
> and Documentation/devicetree/bindings/pwm/pwm.txt
>
> - update the rest of the documentation
>
> - update the .dts files
I think that sounds reasonable.
>> That way, the one patch that introduces would be
>> available to be merged into any other tree that wanted to take patches
>> to use the new defines.
>>
>>> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
>>>
>>> enum pwm_polarity {
>>>
>>> - PWM_POLARITY_NORMAL,
>>> - PWM_POLARITY_INVERSED,
>>> + PWM_POLARITY_NORMAL = 0,
>>> + PWM_POLARITY_INVERSED = 1,
>>>
>>> };
>>
>> Rather than manually editing that to ensure the enum matches the DT bindings
>> header, the whole point of making a separate directory was
>> that drivers could include the binding header files directly to avoid having
>> to duplicate the constant definitions. Can't include > bindings/pwm.h> and remove that enum?
>
> We could do that, but we would then need to modify all drivers to replace
> enum_pwm_polarity with unsigned int. Thierry, what's your opinion on this ?
Or perhaps we could keep the enums around, but force the values to match
the DT constants:
enum pwm_polarity {
PWM_POLARITY_NORMAL = PWM_POLARITY_NORMAL,
PWM_POLARITY_INVERTED = PWM_POLARITY_INVERTED,
};
(although obviously you'd need to avoid the enum and DT constants having
the same name).
Although this brings up one point: let's say we support ACPI/.. bindings
in the future. The enum possibly can't match the binding values from
every different kind of binding definition (DT, ACPI, ...) so perhaps
rather than changing the enum definition in , what we
should be doing is mapping between the different name-spaces in whatever
of_xlate function exists for the PWM flags cell. That would be more
flexible.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] regulator: pfuze100: add pfuze100 regulator driver
On Fri, Jul 12, 2013 at 12:54:15PM +0800, Robin Gong wrote:
> Add pfuze100 regulator driver.
This looks mostly good. A few small issues below but nothing major.
> +enum pfuze_id {
> + PFUZE_ID_PFUZE100,
> + PFUZE_ID_INVALID,
> +};
> +struct pfuze_chip {
Missing blank line here - there are a few other small coding style
things, checkpatch should help.
> +static struct regulator_ops pfuze100_ldo_regulator_ops;
> +static struct regulator_ops pfuze100_fixed_regulator_ops;
> +static struct regulator_ops pfuze100_sw_regulator_ops;
> +static struct regulator_ops pfuze100_swb_regulator_ops;
Better to just reorder things so that no forward declaration is needed.
> +static const int pfuze100_swbst[] = {
> + 500, 505, 510, 515,
> +};
This looks like a linear map, the steps are all 50mV?
> + num_regulators = pfuze_get_num_regulators_dt(&client->dev);
> + if (num_regulators <= 0 && pdata)
> + num_regulators = pdata->num_regulators;
> + if (num_regulators <= 0) {
> + dev_err(&client->dev, "no platform data,please add it!\n");
> + ret = -EINVAL;
> + return ret;
You should just register all the regulators rather than only registering
those that the user explicitly selects. This allows users to inpect the
current configuration and simplifies the code - for example you don't
need to count the DT nodes and you can just have a simple array in the
platform data (see how wm831x does this for an example).
> + /* SW2~SW4 high bit check and modify the voltage value table */
> + if (i > PFUZE100_SW1C && i < PFUZE100_SWBST) {
> + regmap_read(pfuze_chip->regmap, PFUZE100_SW2VOL +
> + (i - PFUZE100_SW2) * 7, &val);
> + if (val & 0x40) {
> + pfuze100_regulators[id].desc.min_uV = 80;
> + pfuze100_regulators[id].desc.uV_step = 5;
> + }
> + }
You should really be doing this on a copy of the regulators table rather
than on the table itself.
> + for (i = 0; i < pfuze_chip->num_regulators; i++)
> + regulator_unregister(pfuze_chip->regulators[i]);
> + kfree(pfuze_chip);
Use devm_kzalloc().
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Reference voltages for ADC channels
Hello, On 07/08/2013 08:13 AM, Hector Palacios wrote: Greetings, The other day at linux-iio-u79uwxl29ty76z2rm5m...@public.gmane.org we discussed the possibility of having ADC channels voltage references moved to the DT [1]. In Freescale's i.MX28 and i.MX23, at least, the CPU has 16 ADC channels some of which are dedicated to measure internal stuff, like CPU temperature and voltages. Some channels have different fixed divisors and so different reference voltages. These reference voltages (in mV) are needed to calculate the scale to show through the sysfs IIO interface, so that a user can easily compute the real measured voltage out of the sampled data. The proposed DT entry also appears at [1]. We would love to here opinions about it. [1] http://www.spinics.net/lists/linux-iio/msg08973.html Any opinion about the suggestion of moving ADC reference voltages into the device tree? Thanks -- Héctor Palacios ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Fixed PHY Device Tree usage?
Dear Florian Fainelli,
On Fri, 12 Jul 2013 13:05:59 +0100, Florian Fainelli wrote:
> I am talking about scanning the MDIO bus DT nodes, not the entire DT.
> That job is already done by of_mdiobus_probe() to register PHY
> devices, so having a central point where the knowledge of how to treat
> PHY deivces could be here I guess.
So, I guess your idea is to call of_mdiobus_register() from
drivers/net/phy/fixed.c:fixed_mdio_bus_init(). But then, what DT node
will you be passing to of_mdiobus_register() ? As a reminder, this
function takes as a second argument the DT node that contains the
various PHYs as sub-nodes.
In all the other PHY drivers, the MDIO bus node as a compatible string,
so the usual platform_driver/platform_device mechanism kicks in, and
calls the ->probe() function, passing the DT node of the MDIO bus,
which is then used by the PHY driver ->probe() function as the second
argument of of_mdiobus_register().
But the fixed.c PHY driver is not a platform_driver, and in our
discussion, we mentioned that it wouldn't make sense to have a
compatible string for the fixed MDIO bus DT node.
So I'm still unsure *which* DT node you'll pass as the second argument
of of_mdiobus_register() :-)
> Well either we go with some specific compatible property like
> "ethernet-phy-fixed" for instance, or we simply add a boolean property
> to the node, so a fixed PHY would either look like this:
>
> phy {
> compatible = "linux,ethernet-phy-fixed";
> speed = <1000>;
> duplex = <1>;
> pause;
> asym-pause;
> };
>
> or respectively, something like this:
>
> phy {
> fixed;
> speed = <1000>;
> duplex = <1>;
> pause;
> asym-pause;
> };
Yeah, that's fine, I have no problem with the internal properties of
the PHY nodes themselves. My question is really the one described above.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Fixed PHY Device Tree usage?
Hello Thomas,
2013/7/12 Thomas Petazzoni :
>> Why not? Since we are already have to scan the entire MDIO bus we are
>> attached to, when we encounter such a PHY node with the special
>> "fixed" properties, we just call fixed_phy_add() with the right
>> parameters and voila. Which is also the reason why I was suggesting to
>> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
>> we have this logic only in one place.
>
> I'm still not sure to understand you here. Scanning the *entire* DT
> tree and consider all nodes having a property named "fixed" as fixed
> PHYs is definitely not acceptable. So I suppose you have a different
> idea, but I'm still not getting it. Where in the DT would the fixed PHY
> driver start looking for fixed PHYs ?
I am talking about scanning the MDIO bus DT nodes, not the entire DT.
That job is already done by of_mdiobus_probe() to register PHY
devices, so having a central point where the knowledge of how to treat
PHY deivces could be here I guess.
>
>> > So that's really what I was asking: how is the fixed PHY driver going
>> > to know which DT nodes to look at. Is it a platform_driver, where the
>> > corresponding DT node has sub-nodes? Is it something else? Or a
>> > specific compatible string?
>>
>> Without DT at play here, the usual way to register a fixed PHY is:
>>
>> 1) make your arch code or whatever runs before the fixed MDIO bus
>> probing to call fixed_phy_add() with the expected parameters
>> 2) when your ethernet driver probes its PHY devices, format the phy
>> name to be bound to the fixed bus with the expected address by then
>> the fixed MDIO bus would have already been probed and would find the
>> fixed PHY nodes because of the first step
>> 3) call of_phy_connect() from your driver to attach to the fixed PHY
>
> Right, but that's still doesn't answer the question of how the fixed
> PHY driver discovers from the DT which PHYs to instantiate.
>
> For example, we would probably have something:
>
> phys {
> phy0: phy@0 {
> ... PHY properties ...
> };
> phy1: phy@1 {
> ... PHY properties ...
> };
> };
>
> soc {
> ethernet@0 {
> compatible = "...";
> ...
> phy = <&phy0>;
> };
> ethernet@1 {
> compatible = "...";
> ...
> phy = <&phy1>;
> };
> };
>
> How will the fixed PHY driver know that it should instantiate
> phy@0 and phy@1 as PHY devices?
Well either we go with some specific compatible property like
"ethernet-phy-fixed" for instance, or we simply add a boolean property
to the node, so a fixed PHY would either look like this:
phy {
compatible = "linux,ethernet-phy-fixed";
speed = <1000>;
duplex = <1>;
pause;
asym-pause;
};
or respectively, something like this:
phy {
fixed;
speed = <1000>;
duplex = <1>;
pause;
asym-pause;
};
--
Florian
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Fixed PHY Device Tree usage?
Dear Florian Fainelli,
On Wed, 10 Jul 2013 18:23:30 +0100, Florian Fainelli wrote:
> >> - declare all PHY nodes in the system as sub nodes of their belonging
> >> real hardware MDIO bus node
> >> - flag specific PHY nodes as "fixed" with a "fixed-link" boolean for
> >> instance
> >> - if we see that flag, make that specific PHY node bind to the
> >> fixed-phy driver instead
> >
> > So the fixed PHY driver is going to travel through *all* nodes of the
> > DT, and whenever some random node has a "fixed" property, it's going to
> > say it corresponds to a fixed PHY? That doesn't seem like a good idea.
>
> Why not? Since we are already have to scan the entire MDIO bus we are
> attached to, when we encounter such a PHY node with the special
> "fixed" properties, we just call fixed_phy_add() with the right
> parameters and voila. Which is also the reason why I was suggesting to
> put the "fixed" PHY nodes as sub-nodes of the real MDIO node such that
> we have this logic only in one place.
I'm still not sure to understand you here. Scanning the *entire* DT
tree and consider all nodes having a property named "fixed" as fixed
PHYs is definitely not acceptable. So I suppose you have a different
idea, but I'm still not getting it. Where in the DT would the fixed PHY
driver start looking for fixed PHYs ?
> > So that's really what I was asking: how is the fixed PHY driver going
> > to know which DT nodes to look at. Is it a platform_driver, where the
> > corresponding DT node has sub-nodes? Is it something else? Or a
> > specific compatible string?
>
> Without DT at play here, the usual way to register a fixed PHY is:
>
> 1) make your arch code or whatever runs before the fixed MDIO bus
> probing to call fixed_phy_add() with the expected parameters
> 2) when your ethernet driver probes its PHY devices, format the phy
> name to be bound to the fixed bus with the expected address by then
> the fixed MDIO bus would have already been probed and would find the
> fixed PHY nodes because of the first step
> 3) call of_phy_connect() from your driver to attach to the fixed PHY
Right, but that's still doesn't answer the question of how the fixed
PHY driver discovers from the DT which PHYs to instantiate.
For example, we would probably have something:
phys {
phy0: phy@0 {
... PHY properties ...
};
phy1: phy@1 {
... PHY properties ...
};
};
soc {
ethernet@0 {
compatible = "...";
...
phy = <&phy0>;
};
ethernet@1 {
compatible = "...";
...
phy = <&phy1>;
};
};
How will the fixed PHY driver know that it should instantiate
phy@0 and phy@1 as PHY devices?
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 2/7] sound: sam9x5_wm8731: machine driver for at91sam9x5 wm8731 boards
On Thu, Jul 11, 2013 at 06:15:54PM +0200, Richard Genoud wrote:
> From: Nicolas Ferre
>
> Description of the Asoc machine driver for an at91sam9x5 based board
ASoC.
> +sam9x5 pins:
> + * LOUT
> + * ROUT
> + * LHPOUT
> + * RHPOUT
> + * LLINEIN
> + * RLINEIN
> + * MICIN
These aren't pins on the CPU, they're pins on the CODEC, and you should
be adding this to the binding document for the CODEC and referring to
that rather than having them in each individual binding document. This
also helps if any new variants are added (not that this is likely for
the WM8731).
> +static struct sam9x5_drvdata sam9x5_priv;
Why is this a global static?
> + ret = snd_soc_register_card(&snd_soc_sam9x5);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "ASoC: Platform device allocation failed\n");
> + goto out_put_audio;
> + }
> + platform_set_drvdata(pdev, &snd_soc_sam9x5);
It should be being dynamically allocated and retrieved as driver data
when needed.
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v5 1/7] sound: codec: wm8731: add rates constraints
On Thu, Jul 11, 2013 at 06:15:53PM +0200, Richard Genoud wrote:
Please always try to use commit logs that look like normal commit logs
for the subsystem.
> switch (freq) {
> - case 11289600:
> case 1200:
> + wm8731->constraints = &wm8731_constraints_1200;
> + break;
> case 12288000:
> - case 16934400:
> case 18432000:
> - wm8731->sysclk = freq;
> + wm8731->constraints = &wm8731_constraints_12288000_18432000;
> + break;
> + case 16934400:
> + case 11289600:
> + wm8731->constraints = &wm8731_constraints_11289600_16934400;
> break;
> default:
> return -EINVAL;
> }
This isn't going to work with systems which have a variable clock as the
input to the CODEC. If it's imposing constraints the driver needs to
allow setting the clock to zero as a way of removing constraints (and
any existing drivers should be updated to do this if needed).
signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
Hi, On Thursday 11 July 2013 14:06:44 Stephen Warren wrote: > On 07/11/2013 01:32 PM, Thierry Reding wrote: > > On Thu, Jul 11, 2013 at 11:50:48AM -0600, Stephen Warren wrote: > >> On 07/11/2013 09:36 AM, Thierry Reding wrote: > >>> On Thu, Jul 11, 2013 at 04:37:48PM +0200, Laurent Pinchart > >>> wrote: [...] > >>> > diff --git > a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > index de0eaed..be09be4 100644 --- > a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > +++ b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > @@ -4,9 +4,9 @@ Required properties: - compatible: should be > "atmel,tcb-pwm" - #pwm-cells: Should be 3. The first cell > specifies the per-chip index of the PWM to use, the second > cell is the period in nanoseconds and - bit 0 in the third > cell is used to encode the polarity of PWM output. - Set bit > 0 of the third cell in PWM specifier to 1 for inverse > polarity & - set to 0 for normal polarity. + the third cell > is used to encode the polarity of PWM output. Set the + > PWM_POLARITY_NORMAL flag for normal polarity or the > PWM_POLARITY_INVERSED + flag for inverted polarity. PWM > flags are defined in . - tc-block: The > Timer Counter block to use as a PWM chip. > >>> > Example: > >>> I'd prefer for the original text to stay in place and the reference to > >>> the dt-bindings/pwm/pwm.h file to go below that block. > >> > >> I disagree here. The whole point of creating header files for the > >> constants in binding definitions was so that you wouldn't have to > >> duplicate all the values into the binding definitions. Rather, you'd > >> simply say "see ". > > > > But that's not something that this patch solves. > > Well, if the comments I made on the patch re: that should > simply #include instead of duplicating the > constants, then yet this patch will solve that. There will be a single place > where the constants are defined. As explained in another reply, this would require replacing the enum with an unsigned int. I can write a patch if we agree on this. > > And it could be solved even in the absence of the header file defining the > > symbolic constants. If all the standard flags that dt-bindings/pwm/pwm.txt > > now specifies were to be listed in pwm.txt (they actually are) then > > referring to that document as the canonical source works equally well. > > If that's all the happens, then there will still be duplication > between pwm.txt and . I've explicitly mentioned the flags in individual DT bindings to ease adding new flags in the future. At the moment the defined flags are either all supported or not used at all by drivers. If we later add a new flag supported by a subset of drivers only the driver bindings should list supported flags for each driver. I'm fine with removing the explicit mentions of individual flags right now and adding it back when needed if you think that's better. > > If we can take both of the above for granted, then sure, let's refer to > > the header from within the generic pwm.txt file and add a reference to > > that in bindings for drivers that use the standard flags. > > > >>> Another issue might be that people without access to recent versions of > >>> DTC won't be able to use the new #include feature, so keeping the > >>> documentation backwards compatible seems like a good idea. > >> > >> The dtc source tree is duplicated into the kernel source tree, so that > >> isn't an issue for now. > >> > >> Besides, the dtc version is an entirely unrelated issue to how the > >> documentation is written. > > > > Well, not really. If the documentation specifies the binding in a way that > > the DTC can't handle that's still a problem. People will end up with a DTS > > that their DTC can't compile. I guess that can be resolved by adding a > > note to the upstream device tree repository about the minimum required > > version of DTC. > > Yes, the separate repository would obviously require a version of dtc that's > able to compile the files there; i.e. a version equivalent to what's already > in the kernel tree (upstream 1.4.0 specifically). > > Again, right now, all of the binding docs, the *.dts files, and the dtc > required to use them are part of the kernel; a single package, so there's no > scope for issues re: using dtc features that aren't supported. If those > components get separated later, obviously there will be a requirement to > install a specific version of dtc to use with the separated *.dts and > binding files. -- Regards, Laurent Pinchart ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
Hi Thierry, On Thursday 11 July 2013 08:36:00 Thierry Reding wrote: > On Thu, Jul 11, 2013 at 04:37:48PM +0200, Laurent Pinchart wrote: > [...] > > > diff --git a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > > b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt index > > de0eaed..be09be4 100644 > > --- a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > > +++ b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt > > > > @@ -4,9 +4,9 @@ Required properties: > > - compatible: should be "atmel,tcb-pwm" > > - #pwm-cells: Should be 3. The first cell specifies the per-chip index > >of the PWM to use, the second cell is the period in nanoseconds and > > - bit 0 in the third cell is used to encode the polarity of PWM output. > > - Set bit 0 of the third cell in PWM specifier to 1 for inverse polarity > > & > > - set to 0 for normal polarity. > > + the third cell is used to encode the polarity of PWM output. Set the > > + PWM_POLARITY_NORMAL flag for normal polarity or the > > PWM_POLARITY_INVERSED > > + flag for inverted polarity. PWM flags are defined in . > > - tc-block: The Timer Counter block to use as a PWM chip. > > > Example: > > I'd prefer for the original text to stay in place and the reference to the > dt-bindings/pwm/pwm.h file to go below that block. The reason is that > perhaps somebody will look at an older version of a given DT (with the > numerical value) and not have access to the include and therefore not know > which flag was being set by just looking at the documentation. I'm also not > sure about what the plans are with respect to shipping device trees in a > separate repository and if they are, whether that repository would ship the > includes as well. > > Another issue might be that people without access to recent versions of > DTC won't be able to use the new #include feature, so keeping the > documentation backwards compatible seems like a good idea. > > Perhaps the include file should even only be mentioned in the general > PWM binding documentation. > > Perhaps Grant and Rob (Cc'ed) can comment on how they want to handle > this. I'll comment on this in a reply to Stephen. > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-samsung.txt > > b/Documentation/devicetree/bindings/pwm/pwm-samsung.txt index > > ac67c68..bece18b 100644 > > --- a/Documentation/devicetree/bindings/pwm/pwm-samsung.txt > > +++ b/Documentation/devicetree/bindings/pwm/pwm-samsung.txt > > > > @@ -24,8 +24,9 @@ Required properties: > > - phandle to PWM controller node > > - index of PWM channel (from 0 to 4) > > - PWM signal period in nanoseconds > > > > - - bitmask of optional PWM flags: > > -0x1 - invert PWM signal > > + - bitmask of optional PWM flags as defined in > > : +PWM_POLARITY_NORMAL - normal polarity > > + PWM_POLARITY_INVERSED - inverted polarity > > This part mixes spaces and tabs for indentation. Please stick to spaces. OK. > > diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt > > b/Documentation/devicetree/bindings/pwm/pwm.txt index 06e6724..38c357a > > 100644 > > --- a/Documentation/devicetree/bindings/pwm/pwm.txt > > +++ b/Documentation/devicetree/bindings/pwm/pwm.txt > > @@ -43,13 +43,15 @@ because the name "backlight" would be used as fallback > > anyway.> > > pwm-specifier typically encodes the chip-relative PWM number and the PWM > > period in nanoseconds. > > > > -Optionally, the pwm-specifier can encode a number of flags in a third > > cell: > > -- bit 0: PWM signal polarity (0: normal polarity, 1: inverse polarity) > > +Optionally, the pwm-specifier can encode a number of flags (defined in > > +) in a third cell: > > +- PWM_POLARITY_NORMAL: use the normal PWM signal polarity > > +- PWM_POLARITY_INVERSED: invert the PWM signal polarity > > You'd have a hard time finding those in the GPIO header. =) Oops :-) Will fix. > > diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h > > new file mode 100644 > > index 000..f82be30 > > --- /dev/null > > +++ b/include/dt-bindings/pwm/pwm.h > > @@ -0,0 +1,15 @@ > > +/* > > + * This header provides constants for most PWM bindings. > > + * > > + * Most PWM bindings can include a flags cell as part of the PWM > > specifier. > > + * In most cases, the format of the flags cell uses the standard values > > + * defined in this header. > > + */ > > + > > +#ifndef _DT_BINDINGS_PWM_PWM_H > > +#define _DT_BINDINGS_PWM_PWM_H > > + > > +#define PWM_POLARITY_NORMAL(0 << 0) > > +#define PWM_POLARITY_INVERSED (1 << 0) > > + > > +#endif > > I think this should go into a patch separate from the DT changes above > because they'll likely go in via different trees. Or maybe they won't, > but it'd still be good to introduce the header first and only then > change the DTS files. I'll fix that as well. Please see my reply to Stephen for details. > > diff --git a/include/linux/pwm.h b/i
Re: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT
Hi Stephen,
On Thursday 11 July 2013 11:40:37 Stephen Warren wrote:
> On 07/11/2013 08:37 AM, Laurent Pinchart wrote:
> > Define PWM_POLARITY_NORMAL and PWM_POLARITY_INVERTED macros in
> > include/dt-bindings/pwm/pwm.h to be used by device tree sources.
> >
> > Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt | 6 +++---
> > Documentation/devicetree/bindings/pwm/pwm-samsung.txt | 5 +++--
> > Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt | 5 +++--
> > Documentation/devicetree/bindings/pwm/pwm.txt | 8 +---
> > Documentation/devicetree/bindings/pwm/vt8500-pwm.txt| 5 +++--
> > arch/arm/boot/dts/am335x-evm.dts| 3 ++-
> > arch/arm/boot/dts/am335x-evmsk.dts | 3 ++-
> > arch/arm/boot/dts/wm8850-w70v2.dts | 3 ++-
> > include/dt-bindings/pwm/pwm.h | 15
> > include/linux/pwm.h | 4 ++--
>
> I think this needs to be separate patches; at least the new pwm.h should
> be introduced separately to the board-specific *.dts edits, and perhaps
> further split up?
What about splitting it in three patches that
- add the include/dt-bindings/pwm/pwm.h header, and update include/linux/pwm.h
and Documentation/devicetree/bindings/pwm/pwm.txt
- update the rest of the documentation
- update the .dts files
> That way, the one patch that introduces would be
> available to be merged into any other tree that wanted to take patches
> to use the new defines.
>
> > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> >
> > enum pwm_polarity {
> >
> > - PWM_POLARITY_NORMAL,
> > - PWM_POLARITY_INVERSED,
> > + PWM_POLARITY_NORMAL = 0,
> > + PWM_POLARITY_INVERSED = 1,
> >
> > };
>
> Rather than manually editing that to ensure the enum matches the DT bindings
> header, the whole point of making a separate directory was
> that drivers could include the binding header files directly to avoid having
> to duplicate the constant definitions. Can't include bindings/pwm.h> and remove that enum?
We could do that, but we would then need to modify all drivers to replace
enum_pwm_polarity with unsigned int. Thierry, what's your opinion on this ?
Replying to a comment from another e-mail, I know that the above change to
include/linux/pwm.h is not strictly needed as the enum values are already
correct. The point of specifying the enum values explicitly was to hint that
the values matter and should not be changed. A comment in the source would
probably be more appropriate though.
--
Regards,
Laurent Pinchart
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] usb: phy: samsung-usb2: Toggle HSIC GPIO from device tree
Hi, On Friday 12 of July 2013 09:48:54 Jingoo Han wrote: > On Friday, July 12, 2013 6:46 AM, Julius Werner wrote: > > Hi Jingoo, > > > > Yeah, I followed that discussion back then, but it seems to have > > stalled a little (at least the HSIC patches haven't been picked up in > > any kernel.org repo yet to my knowledge). > > > > The problem is that I think these approaches cannot work reliably. I > > agree that it would be nice to control the HSIC device from its own > > driver, and have spent quite some time playing around with the > > usb/misc/usb3503.c driver to try to make this work... but there's a > > timing dependency here that you just can't model correctly with > > independent drivers. > > > > If the HSIC device is already active during boot (e.g. because it was > > used by firmware), there's always a chance that the USB stack will > > come up before the driver that resets it does. The device will be > > enumerated as normal, and when the other driver later pulls the reset > > signal the USB stack will not notice because there is no real > > disconnect detection on HSIC. Only when you eventually try to send > > another transfer to the device will you start to get timeouts, and the > > newly reset device will not be able to reenumerate because the host > > never asks it to. > > > > I really don't see how you could solve this without putting some kind > > of synchronization mechanism in the USB drivers. So this leaves > > ehci-s5p and phy-samsung-usb2 as the only possible places, and I chose > > the latter since all the host-side HSIC initialization is also there > > already. I think if you think of it as "reset whatever is on the other > > side of this PHY", it's okay to put it as an optional feature into the > > PHY driver. > > CC'ed Tomasz Figa, Dongjin Kim, Yulgon Kim > > > Hi Tomasz, Dongjin, > > Julius Werner wants to put 'SMSC 3503 hub reset on Arndale board' > to 'phy-samsung-usb*.c' files, because there is a timing dependency > above mentioned. > The following is the original patch sent by Julius Werner two day ago. > (http://www.spinics.net/lists/linux-samsung-soc/msg20250.html) Well, I think this is simply broken. Following are the reasons why I think so: a) you can use the smsc3503 chip on any USB/HSIC controller and with any USB/HSIC PHY, so you would have to add such reset GPIO to _every_ PHY or controller driver, b) you might want to use other USB/HSIC hubs like this one that requires some initialization sequence, other than just toggling a GPIO, so you would have to add cases for all of those hubs or other chips in _every_ PHY or controller driver. > Previously, Olof mentioned that 'drivers/platform/arm/' would be used. > (http://patches.linaro.org/16856/) > > Also, another way was mentioned by Fabio Estevam, using > 'drivers/reset/gpio-reset.c' which is not merged yet. > (http://permalink.gmane.org/gmane.linux.drivers.devicetree/36830) > > I think that 'phy-samsung-usb*.c' files are not a good place. > However, Julius Werner's comment looks reasonable enough. I can see this problem almost equivalent to the problem with on-board WLAN adapters connected using SDIO. Those adapters require some kind of power on sequence before they can be enumerated using standard MMC/SDIO mechanisms and so does this USB/HSIC hub. So basically this is a thing that we should consider on a more generic level, not just for this particular single chip. CCing some extra people to increase chance of getting more opinions on this. Best regards, Tomasz ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v3] Input: sysrq - DT binding for key sequences
From: "Mathieu J. Poirier"
Adding a simple device tree binding for the specification of key sequences.
Definition of the keys found in the sequence are located in
'include/uapi/linux/input.h'.
For the sysrq driver, holding the sequence of keys down for a specific amount
of time
will reset the system.
Signed-off-by: Mathieu Poirier
---
changes for v3:
- Simplified binding definition.
- Renamed binding and moved to chosen.
---
.../devicetree/bindings/input/input-reset.txt | 34 +
drivers/tty/sysrq.c| 58 ++
2 files changed, 92 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/input-reset.txt
diff --git a/Documentation/devicetree/bindings/input/input-reset.txt
b/Documentation/devicetree/bindings/input/input-reset.txt
new file mode 100644
index 000..79504af
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input-reset.txt
@@ -0,0 +1,34 @@
+Input: sysrq reset sequence
+
+A simple binding to represent a set of keys as described in
+include/uapi/linux/input.h. This is to communicate a
+sequence of keys to the sysrq driver. Upon holding the keys
+for a specified amount of time (if specified) the system is
+sync'ed and reset.
+
+Key sequences are global to the system but all the keys in a
+set must be coming from the same input device.
+
+The /chosen node should contain a 'linux,sysrq-reset-seq' child
+node to define a set of keys.
+
+Required property:
+sysrq-reset-seq: array of keycodes
+
+Optional property:
+timeout-ms: duration keys must be pressed together in microseconds
+before generating a sysrq
+
+Example:
+
+ chosen {
+linux,sysrq-reset-seq {
+keyset = <0x03
+ 0x04
+ 0x0a>;
+timeout-ms = <3000>;
+};
+ };
+
+Would represent KEY_2, KEY_3 and KEY_9.
+
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index b51c154..4b77adf 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include
@@ -671,6 +672,54 @@ static void sysrq_detect_reset_sequence(struct sysrq_state
*state,
}
}
+static void sysrq_of_get_keyreset_config(void)
+{
+ unsigned short key;
+ struct device_node *np;
+ const struct property *prop;
+ const __be32 *val;
+ int count, i;
+
+ np = of_find_node_by_path("/chosen");
+ if (!np)
+ goto out;
+
+ np = of_find_node_by_name(np, "linux,sysrq-reset-seq");
+ if (!np) {
+ pr_debug("No sysrq node found");
+ goto out;
+ }
+
+ prop = of_find_property(np, "keyset", NULL);
+ if (!prop || !prop->value) {
+ pr_err("Invalid input keyset");
+ goto out;
+ }
+
+ count = prop->length / sizeof(u32);
+ val = prop->value;
+
+ if (count > SYSRQ_KEY_RESET_MAX)
+ count = SYSRQ_KEY_RESET_MAX;
+
+ /* reset in case a __weak definition was present */
+ sysrq_reset_seq_len = 0;
+
+ for (i = 0; i < count; i++) {
+ key = (unsigned short)be32_to_cpup(val++);
+ if (key == KEY_RESERVED || key > KEY_MAX)
+ break;
+
+ sysrq_reset_seq[sysrq_reset_seq_len++] = key;
+ }
+
+ /* get reset timeout if any */
+ of_property_read_u32(np, "timeout-ms", &sysrq_reset_downtime_ms);
+
+ out:
+ ;
+}
+
static void sysrq_reinject_alt_sysrq(struct work_struct *work)
{
struct sysrq_state *sysrq =
@@ -688,6 +737,7 @@ static void sysrq_reinject_alt_sysrq(struct work_struct
*work)
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 1);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
+
input_inject_event(handle, EV_KEY, KEY_SYSRQ, 0);
input_inject_event(handle, EV_KEY, alt_code, 0);
input_inject_event(handle, EV_SYN, SYN_REPORT, 1);
@@ -903,6 +953,7 @@ static inline void sysrq_register_handler(void)
int error;
int i;
+ /* first check if a __weak interface was instantiated */
for (i = 0; i < ARRAY_SIZE(sysrq_reset_seq); i++) {
key = platform_sysrq_reset_seq[i];
if (key == KEY_RESERVED || key > KEY_MAX)
@@ -911,6 +962,13 @@ static inline void sysrq_register_handler(void)
sysrq_reset_seq[sysrq_reset_seq_len++] = key;
}
+ /*
+* DT configuration takes precedence over anything
+* that would have been defined via the __weak
+* interface
+*/
+ sysrq_of_get_keyreset_config();
+
error = input_register_handler(&sysrq_handler);
if (error)
pr_err("Failed to register input handler, error %d", error);
--
1.8.1.2
___
devicetree-
Re: [PATCH 0/2] ARM: sunxi: Convert DTSI to new CPU bindings
Hi Lorenzo, On Fri, Jul 05, 2013 at 11:19:46AM +0100, Lorenzo Pieralisi wrote: > On Sun, Jun 30, 2013 at 10:48:46AM +0100, Lorenzo Pieralisi wrote: > > On Sat, Jun 29, 2013 at 08:38:19PM +0100, Russell King - ARM Linux wrote: > > > On Fri, Jun 28, 2013 at 01:05:42PM -0700, Olof Johansson wrote: > > > > On Fri, Jun 28, 2013 at 1:03 PM, Maxime Ripard > > > > wrote: > > > > > On Fri, Jun 28, 2013 at 06:15:32PM +0100, Lorenzo Pieralisi wrote: > > > > >> The patch above should already be queued in next/dt right ? > > > > > > > > > > Indeed. > > > > > > > > > > Then why the latest patch of your patchset got in 3.10, while the > > > > > patches actually fixing the DT it would have impacted were delayed to > > > > > 3.11? > > > > > > > > > > (And why was it merged so late in the development cycle?) > > > > > > > > This. So now we have to scramble because some device trees will > > > > produce warnings at boot. > > > > > > > > Russell, the alternative is to revert Lorenzo's patch for 3.10 (and > > > > re-introduce it for 3.11). Do you have a preference? > > > > > > Sorry but I really don't understand what all the fuss in this thread > > > is about. > > > > > > This thread seems to be saying that two development patches were > > > merged, which were 7762/1 and 7763/1, and that 7764/1 is a fix? > > > Are you sure about that, because that's not how they're described, > > > and not how they look either. > > > > As Olof's warning downgrade is being merged (thanks for that and apologies > > for > > failing to explain patches dependencies properly and stable related issues), > > 7764/1 won't apply cleanly anymore. Can you please drop it from the patch > > system, I will update it and test it first thing tomorrow and send a > > final version to the patch system. > > Patch 7779/1, replacing 7764/1 is in the patch system now, and is ready > to get merged. > > Unfortunately cpu/cpus bindings documentation updates, following: > > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036735.html > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-May/033779.html > > were not pulled in the kernel. This is an issue since this means that > we still have no reference in the kernel or wherever it has to be, to > the final cpus/cpu bindings for ARM and ARM64 provided in the pull > request link above (that has been reviewed to death and acknowledged). > > It is a significant overhaul of cpu/cpus bindings standard for ARM/ARM64, > covering all CPUs harking back to arm926 and beyond, and should be final. > > dts updates following that standard have already been pulled into 3.11 > through arm-soc. > > IMHO the bindings contained in pull request above must be merged in the > kernel asap, I would like to ask you please what should I do to get them in > please. If we want to move bindings documentation elsewhere let's do it, > as long as there is a published standard I am happy and will stop annoying > you with this stuff. Just to be clear, I had no problems with the patches themselves, but just the way it was merged. That being said, I think every DTS patch you did should be merged by now, only the second patch of this serie for the A10S hasn't. Arnd, Olof, could you just apply the patch 2 for a 3.11-rc*? It's the only rc patch for the sunxi platform for now, so I don't think a pull request would be worth it, but I can send one anyway if you prefer. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] ARM i.MX53: mba53: Fix PWM backlight DT node
On Thu, Jul 11, 2013 at 04:37:47PM +0200, Laurent Pinchart wrote: > The i.MX53 PWM controller uses two cells to describe the PWM specifier. > Remove the extra unused values from the backlight DT node pwms property. > > Signed-off-by: Laurent Pinchart Applied, thanks. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
