* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Kevin Hilman @ 2016-12-01 0:14 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <20161130225136.GO16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> Hi Kevin,
>
> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>> Hi Sakari,
>>
>> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
>>
>> > On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>> >> Allow getting of subdevs from DT ports and endpoints.
>> >>
>> >> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> >
>> > vpif_capture_get_pdata and "largely"?
>>
>> Yes, thanks.
>>
>> >> am437x-vpfe.c
>> >>
>> >> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> >> ---
>> >> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
>> >> include/media/davinci/vpif_types.h | 9 +-
>> >> 2 files changed, 133 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> >> index 94ee6cf03f02..47a4699157e7 100644
>> >> --- a/drivers/media/platform/davinci/vpif_capture.c
>> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> >> @@ -26,6 +26,8 @@
>> >> #include <linux/slab.h>
>> >>
>> >> #include <media/v4l2-ioctl.h>
>> >> +#include <media/v4l2-of.h>
>> >> +#include <media/i2c/tvp514x.h>
>> >
>> > Do you need this header?
>> >
>>
>> Yes, based on discussion with Hans, since there is no DT binding for
>> selecting the input pins of the TVP514x, I have to select it in the
>> driver, so I need the defines from this header. More on this below...
>>
>> >>
>> >> #include "vpif.h"
>> >> #include "vpif_capture.h"
>> >> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>> >>
>> >> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>> >>
>> >> + if (!chan_cfg)
>> >> + return -1;
>> >> + if (input_index >= chan_cfg->input_count)
>> >> + return -1;
>> >> subdev_name = chan_cfg->inputs[input_index].subdev_name;
>> >> if (subdev_name == NULL)
>> >> return -1;
>> >> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
>> >> /* loop through the sub device list to get the sub device info */
>> >> for (i = 0; i < vpif_cfg->subdev_count; i++) {
>> >> subdev_info = &vpif_cfg->subdev_info[i];
>> >> - if (!strcmp(subdev_info->name, subdev_name))
>> >> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>> >> return i;
>> >> }
>> >> return -1;
>> >> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>> >> {
>> >> int i;
>> >>
>> >> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>> >> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
>> >> + const struct device_node *node = _asd->match.of.node;
>> >> +
>> >> + if (node == subdev->of_node) {
>> >> + vpif_obj.sd[i] = subdev;
>> >> + vpif_obj.config->chan_config->inputs[i].subdev_name =
>> >> + (char *)subdev->of_node->full_name;
>> >> + vpif_dbg(2, debug,
>> >> + "%s: setting input %d subdev_name = %s\n",
>> >> + __func__, i, subdev->of_node->full_name);
>> >> + return 0;
>> >> + }
>> >> + }
>> >> +
>> >> for (i = 0; i < vpif_obj.config->subdev_count; i++)
>> >> if (!strcmp(vpif_obj.config->subdev_info[i].name,
>> >> subdev->name)) {
>> >> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
>> >> return vpif_probe_complete();
>> >> }
>> >>
>> >> +static struct vpif_capture_config *
>> >> +vpif_capture_get_pdata(struct platform_device *pdev)
>> >> +{
>> >> + struct device_node *endpoint = NULL;
>> >> + struct v4l2_of_endpoint bus_cfg;
>> >> + struct vpif_capture_config *pdata;
>> >> + struct vpif_subdev_info *sdinfo;
>> >> + struct vpif_capture_chan_config *chan;
>> >> + unsigned int i;
>> >> +
>> >> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
>> >> +
>> >> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> >> + return pdev->dev.platform_data;
>> >> +
>> >> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> >> + if (!pdata)
>> >> + return NULL;
>> >> + pdata->subdev_info =
>> >> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
>> >> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
>> >> +
>> >> + if (!pdata->subdev_info)
>> >> + return NULL;
>> >> + dev_dbg(&pdev->dev, "%s\n", __func__);
>> >> +
>> >> + for (i = 0; ; i++) {
>> >> + struct device_node *rem;
>> >> + unsigned int flags;
>> >> + int err;
>> >> +
>> >> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
>> >> + endpoint);
>> >> + if (!endpoint)
>> >> + break;
>> >> +
>> >> + sdinfo = &pdata->subdev_info[i];
>> >
>> > subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
>> >
>>
>> Right, I need to make the loop only go for a max of
>> VPIF_CAPTURE_MAX_CHANNELS iterations.
>>
>> >> + chan = &pdata->chan_config[i];
>> >> + chan->inputs = devm_kzalloc(&pdev->dev,
>> >> + sizeof(*chan->inputs) *
>> >> + VPIF_DISPLAY_MAX_CHANNELS,
>> >> + GFP_KERNEL);
>> >> +
>> >> + chan->input_count++;
>> >> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
>> >
>> > I wonder what's the purpose of using index i on this array as well.
>>
>> The number of endpoints in DT is the number of input channels configured
>> (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
>>
>> > If you use that to access a corresponding entry in a different array, I'd
>> > just create a struct that contains the port configuration and the async
>> > sub-device. The omap3isp driver does that, for instance; see
>> > isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
>> > interested. Up to you.
>>
>> OK, I'll have a look at that driver. The goal here with this series is
>> just to get this working with DT, but also not break the existing legacy
>> platform_device support, so I'm trying not to mess with the
>> driver-interal data structures too much.
>
> Ack.
>
>>
>> >> + chan->inputs[i].input.std = V4L2_STD_ALL;
>> >> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
>> >> +
>> >> + /* FIXME: need a new property? ch0:composite ch1: s-video */
>> >> + if (i == 0)
>> >
>> > Can you assume that the first endopoint has got a particular kind of input?
>> > What if it's not connected?
>>
>> On all the boards I know of (there aren't many using this SoC), it's a
>> safe assumption.
>>
>> > If this is a different physical port (not in the meaning another) in the
>> > device, I'd use the reg property for this. Please see
>> > Documentation/devicetree/bindings/media/video-interfaces.txt .
>>
>> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
>> that it's not physically a different port. Instead, it's just telling
>> the TVP514x which pin(s) will be active inputs (and what kind of signal
>> will be present.)
>>
>> I'm open to a better way to describe this input select from DT, but
>> based on what I heard from Hans, there isn't currently a good way to do
>> that except for in the driver:
>> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
>>
>> Based on further discussion in that thread, it sounds like there may be
>> a way forward coming soon, and I'll be glad to switch to that when it
>> arrives.
>
> I'm not sure that properly supporting connectors will provide any help here.
>
> Looking at the s_routing() API, it's the calling driver that has to be aware
> of sub-device specific function parameters. As such it's not a very good
> idea to require that a driver is aware of the value range of another
> driver's parameter. I wonder if a simple enumeration interface would help
> here --- if I understand correctly, the purpose is just to provide a way to
> choose the input using VIDIOC_S_INPUT.
>
> I guess that's somehow ok as long as you have no other combinations of these
> devices but this is hardly future-proof. (And certainly not a problem
> created by this patch.)
Yeah, this is far from future proof.
> It'd be still nice to fix that as presumably we don't have the option of
> reworking how we expect the device tree to look like.
Agreed.
I'm just hoping someone can shed som light on "how we expect the device
tree to look". ;)
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation
From: Kevin Hilman @ 2016-11-30 23:48 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, linux-media@vger.kernel.org, Hans Verkuil,
devicetree@vger.kernel.org, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
g.liakhovetski-Mmb7MZpHnFY,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <20161130214835.GN16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> Hi Rob and Kevin,
>
> On Tue, Nov 29, 2016 at 08:41:44AM -0600, Rob Herring wrote:
>> On Mon, Nov 28, 2016 at 4:30 PM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>> > Hi Rob,
>> >
>> > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>> >
>> >> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
>> >>> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> >>> ---
>> >>> .../bindings/media/ti,da850-vpif-capture.txt | 65 ++++++++++++++++++++++
>> >>> .../devicetree/bindings/media/ti,da850-vpif.txt | 8 +++
>> >>> 2 files changed, 73 insertions(+)
>> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>> new file mode 100644
>> >>> index 000000000000..c447ac482c1d
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
>> >>> @@ -0,0 +1,65 @@
>> >>> +Texas Instruments VPIF Capture
>> >>> +------------------------------
>> >>> +
>> >>> +The TI Video Port InterFace (VPIF) capture component is the primary
>> >>> +component for video capture on the DA850 family of TI DaVinci SoCs.
>> >>> +
>> >>> +TI Document number reference: SPRUH82C
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: must be "ti,da850-vpif-capture"
>> >>> +- reg: physical base address and length of the registers set for the device;
>> >>> +- interrupts: should contain IRQ line for the VPIF
>> >>> +
>> >>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
>> >>> +channels or a single 16-bit channel. It should contain at least one
>> >>> +port child node with child 'endpoint' node. Please refer to the
>> >>> +bindings defined in
>> >>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> >>> +
>> >>> +Example using 2 8-bit input channels, one of which is connected to an
>> >>> +I2C-connected TVP5147 decoder:
>> >>> +
>> >>> + vpif_capture: video-capture@0x00217000 {
>> >>> + reg = <0x00217000 0x1000>;
>> >>> + interrupts = <92>;
>> >>> +
>> >>> + port {
>> >>> + vpif_ch0: endpoint@0 {
>> >>> + reg = <0>;
>> >>> + bus-width = <8>;
>> >>> + remote-endpoint = <&composite>;
>> >>> + };
>> >>> +
>> >>> + vpif_ch1: endpoint@1 {
>> >>
>> >> I think probably channels here should be ports rather than endpoints.
>> >> AIUI, having multiple endpoints is for cases like a mux or 1 to many
>> >> connections. There's only one data flow, but multiple sources or sinks.
>> >
>> > Looking at this closer... , I used an endpoint because it's bascially a
>> > 16-bit parallel bus, that can be configured as (up to) 2 8-bit
>> > "channels. So, based on the video-interfaces.txt doc, I configured this
>> > as a single port, with (up to) 2 endpoints. That also allows me to
>> > connect output of the decoder directly, using the remote-endpoint
>> > property.
>> >
>> > So I guess I'm not fully understanding your suggestion.
>>
>> NM, looks like video-interfaces.txt actually spells out this case and
>> defines doing it as you did.
>
> It's actually the first time I read that portion (at least so that I could
> remember) of video-interfaces.txt. I'm not sure if anyone has implemented
> that previously, nor how we ended up with the text. The list archive could
> probably tell. Cc Guennadi who wrote it. :-) I couldn't immediately find DT
> source with this arrangement.
>
> In case of splitting the port into two parallel interfaces, how do you
> determine which wires belong to which endpoint? I guess they'd be particular
> sets of wires but as there's just a single port it isn't defined by the
> port.
Isn't that the point of data-shift?
e.g. it's a single 16-bit parallel bus, where the lower 8 bits are for
channel 0 and the upper 8 bits are for channel 1. Alternately, the port
can also be configured as a single 16-bit channel (e.g. for raw
capture.)
If you want more details on this hardware, it's pretty well described in
Chapter 35 of http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf.
FWIW, I'm not really picky about how to do this. I'm trying to learn
"the right way" and am happy to do that, but the feedback so far has
been confusing (at least for someone relatively new to the DT side of
the media framework.)
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
From: Rob Herring @ 2016-11-30 23:30 UTC (permalink / raw)
To: Stephen Boyd
Cc: Frank Rowand, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org, Pantelis Antoniou, Linus Walleij,
Mark Brown
In-Reply-To: <20161124102529.20212-2-stephen.boyd@linaro.org>
On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
>
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
>
> soc {
> soc_gpio1: gpio-controller1 {
> #gpio-cells = <2>;
> };
>
> soc_gpio2: gpio-controller2 {
> #gpio-cells = <2>;
> };
> };
>
> connector: connector {
> #gpio-cells = <2>;
> gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
> <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
> <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
> <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
> gpio-map-mask = <0xf 0x1>;
I think the common case is something more like this:
gpio-map = <0 0 &soc_gpio1 1 0>,
<1 0 &soc_gpio2 4 0>,
<2 0 &soc_gpio1 3 0>,
<3 0 &soc_gpio2 2 0>;
gpio-map-mask = <0xf 0>;
where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
&soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
combination or it will be specific to the daughterboard's usage.
Also, GPIO cells are pretty well standardized, but some cases may need
a translation function which I guess would be part of a connector
driver. I don't think that affects the binding nor needs to be solved
now, but just want to raise that possibility.
> };
>
> expansion_device {
> reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
> };
>
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
GPIOs also are interrupts frequently, so we need to make sure
interrupt translation works too. It's a bit tricky as interrupt-map
depends on #address-cells and #interrupt-cells. I think we just set
the #address-cells to 0 on the connector node and it will be fine. We
may need the same pass thru of flags though.
>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> drivers/of/base.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h | 14 +++++
> 2 files changed, 160 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
> EXPORT_SYMBOL(of_parse_phandle_with_args);
>
> /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np: pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name: property name that specifies phandles' arguments count
> + * @index: index of a phandle to parse out
> + * @out_args: optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + * #list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + * #list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + * #list-cells = <1>;
> + * list-map = <0 &phandle2 3>,
> + * <1 &phandle2 2>,
> + * <2 &phandle1 5 1>;
> + * list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + * list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + * "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> + const char *list_name,
> + const char *cells_name,
> + const char *map_name,
> + const char *mask_name,
Perhaps these 3 could be just a single base name (e.g. "gpio")?
Doesn't really buy much other than enforce we don't mix 'gpios' and
'gpio'. That could never happen. ;)
> + int index, struct of_phandle_args *out_args)
> +{
Rob
^ permalink raw reply
* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Sakari Ailus @ 2016-11-30 22:51 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <m2a8cpvkwj.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Hi Kevin,
On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
> Hi Sakari,
>
> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
>
> > On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
> >> Allow getting of subdevs from DT ports and endpoints.
> >>
> >> The _get_pdata() function was larely inspired by (i.e. stolen from)
> >
> > vpif_capture_get_pdata and "largely"?
>
> Yes, thanks.
>
> >> am437x-vpfe.c
> >>
> >> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >> ---
> >> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
> >> include/media/davinci/vpif_types.h | 9 +-
> >> 2 files changed, 133 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> >> index 94ee6cf03f02..47a4699157e7 100644
> >> --- a/drivers/media/platform/davinci/vpif_capture.c
> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
> >> @@ -26,6 +26,8 @@
> >> #include <linux/slab.h>
> >>
> >> #include <media/v4l2-ioctl.h>
> >> +#include <media/v4l2-of.h>
> >> +#include <media/i2c/tvp514x.h>
> >
> > Do you need this header?
> >
>
> Yes, based on discussion with Hans, since there is no DT binding for
> selecting the input pins of the TVP514x, I have to select it in the
> driver, so I need the defines from this header. More on this below...
>
> >>
> >> #include "vpif.h"
> >> #include "vpif_capture.h"
> >> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
> >>
> >> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
> >>
> >> + if (!chan_cfg)
> >> + return -1;
> >> + if (input_index >= chan_cfg->input_count)
> >> + return -1;
> >> subdev_name = chan_cfg->inputs[input_index].subdev_name;
> >> if (subdev_name == NULL)
> >> return -1;
> >> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
> >> /* loop through the sub device list to get the sub device info */
> >> for (i = 0; i < vpif_cfg->subdev_count; i++) {
> >> subdev_info = &vpif_cfg->subdev_info[i];
> >> - if (!strcmp(subdev_info->name, subdev_name))
> >> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
> >> return i;
> >> }
> >> return -1;
> >> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
> >> {
> >> int i;
> >>
> >> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> >> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> >> + const struct device_node *node = _asd->match.of.node;
> >> +
> >> + if (node == subdev->of_node) {
> >> + vpif_obj.sd[i] = subdev;
> >> + vpif_obj.config->chan_config->inputs[i].subdev_name =
> >> + (char *)subdev->of_node->full_name;
> >> + vpif_dbg(2, debug,
> >> + "%s: setting input %d subdev_name = %s\n",
> >> + __func__, i, subdev->of_node->full_name);
> >> + return 0;
> >> + }
> >> + }
> >> +
> >> for (i = 0; i < vpif_obj.config->subdev_count; i++)
> >> if (!strcmp(vpif_obj.config->subdev_info[i].name,
> >> subdev->name)) {
> >> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
> >> return vpif_probe_complete();
> >> }
> >>
> >> +static struct vpif_capture_config *
> >> +vpif_capture_get_pdata(struct platform_device *pdev)
> >> +{
> >> + struct device_node *endpoint = NULL;
> >> + struct v4l2_of_endpoint bus_cfg;
> >> + struct vpif_capture_config *pdata;
> >> + struct vpif_subdev_info *sdinfo;
> >> + struct vpif_capture_chan_config *chan;
> >> + unsigned int i;
> >> +
> >> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> >> +
> >> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> >> + return pdev->dev.platform_data;
> >> +
> >> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> >> + if (!pdata)
> >> + return NULL;
> >> + pdata->subdev_info =
> >> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
> >> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> >> +
> >> + if (!pdata->subdev_info)
> >> + return NULL;
> >> + dev_dbg(&pdev->dev, "%s\n", __func__);
> >> +
> >> + for (i = 0; ; i++) {
> >> + struct device_node *rem;
> >> + unsigned int flags;
> >> + int err;
> >> +
> >> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> >> + endpoint);
> >> + if (!endpoint)
> >> + break;
> >> +
> >> + sdinfo = &pdata->subdev_info[i];
> >
> > subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
> >
>
> Right, I need to make the loop only go for a max of
> VPIF_CAPTURE_MAX_CHANNELS iterations.
>
> >> + chan = &pdata->chan_config[i];
> >> + chan->inputs = devm_kzalloc(&pdev->dev,
> >> + sizeof(*chan->inputs) *
> >> + VPIF_DISPLAY_MAX_CHANNELS,
> >> + GFP_KERNEL);
> >> +
> >> + chan->input_count++;
> >> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> >
> > I wonder what's the purpose of using index i on this array as well.
>
> The number of endpoints in DT is the number of input channels configured
> (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
>
> > If you use that to access a corresponding entry in a different array, I'd
> > just create a struct that contains the port configuration and the async
> > sub-device. The omap3isp driver does that, for instance; see
> > isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
> > interested. Up to you.
>
> OK, I'll have a look at that driver. The goal here with this series is
> just to get this working with DT, but also not break the existing legacy
> platform_device support, so I'm trying not to mess with the
> driver-interal data structures too much.
Ack.
>
> >> + chan->inputs[i].input.std = V4L2_STD_ALL;
> >> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> >> +
> >> + /* FIXME: need a new property? ch0:composite ch1: s-video */
> >> + if (i == 0)
> >
> > Can you assume that the first endopoint has got a particular kind of input?
> > What if it's not connected?
>
> On all the boards I know of (there aren't many using this SoC), it's a
> safe assumption.
>
> > If this is a different physical port (not in the meaning another) in the
> > device, I'd use the reg property for this. Please see
> > Documentation/devicetree/bindings/media/video-interfaces.txt .
>
> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
> that it's not physically a different port. Instead, it's just telling
> the TVP514x which pin(s) will be active inputs (and what kind of signal
> will be present.)
>
> I'm open to a better way to describe this input select from DT, but
> based on what I heard from Hans, there isn't currently a good way to do
> that except for in the driver:
> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
>
> Based on further discussion in that thread, it sounds like there may be
> a way forward coming soon, and I'll be glad to switch to that when it
> arrives.
I'm not sure that properly supporting connectors will provide any help here.
Looking at the s_routing() API, it's the calling driver that has to be aware
of sub-device specific function parameters. As such it's not a very good
idea to require that a driver is aware of the value range of another
driver's parameter. I wonder if a simple enumeration interface would help
here --- if I understand correctly, the purpose is just to provide a way to
choose the input using VIDIOC_S_INPUT.
I guess that's somehow ok as long as you have no other combinations of these
devices but this is hardly future-proof. (And certainly not a problem
created by this patch.)
It'd be still nice to fix that as presumably we don't have the option of
reworking how we expect the device tree to look like.
Cc Laurent.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: Arnd Bergmann @ 2016-11-30 22:51 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Sudeep Holla, Jeremy Linton, devicetree-u79uwXL29TY76Z2rM5mHXA,
mark.rutland-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, liviu.dudau-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <92ac8013-b88e-2f74-9a49-7d5f38a4e7a5-5wv7dgnIgG8@public.gmane.org>
On Wednesday, November 30, 2016 4:29:35 PM CET Sudeep Holla wrote:
> Hi Jeremy,
>
> On 29/11/16 20:45, Jeremy Linton wrote:
> > The PCIe root complex on Juno translates the MMIO mapped
> > at 0x5f800000 to the PIO address range starting at 0
> > (which is common because PIO addresses are generally < 64k).
> > Correct the DT to reflect this.
> >
>
> I have another DT fix that I have asked ARM-SoC guys to pick up directly
> from the list. If that doesn't happen, I will send PR including both.
>
> If that happens then we need to send this to them to be picked directly.
> At this point I want to wait for couple of days to avoid confusion.
I ended up taking the other one for v4.10, but this one seems more
important so I applied it for v4.9.
Let me know if you disagree with the priorities, as I plan to send out
the last 4.9 fixes pull request to Linus tomorrow.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/5] Meson GXL and GXM USB support
From: Martin Blumenstingl @ 2016-11-30 22:49 UTC (permalink / raw)
To: Rob Herring
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, narmstrong-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20161130222228.zu6ampaajg3gkdz4@rob-hp-laptop>
On Wed, Nov 30, 2016 at 11:22 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sun, Nov 27, 2016 at 11:42:02PM +0100, Martin Blumenstingl wrote:
>> Hello Kishon,
>>
>> On Sat, Nov 26, 2016 at 3:56 PM, Martin Blumenstingl
>> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
>> > USB support on GXL and GXM differs a lot from Meson8b and GXBB:
>> > The most obvious change is that GXL and GXM now have one dwc3
>> > controller and one dwc2 controller (instead of two dwc2 controllers).
>> > With that there are also new USB PHYs.
>> >
>> > Due to lack of hardware I was only able to test this on a board with
>> > GXM, but as far as I understand the hardware my preparations should be
>> > correct (so it should also work on GXL).
>> >
>> > dwc2 will probably stay unused on most GXM devices since it's limited
>> > to device mode via some dwc2 hardware configuration register.
>> >
>> > dwc3 is probably used on all devices, even if there is more than just
>> > one USB port. dwc3 has a built-in USB2 hub - on GXL this hub has two
>> > ports enabled, while on GXM there are three ports enabled (see below
>
> This hub is an actual USB hub? If so, then you should probably model the
> USB bus topology (which we have a binding definition for).
(the following explanation is based on a) what I found is going on in
the hardware registers b) reading the vendor drivers - unfortunately
there are no datasheets available which could give more details).
lsusb on my GXM gives:
...
Hub Port Status:
Port 1: 0000.0100 power
Port 2: 0000.0100 power
Port 3: 0000.0100 power
The layout looks like this:
dwc3 provides a USB hub with 2 (on GXL) or 3 (on GXM) USB ports.
Each of the port is driven by a PHY (port 1 = abp@0x78000, port2 =
abp@0x78020, etc...).
On GXM USB2 PHY port 3 = abp@0x78040 is connected to the third dwc3 hub port.
On GXL PHY port 3 = abp@0x78040 is connected to the dwc2 (I could not
prove this yet as I don't have access to any GXL hardware).
So the answer is: we don't have an actual USB hub here (as this hub is
provided by dwc3), but rather a set of PHYs which is assigned to
dwc3's hub (if we don't configure *all* PHYs then none of the USB
ports provided by the dwc3 hub works).
Could you please point me to the USB bus topology binding (is it the
one described in usb-device.txt)?
>> > for lsusb output). There are no USB3 ports enabled in the dwc3 hardware
>> > configuration, meaning that the SoC is limited to high-speed mode.
>> > On my GXM device the dwc3 hardware configuration forces it into "host
>> > only" mode.
>> >
>> > The SoCs contain two PHY blocks: one USB3 PHY and up to four USB2 PHYs
>> > (on GXM there are only three enabled, but the registers should support
>> > up to four).
>> > The USB3 PHY also handles the OTG interrupts, but since dwc3's hardware
>> > configuration enforces "host only" mode I was not able to test this. It
>> > simply takes care of an interrupt and then notifies all related PHYs
>> > about the new mode.
>> > The USB2 PHY block is a bit different: I created one PHY driver which
>> > spans all "PHY ports" because the handling is a bit tricky. It turns
>> > out that for each available USB port in dwc3's hub the corresponding
>> > PHY must be enabled (even if there is no physical port - in my case
>> > port 3 is not connected to anything, but disabling the PHY breaks
>> > ports 1 and 2 as well).
>> > I decided not not pass the USB2 PHYs directly to dwc3 due to three
>> > reasons: 1. the USB3 PHY (which holds a reference to all relevant
>> > USB2 PHY ports) controls the mode of the USB2 PHY ports (since both
>> > are used with the same controller and thus it makes sense to keep the
>> > mode consistent across all ports) 2. the dwc3 driver does not support
>> > passing multiple USB2 PHYs (only one USB2 and one USB3 PHY can be
>> > passed to it) 3. it is similar to how the vendor reference driver
>> > manages the PHYs. Please note that this coupling is not a fixed, this
>> > is all configurable via devicetree (so if the third USB2 PHY has to
>> > be passed two the dwc2 controller then this is still possible by
>> > just moving on PHY reference in the .dts).
>> after not staring at my own code for 24 hours I realized this:
>> (I went through quite a few iterations before getting these drivers to work)
>> I'm basically re-modelling an "USB PHY hub" with my USB3 PHY driver
>> (there's one "upstream" PHY interface which is passed to dwc3 and
>> multiple downstream PHYs, each for one port on dwc3's internal hub).
>> With this approach I could split each of the the USB2s into separate
>> nodes again (instead of one devicetree node with #phy-cells = <1>) as
>> the USB3 PHY is taking care of that special "we have to enable all
>> ports or no port will be usable".
>>
>> We could go even one step further: why implement this in the Meson GXL
>> specific PHY driver - why not implement a generic "phy-hub" driver
>> (which would be valid whenever the PHY controller has to manage
>> multiple PHYs at once, but wants to keep them all in a consistent
>> state).
>> The devicetree could look like this:
>> usb2_phy_hub: phy@0 {
>> compatible = "phy-hub";
>> phys = <&other_phy1>, <&other_phy 2>;
>> };
>>
>> &dwc3 {
>> phys = <&usb2_phy_hub>, <&usb3_phy0>;
>> phy-names = "usb2-phy", "usb3-phy";
>> };
>
> I'm okay with a hub if it is modeled as a USB hub. Here though, it
> looks like you are just trying to group things which doesn't need to be
> in DT.
I hope my answer above makes things more clear
>> The generic phy-hub driver would then implement all phy_ops callbacks
>> and pass then to each of it's downstream PHYs.
>
> You can have generic drivers without a generic binding.
So you'd rather implement a generic driver which would provide
functions like of_create_grouped_phy(struct device_node *np) which
would implement the grouping logic as described (but has no binding
itself, but rather relies on the actual platform driver taking care of
creating that binding and re-using generic code)?
>> That's just what came into my head - please let me know what you think
>> of this or share your ideas on how to approach this!
>>
>> > The coupling of the USB2 and USB3 PHYs is the reason why I sent the
>> > two drivers in one patch, even though they are handling different IP
>> > blocks (different registers, etc.).
>> >
>> > Unfortunately there are no datasheets available for any of these PHYs.
>> > Both drivers were written by reading the reference drivers provided by
>> > Amlogic and analyzing the registers on the kernel that was shipped with
>> > my board.
>> >
>> > As a last note: the dwc3 driver currently only explicitly enables the
>> > first USB port "DWC3_GUSB2PHYCFG(0)" in the internal hub. The hardware
>> > seems to enable the other two (DWC3_GUSB2PHYCFG(1) and
>> > DWC3_GUSB2PHYCFG(2)) automatically. I will ask the dwc3 maintainers if
>> > changes to dwc3 are desired any how these should look like, but for now
>> > it's working fine even without changes there.
>> >
>> > lsusb output on GXM for the dwc3 hub:
>> > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>> > ...
>> > Hub Port Status:
>> > Port 1: 0000.0100 power
>> > Port 2: 0000.0100 power
>> > Port 3: 0000.0100 power
>> >
>> > NOTE: The devicetree changes depend on my previous series:
>> > "[PATCH 0/2] minor GXL and GXM improvements" - see [0]
>> >
>> > NOTE2: This series depends on an upstream dwc3/xhci-plat DMA fix
>> > (special thanks to Arnd Bergmann and Sriram Dash for fixing that):
>> > "[PATCH v5 0/6] inherit dma configuration from parent dev" - see [1]
>> >
>> > I have a tree with all dependencies applied available at [2] if
>> > someone wants a quick way to test this (I don't take any responsibility
>> > if anything explodes though).
>> >
>> > [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001665.html
>> > [1] http://marc.info/?l=linux-usb&m=147938307209685&w=2
>> > [2] https://github.com/xdarklight/linux/commits/meson-gx-integration-4.10-20161126
>> >
>> > Martin Blumenstingl (5):
>> > Documentation: dt-bindings: Add documentation for Meson GXL USB2/3
>> > PHYs
>> > phy: meson: add USB2 and USB3 PHY support for Meson GXL
>> > arm64: dts: meson-gxl: add USB support
>> > ARM64: dts: meson-gxm: add GXM specific USB configuration
>> > ARM64: dts: meson-gx-p23x-q20x: enable USB on P23x and Q20x boards
>> >
>> > .../devicetree/bindings/phy/meson-gxl-usb2-phy.txt | 25 ++
>> > .../devicetree/bindings/phy/meson-gxl-usb3-phy.txt | 27 ++
>> > .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 12 +
>> > arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 49 +++
>> > .../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 17 +
>> > arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 10 +
>> > drivers/phy/Kconfig | 13 +
>> > drivers/phy/Makefile | 2 +
>> > drivers/phy/phy-meson-gxl-usb2.c | 374 ++++++++++++++++++++
>> > drivers/phy/phy-meson-gxl-usb3.c | 377 +++++++++++++++++++++
>> > 10 files changed, 906 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/phy/meson-gxl-usb2-phy.txt
>> > create mode 100644 Documentation/devicetree/bindings/phy/meson-gxl-usb3-phy.txt
>> > create mode 100644 drivers/phy/phy-meson-gxl-usb2.c
>> > create mode 100644 drivers/phy/phy-meson-gxl-usb3.c
>> >
>> > --
>> > 2.10.2
>> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] gpio: Support gpio nexus dt bindings
From: Rob Herring @ 2016-11-30 22:48 UTC (permalink / raw)
To: Stephen Boyd
Cc: Frank Rowand, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org, Pantelis Antoniou, Linus Walleij,
Mark Brown
In-Reply-To: <20161124102529.20212-4-stephen.boyd@linaro.org>
On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> Now that we have nexus support in the OF core let's change the
> function call here that parses the phandle lists of gpios to use
> the nexus variant. This allows us to remap phandles and their
> arguments through any number of nexus nodes and end up with the
> actual gpio provider being used.
>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>
> TODO: Document gpio-map and gpio-map-mask in GPIO devicetree binding
>
> drivers/gpio/gpiolib-of.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index ecad3f0e3b77..3117397c4c41 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -71,8 +71,9 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
> struct gpio_desc *desc;
> int ret;
>
> - ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
> - &gpiospec);
> + ret = of_parse_phandle_with_args_map(np, propname, "#gpio-cells",
> + "gpio-map", "gpio-map-mask",
> + index, &gpiospec);
> if (ret) {
> pr_debug("%s: can't parse '%s' property of node '%s[%d]'\n",
> __func__, propname, np->full_name, index);
> --
> 2.10.0.297.gf6727b0
>
^ permalink raw reply
* Re: [PATCH V6 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Stephen Boyd @ 2016-11-30 22:37 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Viresh Kumar, Nishanth Menon,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3b3ce1dac198fb668b97e81490bcbcf03d31e40d.1480481312.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 11/30, Viresh Kumar wrote:
> @@ -1337,26 +1408,44 @@ struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name)
> goto err;
> }
>
> - /* Already have a regulator set */
> - if (WARN_ON(!IS_ERR(opp_table->regulator))) {
> + /* Already have regulators set */
> + if (WARN_ON(opp_table->regulators)) {
> opp_table = ERR_PTR(-EBUSY);
> goto err;
> }
> - /* Allocate the regulator */
> - reg = regulator_get_optional(dev, name);
> - if (IS_ERR(reg)) {
> - opp_table = (struct opp_table *)reg;
> - if (PTR_ERR(reg) != -EPROBE_DEFER)
> - dev_err(dev, "%s: no regulator (%s) found: %ld\n",
> - __func__, name, PTR_ERR(reg));
> +
> + opp_table->regulators = kmalloc_array(count,
> + sizeof(*opp_table->regulators),
> + GFP_KERNEL);
> + if (!opp_table->regulators) {
> + opp_table = ERR_PTR(-ENOMEM);
> goto err;
> }
>
> - opp_table->regulator = reg;
> + for (i = 0; i < count; i++) {
> + reg = regulator_get_optional(dev, names[i]);
> + if (IS_ERR(reg)) {
> + opp_table = (struct opp_table *)reg;
This should be an ERR_CAST() instead. But this is not based on
the correct patch anyway so this should be rebased.
> + if (PTR_ERR(reg) != -EPROBE_DEFER)
> + dev_err(dev, "%s: no regulator (%s) found: %ld\n",
> + __func__, names[i], PTR_ERR(reg));
> + goto free_regulators;
> + }
> +
> + opp_table->regulators[i] = reg;
> + }
> +
> + opp_table->regulator_count = count;
>
> mutex_unlock(&opp_table_lock);
> return opp_table;
>
> +free_regulators:
> + while (i != 0)
> + regulator_put(opp_table->regulators[--i]);
> +
> + kfree(opp_table->regulators);
> + opp_table->regulators = NULL;
> err:
> _remove_opp_table(opp_table);
> unlock:
> @@ -1364,10 +1453,10 @@ struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name)
>
> return opp_table;
> }
> -EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
> +EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulators);
>
> /**
> - * dev_pm_opp_put_regulator() - Releases resources blocked for regulator
> + * dev_pm_opp_put_regulators() - Releases resources blocked for regulators
> * @opp_table: opp_table returned from dev_pm_opp_set_regulator
This needs an update too ^
> *
> * Locking: The internal opp_table and opp structures are RCU protected.
> diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
> index c897676ca35f..95f433db4ac7 100644
> --- a/drivers/base/power/opp/debugfs.c
> +++ b/drivers/base/power/opp/debugfs.c
> @@ -34,6 +35,46 @@ void opp_debug_remove_one(struct dev_pm_opp *opp)
> debugfs_remove_recursive(opp->dentry);
> }
>
> +static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
> + struct opp_table *opp_table,
> + struct dentry *pdentry)
> +{
> + struct dentry *d;
> + int i = 0;
> + char *name;
> +
> + /* Always create at least supply-0 directory */
> + do {
> + name = kasprintf(GFP_KERNEL, "supply-%d", i);
Sorry I haven't noticed this before. Wouldn't we want to put the
supply name here instead of a number?
> +
> + /* Create per-opp directory */
> + d = debugfs_create_dir(name, pdentry);
> +
> + kfree(name);
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 4d3ec92cbabf..1cd1fcfe835a 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -188,7 +188,10 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> */
> name = find_supply_name(cpu_dev);
> if (name) {
> - opp_table = dev_pm_opp_set_regulator(cpu_dev, name);
> + const char *names[] = {name};
> +
> + opp_table = dev_pm_opp_set_regulators(cpu_dev, names,
> + ARRAY_SIZE(names));
This can be simplified to:
opp_table = dev_pm_opp_set_regulators(cpu_dev, &name, 1);
> if (IS_ERR(opp_table)) {
> ret = PTR_ERR(opp_table);
> dev_err(cpu_dev, "Failed to set regulator for cpu%d: %d\n",
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Martin Blumenstingl @ 2016-11-30 22:33 UTC (permalink / raw)
To: Rob Herring
Cc: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
linux-arm-kernel, alexandre.torgue, peppe.cavallaro, will.deacon,
catalin.marinas, carlo, f.fainelli
In-Reply-To: <20161130214429.albylffpoumbeugi@rob-hp-laptop>
On Wed, Nov 30, 2016 at 10:44 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Nov 25, 2016 at 02:01:50PM +0100, Martin Blumenstingl wrote:
>> This allows configuring the RGMII TX clock delay. The RGMII clock is
>> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
>> The configuration depends on the actual hardware (no delay may be
>> needed due to the design of the actual circuit, the PHY might add this
>> delay, etc.).
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>> Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> index 89e62dd..f8bc540 100644
>> --- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> +++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
>> @@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
>> - "clkin0" - first parent clock of the internal mux
>> - "clkin1" - second parent clock of the internal mux
>>
>> +Optional properties on Meson8b and newer:
>> +- amlogic,tx-delay-ns: The internal RGMII TX clock delay (provided
>> + by this driver) in nanoseconds. Allowed values
>> + are: 0ns, 2ns, 4ns, 6ns.
>> + This must be configured when the phy-mode is
>> + "rgmii" (typically a value of 2ns is used in
>> + this case).
>> + When phy-mode is set to "rgmii-id" or
>> + "rgmii-txid" the TX clock delay is already
>> + provided by the PHY. In that case this
>> + property should be set to 0ns (which disables
>> + the TX clock delay in the MAC to prevent the
>> + clock from going off because both PHY and MAC
>> + are adding a delay).
>
> What's the default? Why can't no property mean 0ns delay?
This value (2ns) was previously hardcoded. Having a default of 0ns
means that patch 7 ("ARM64: dts: amlogic: add the ethernet TX delay
configuration") becomes mandatory (otherwise we'll have broken Gbit
ethernet again because the TX delay is now disabled in the PHY when
phy-mode is "rgmii" and additionally we don't apply the 2ns TX delay
on the MAC side when the property is missing).
I'm fine with either way though - just let me know so I can adjust the
code accordingly.
^ permalink raw reply
* Re: [PATCH v2 8/9] arm64: dts: rockchip: support dwc3 USB for rk3399
From: Brian Norris @ 2016-11-30 22:28 UTC (permalink / raw)
To: Caesar Wang
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
Grygorii Strashko, Catalin Marinas, Heiko Stuebner, Arnd Bergmann,
Felipe Balbi, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Shawn Lin,
zhangqing, Will Deacon, Douglas Anderson,
tfiga-F7+t8E8rja9g9hUCZPvPmw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
eddie.cai-TNX95d0MmH7DzftRWevZcw, David Wu, Sriram Dash,
Jianqun Xu, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478697721-2323-9-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ Felipe, Arnd, others
Hi Caesar,
On Wed, Nov 09, 2016 at 09:22:00PM +0800, Caesar Wang wrote:
> From: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> Add the dwc3 usb needed node information for rk3399.
>
> Signed-off-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Note that none of this can work yet (at least for host mode, which I've
been testing), because DWC3 still hasn't been patched for ARM64 support.
It's been 7+ months and multiple people have tried to patch the issue,
but nothing has been merged.
See for instance:
usb: dwc3: host: inherit dma configuration from parent dev
https://lkml.org/lkml/2016/4/25/813
https://lkml.org/lkml/2016/5/5/391
Thread was resurrected in September:
https://lkml.org/lkml/2016/9/1/715
but there's still no end in sight. Maybe the following is the latest
incarnation?
[PATCH v2 0/6] inherit dma configuration from parent dev
https://www.mail-archive.com/linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg82369.html
I guess nothing prevents a valid device tree being merged here, but it's
severely limited by the above bug still, so I just wanted to call
attention to it.
(Let me guess: you've still been testing an internal non-upstream tree
that has this patched already, Caesar?)
> ---
>
> Changes in v2:
> - the original patches from brian posting on
> https://chromium-review.googlesource.com/343603
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 54 ++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 09ebf4e..3659c56 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -353,6 +353,60 @@
> status = "disabled";
> };
>
> + usbdrd3_0: usb@fe800000 {
> + compatible = "rockchip,rk3399-dwc3";
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> + <&cru ACLK_USB3OTG0>, <&cru ACLK_USB3_GRF>;
> + clock-names = "ref_clk", "suspend_clk",
> + "bus_clk", "grf_clk";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + status = "disabled";
> + usbdrd_dwc3_0: dwc3@fe800000 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xfe800000 0x0 0x100000>;
> + interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH 0>;
> + dr_mode = "otg";
> + phys = <&tcphy0_usb3>;
> + phy-names = "usb3-phy";
The USB3/TypeC PHY won't probe without extcon support, and no rk3399
platforms have proper extcon support upstream AFAIK, right? Seems like
it'd be better to leave this phy property off for now, with the hope of
at least getting USB2 working. We can add it later once somebody proves
USB3 support upstream.
> + phy_type = "utmi_wide";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u2-freeclk-exists-quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis-del-phy-power-chg-quirk;
> + snps,xhci-slow-suspend-quirk;
This property isn't supported upstream. Seems like we should drop it for
now.
Same comments on the other DWC3 instance below, of course.
Brian
> + status = "disabled";
> + };
> + };
> +
> + usbdrd3_1: usb@fe900000 {
> + compatible = "rockchip,rk3399-dwc3";
> + clocks = <&cru SCLK_USB3OTG1_REF>, <&cru SCLK_USB3OTG1_SUSPEND>,
> + <&cru ACLK_USB3OTG1>, <&cru ACLK_USB3_GRF>;
> + clock-names = "ref_clk", "suspend_clk",
> + "bus_clk", "grf_clk";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + status = "disabled";
> + usbdrd_dwc3_1: dwc3@fe900000 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xfe900000 0x0 0x100000>;
> + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH 0>;
> + dr_mode = "host";
> + phys = <&tcphy1_usb3>;
> + phy-names = "usb3-phy";
> + phy_type = "utmi_wide";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u2-freeclk-exists-quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis-del-phy-power-chg-quirk;
> + snps,xhci-slow-suspend-quirk;
> + status = "disabled";
> + };
> + };
> +
> gic: interrupt-controller@fee00000 {
> compatible = "arm,gic-v3";
> #interrupt-cells = <4>;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v3 0/2] Support for Axentia TSE-850
From: Arnd Bergmann @ 2016-11-30 22:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring,
Alexandre Belloni, Peter Rosin
In-Reply-To: <1480510102-24587-1-git-send-email-peda@axentia.se>
On Wednesday, November 30, 2016 1:48:20 PM CET Peter Rosin wrote:
> Hi!
>
> changes v2 -> v3
> - document the new compatible strings prefixed with "axentia,".
>
> changes v1 -> v2
> - squash the fixup into the correct patch, sorry for the noise.
>
> After finally having all essintial drivers upstreamed (the
> last ones are currently in -next) I would like to have the
> dts and the defconfig also upstreamed.
>
> Cheers,
> Peter
>
> Peter Rosin (2):
> ARM: dts: add devicetree for the Axentia TSE-850
> ARM: tse850_defconfig: add Axentia TSE-850
>
> Documentation/devicetree/bindings/arm/axentia.txt | 19 ++
> MAINTAINERS | 8 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/axentia-linea.dtsi | 53 +++++
> arch/arm/boot/dts/axentia-tse850-3.dts | 276 ++++++++++++++++++++++
> arch/arm/configs/tse850_defconfig | 223 +++++++++++++++++
> 6 files changed, 580 insertions(+)
Hi Peter,
I'm a bit confused. Are these just boards using the sama5d31 SoC,
or something else?
Normally, dts files are picked up by the SoC platform maintainers
and named with a prefix for that soc.
Also, we don't normally add a defconfig file for a specific
machine, just add the options you want to sama5_defconfig
and multi_v7_defconfig, and send all patches to the
at91 maitainers.
Arnd
^ permalink raw reply
* Re: [PATCH 0/5] Meson GXL and GXM USB support
From: Rob Herring @ 2016-11-30 22:22 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, narmstrong-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <CAFBinCAA_JEtr_0Ze0thoRaEKMnWQMKcPxJ8y88zkWAAhuxsMw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sun, Nov 27, 2016 at 11:42:02PM +0100, Martin Blumenstingl wrote:
> Hello Kishon,
>
> On Sat, Nov 26, 2016 at 3:56 PM, Martin Blumenstingl
> <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > USB support on GXL and GXM differs a lot from Meson8b and GXBB:
> > The most obvious change is that GXL and GXM now have one dwc3
> > controller and one dwc2 controller (instead of two dwc2 controllers).
> > With that there are also new USB PHYs.
> >
> > Due to lack of hardware I was only able to test this on a board with
> > GXM, but as far as I understand the hardware my preparations should be
> > correct (so it should also work on GXL).
> >
> > dwc2 will probably stay unused on most GXM devices since it's limited
> > to device mode via some dwc2 hardware configuration register.
> >
> > dwc3 is probably used on all devices, even if there is more than just
> > one USB port. dwc3 has a built-in USB2 hub - on GXL this hub has two
> > ports enabled, while on GXM there are three ports enabled (see below
This hub is an actual USB hub? If so, then you should probably model the
USB bus topology (which we have a binding definition for).
> > for lsusb output). There are no USB3 ports enabled in the dwc3 hardware
> > configuration, meaning that the SoC is limited to high-speed mode.
> > On my GXM device the dwc3 hardware configuration forces it into "host
> > only" mode.
> >
> > The SoCs contain two PHY blocks: one USB3 PHY and up to four USB2 PHYs
> > (on GXM there are only three enabled, but the registers should support
> > up to four).
> > The USB3 PHY also handles the OTG interrupts, but since dwc3's hardware
> > configuration enforces "host only" mode I was not able to test this. It
> > simply takes care of an interrupt and then notifies all related PHYs
> > about the new mode.
> > The USB2 PHY block is a bit different: I created one PHY driver which
> > spans all "PHY ports" because the handling is a bit tricky. It turns
> > out that for each available USB port in dwc3's hub the corresponding
> > PHY must be enabled (even if there is no physical port - in my case
> > port 3 is not connected to anything, but disabling the PHY breaks
> > ports 1 and 2 as well).
> > I decided not not pass the USB2 PHYs directly to dwc3 due to three
> > reasons: 1. the USB3 PHY (which holds a reference to all relevant
> > USB2 PHY ports) controls the mode of the USB2 PHY ports (since both
> > are used with the same controller and thus it makes sense to keep the
> > mode consistent across all ports) 2. the dwc3 driver does not support
> > passing multiple USB2 PHYs (only one USB2 and one USB3 PHY can be
> > passed to it) 3. it is similar to how the vendor reference driver
> > manages the PHYs. Please note that this coupling is not a fixed, this
> > is all configurable via devicetree (so if the third USB2 PHY has to
> > be passed two the dwc2 controller then this is still possible by
> > just moving on PHY reference in the .dts).
> after not staring at my own code for 24 hours I realized this:
> (I went through quite a few iterations before getting these drivers to work)
> I'm basically re-modelling an "USB PHY hub" with my USB3 PHY driver
> (there's one "upstream" PHY interface which is passed to dwc3 and
> multiple downstream PHYs, each for one port on dwc3's internal hub).
> With this approach I could split each of the the USB2s into separate
> nodes again (instead of one devicetree node with #phy-cells = <1>) as
> the USB3 PHY is taking care of that special "we have to enable all
> ports or no port will be usable".
>
> We could go even one step further: why implement this in the Meson GXL
> specific PHY driver - why not implement a generic "phy-hub" driver
> (which would be valid whenever the PHY controller has to manage
> multiple PHYs at once, but wants to keep them all in a consistent
> state).
> The devicetree could look like this:
> usb2_phy_hub: phy@0 {
> compatible = "phy-hub";
> phys = <&other_phy1>, <&other_phy 2>;
> };
>
> &dwc3 {
> phys = <&usb2_phy_hub>, <&usb3_phy0>;
> phy-names = "usb2-phy", "usb3-phy";
> };
I'm okay with a hub if it is modeled as a USB hub. Here though, it
looks like you are just trying to group things which doesn't need to be
in DT.
>
> The generic phy-hub driver would then implement all phy_ops callbacks
> and pass then to each of it's downstream PHYs.
You can have generic drivers without a generic binding.
> That's just what came into my head - please let me know what you think
> of this or share your ideas on how to approach this!
>
> > The coupling of the USB2 and USB3 PHYs is the reason why I sent the
> > two drivers in one patch, even though they are handling different IP
> > blocks (different registers, etc.).
> >
> > Unfortunately there are no datasheets available for any of these PHYs.
> > Both drivers were written by reading the reference drivers provided by
> > Amlogic and analyzing the registers on the kernel that was shipped with
> > my board.
> >
> > As a last note: the dwc3 driver currently only explicitly enables the
> > first USB port "DWC3_GUSB2PHYCFG(0)" in the internal hub. The hardware
> > seems to enable the other two (DWC3_GUSB2PHYCFG(1) and
> > DWC3_GUSB2PHYCFG(2)) automatically. I will ask the dwc3 maintainers if
> > changes to dwc3 are desired any how these should look like, but for now
> > it's working fine even without changes there.
> >
> > lsusb output on GXM for the dwc3 hub:
> > Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> > ...
> > Hub Port Status:
> > Port 1: 0000.0100 power
> > Port 2: 0000.0100 power
> > Port 3: 0000.0100 power
> >
> > NOTE: The devicetree changes depend on my previous series:
> > "[PATCH 0/2] minor GXL and GXM improvements" - see [0]
> >
> > NOTE2: This series depends on an upstream dwc3/xhci-plat DMA fix
> > (special thanks to Arnd Bergmann and Sriram Dash for fixing that):
> > "[PATCH v5 0/6] inherit dma configuration from parent dev" - see [1]
> >
> > I have a tree with all dependencies applied available at [2] if
> > someone wants a quick way to test this (I don't take any responsibility
> > if anything explodes though).
> >
> > [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001665.html
> > [1] http://marc.info/?l=linux-usb&m=147938307209685&w=2
> > [2] https://github.com/xdarklight/linux/commits/meson-gx-integration-4.10-20161126
> >
> > Martin Blumenstingl (5):
> > Documentation: dt-bindings: Add documentation for Meson GXL USB2/3
> > PHYs
> > phy: meson: add USB2 and USB3 PHY support for Meson GXL
> > arm64: dts: meson-gxl: add USB support
> > ARM64: dts: meson-gxm: add GXM specific USB configuration
> > ARM64: dts: meson-gx-p23x-q20x: enable USB on P23x and Q20x boards
> >
> > .../devicetree/bindings/phy/meson-gxl-usb2-phy.txt | 25 ++
> > .../devicetree/bindings/phy/meson-gxl-usb3-phy.txt | 27 ++
> > .../arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 12 +
> > arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 49 +++
> > .../arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 17 +
> > arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 10 +
> > drivers/phy/Kconfig | 13 +
> > drivers/phy/Makefile | 2 +
> > drivers/phy/phy-meson-gxl-usb2.c | 374 ++++++++++++++++++++
> > drivers/phy/phy-meson-gxl-usb3.c | 377 +++++++++++++++++++++
> > 10 files changed, 906 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/meson-gxl-usb2-phy.txt
> > create mode 100644 Documentation/devicetree/bindings/phy/meson-gxl-usb3-phy.txt
> > create mode 100644 drivers/phy/phy-meson-gxl-usb2.c
> > create mode 100644 drivers/phy/phy-meson-gxl-usb3.c
> >
> > --
> > 2.10.2
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/6] net: ethernet: ti: cpts: add ptp pps support
From: Richard Cochran @ 2016-11-30 22:17 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Murali Karicheri, Wingman Kwok, David S. Miller, netdev,
Mugunthan V N, Sekhar Nori, linux-kernel, linux-omap, Rob Herring,
devicetree
In-Reply-To: <875d4cc2-8a47-b06d-fb46-0cacc28dbaee@ti.com>
On Wed, Nov 30, 2016 at 02:43:57PM -0600, Grygorii Strashko wrote:
> > In order to produce the PPS edge correctly, you would have to adjust
> > the comparison value whenever cc.mult changes,
>
> yes. And that is done in cpts_ptp_adjfreq()
> if (cpts->ts_comp_enabled)
> cpts->ts_comp_one_sec_cycs = cpts_cc_ns2cyc(cpts, NSEC_PER_SEC);
> ^^^ re-calculate reload value for
>
> cpts_ts_comp_settime(cpts, ns);
> ^^^ adjust the ts_comp
And it races with the pulse itself. You forgot about this part:
> @@ -172,14 +232,31 @@ static int cpts_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> adj *= ppb;
> diff = div_u64(adj, 1000000000ULL);
>
> + mutex_lock(&cpts->ptp_clk_mutex);
> +
> spin_lock_irqsave(&cpts->lock, flags);
> + if (cpts->ts_comp_enabled) {
> + cpts_ts_comp_disable(cpts);
Sorry, but this is a train wreck.
> > but of course this is unworkable.
> >
>
> Sry, but this is questionable - code for pps comes from TI internal
> branches (SDK releases) where it survived for a pretty long time.
That doesn't mean the code is any good. If you adjust at the right
moment, then no pulse occurs at all!
> I'm, of course, agree that without HW support for freq adjustment
> this PPS feature is not super precise and has some limitation,
> but that is what we agree to live with.
I do NOT agree to live with this. I am one who is going to have to
explain to the world why their beagle bone PPS sucks.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH V6 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Stephen Boyd @ 2016-11-30 22:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Viresh Kumar, Nishanth Menon, linaro-kernel,
linux-pm, Vincent Guittot, robh, d-gerlach, broonie, devicetree
In-Reply-To: <152e4a2c876449f3e47c206f5120cafdfd48b976.1480481312.git.viresh.kumar@linaro.org>
On 11/30, Viresh Kumar wrote:
> The generic set_opp() handler isn't sufficient for platforms with
> complex DVFS. For example, some TI platforms have multiple regulators
> for a CPU device. The order in which various supplies need to be
> programmed is only known to the platform code and its best to leave it
> to it.
>
> This patch implements APIs to register platform specific set_opp()
> callback.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> Tested-by: Dave Gerlach <d-gerlach@ti.com>
>
> ---
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
So this one has the same set/put problem the other APIs has?
Presumably we're going to need to fix and change the API that is
introduced here. Wouldn't it be better to do that first though?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 01/10] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver
From: Rob Herring @ 2016-11-30 22:03 UTC (permalink / raw)
To: Todor Tomov
Cc: mchehab, laurent.pinchart+renesas, hans.verkuil, javier,
s.nawrocki, linux-media, linux-kernel, mark.rutland, devicetree,
bjorn.andersson, srinivas.kandagatla
In-Reply-To: <1480085813-28235-1-git-send-email-todor.tomov@linaro.org>
On Fri, Nov 25, 2016 at 04:56:53PM +0200, Todor Tomov wrote:
> Add DT binding document for Qualcomm Camera subsystem driver.
>
> Signed-off-by: Todor Tomov <todor.tomov@linaro.org>
> ---
> .../devicetree/bindings/media/qcom,camss.txt | 196 +++++++++++++++++++++
> 1 file changed, 196 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt b/Documentation/devicetree/bindings/media/qcom,camss.txt
> new file mode 100644
> index 0000000..76ad89a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
> @@ -0,0 +1,196 @@
> +Qualcomm Camera Subsystem
> +
> +* Properties
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Should contain:
> + - "qcom,8x16-camss"
Don't use wildcards in compatible strings. One string per SoC.
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: Register ranges as listed in the reg-names property.
> +- reg-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Should contain the following entries:
> + - "csiphy0"
> + - "csiphy0_clk_mux"
> + - "csiphy1"
> + - "csiphy1_clk_mux"
> + - "csid0"
> + - "csid1"
> + - "ispif"
> + - "csi_clk_mux"
> + - "vfe0"
Kind of looks like the phy's should be separate nodes since each phy has
its own register range, irq, clocks, etc.
> +- interrupts:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: Interrupts as listed in the interrupt-names property.
> +- interrupt-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Should contain the following entries:
> + - "csiphy0"
> + - "csiphy1"
> + - "csid0"
> + - "csid1"
> + - "ispif"
> + - "vfe0"
> +- power-domains:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: A phandle and power domain specifier pairs to the
> + power domain which is responsible for collapsing
> + and restoring power to the peripheral.
> +- clocks:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: A list of phandle and clock specifier pairs as listed
> + in clock-names property.
> +- clock-names:
> + Usage: required
> + Value type: <stringlist>
> + Definition: Should contain the following entries:
> + - "camss_top_ahb_clk"
> + - "ispif_ahb_clk"
> + - "csiphy0_timer_clk"
> + - "csiphy1_timer_clk"
> + - "csi0_ahb_clk"
> + - "csi0_clk"
> + - "csi0_phy_clk"
> + - "csi0_pix_clk"
> + - "csi0_rdi_clk"
> + - "csi1_ahb_clk"
> + - "csi1_clk"
> + - "csi1_phy_clk"
> + - "csi1_pix_clk"
> + - "csi1_rdi_clk"
> + - "camss_ahb_clk"
> + - "camss_vfe_vfe_clk"
> + - "camss_csi_vfe_clk"
> + - "iface_clk"
> + - "bus_clk"
> +- vdda-supply:
> + Usage: required
> + Value type: <phandle>
> + Definition: A phandle to voltage supply for CSI2.
> +- iommus:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: A list of phandle and IOMMU specifier pairs.
> +
> +* Nodes
> +
> +- ports:
> + Usage: required
> + Definition: As described in video-interfaces.txt in same directory.
> + Properties:
> + - reg:
> + Usage: required
> + Value type: <u32>
> + Definition: Selects CSI2 PHY interface - PHY0 or PHY1.
> + Endpoint node properties:
> + - clock-lanes:
> + Usage: required
> + Value type: <u32>
> + Definition: The clock lane.
> + - data-lanes:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: An array of data lanes.
> + - qcom,settle-cnt:
This should go in phy node ideally.
> + Usage: required
> + Value type: <u32>
> + Definition: The settle count parameter for CSI PHY.
> +
> +* An Example
> +
> + camss: camss@1b00000 {
> + compatible = "qcom,8x16-camss";
> + reg = <0x1b0ac00 0x200>,
> + <0x1b00030 0x4>,
> + <0x1b0b000 0x200>,
> + <0x1b00038 0x4>,
> + <0x1b08000 0x100>,
> + <0x1b08400 0x100>,
> + <0x1b0a000 0x500>,
> + <0x1b00020 0x10>,
> + <0x1b10000 0x1000>;
> + reg-names = "csiphy0",
> + "csiphy0_clk_mux",
> + "csiphy1",
> + "csiphy1_clk_mux",
> + "csid0",
> + "csid1",
> + "ispif",
> + "csi_clk_mux",
> + "vfe0";
> + interrupts = <GIC_SPI 78 0>,
> + <GIC_SPI 79 0>,
> + <GIC_SPI 51 0>,
> + <GIC_SPI 52 0>,
> + <GIC_SPI 55 0>,
> + <GIC_SPI 57 0>;
> + interrupt-names = "csiphy0",
> + "csiphy1",
> + "csid0",
> + "csid1",
> + "ispif",
> + "vfe0";
> + power-domains = <&gcc VFE_GDSC>;
> + clocks = <&gcc GCC_CAMSS_TOP_AHB_CLK>,
> + <&gcc GCC_CAMSS_ISPIF_AHB_CLK>,
> + <&gcc GCC_CAMSS_CSI0PHYTIMER_CLK>,
> + <&gcc GCC_CAMSS_CSI1PHYTIMER_CLK>,
> + <&gcc GCC_CAMSS_CSI0_AHB_CLK>,
> + <&gcc GCC_CAMSS_CSI0_CLK>,
> + <&gcc GCC_CAMSS_CSI0PHY_CLK>,
> + <&gcc GCC_CAMSS_CSI0PIX_CLK>,
> + <&gcc GCC_CAMSS_CSI0RDI_CLK>,
> + <&gcc GCC_CAMSS_CSI1_AHB_CLK>,
> + <&gcc GCC_CAMSS_CSI1_CLK>,
> + <&gcc GCC_CAMSS_CSI1PHY_CLK>,
> + <&gcc GCC_CAMSS_CSI1PIX_CLK>,
> + <&gcc GCC_CAMSS_CSI1RDI_CLK>,
> + <&gcc GCC_CAMSS_AHB_CLK>,
> + <&gcc GCC_CAMSS_VFE0_CLK>,
> + <&gcc GCC_CAMSS_CSI_VFE0_CLK>,
> + <&gcc GCC_CAMSS_VFE_AHB_CLK>,
> + <&gcc GCC_CAMSS_VFE_AXI_CLK>;
> + clock-names = "camss_top_ahb_clk",
> + "ispif_ahb_clk",
> + "csiphy0_timer_clk",
> + "csiphy1_timer_clk",
> + "csi0_ahb_clk",
> + "csi0_clk",
> + "csi0_phy_clk",
> + "csi0_pix_clk",
> + "csi0_rdi_clk",
> + "csi1_ahb_clk",
> + "csi1_clk",
> + "csi1_phy_clk",
> + "csi1_pix_clk",
> + "csi1_rdi_clk",
> + "camss_ahb_clk",
> + "camss_vfe_vfe_clk",
> + "camss_csi_vfe_clk",
> + "iface_clk",
> + "bus_clk";
> + vdda-supply = <&pm8916_l2>;
> + iommus = <&apps_iommu 3>;
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + csiphy0_ep: endpoint {
> + clock-lanes = <1>;
> + data-lanes = <0 2>;
> + qcom,settle-cnt = <0xe>;
> + remote-endpoint = <&ov5645_ep>;
> + };
> + };
> + };
> + };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: usb: add DT binding for s3c2410 USB OHCI controller
From: Rob Herring @ 2016-11-30 21:53 UTC (permalink / raw)
To: Sergio Prado
Cc: gregkh, mark.rutland, stern, kgene, krzk, javier, linux-usb,
devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <1480085249-25014-2-git-send-email-sergio.prado@e-labworks.com>
On Fri, Nov 25, 2016 at 12:47:28PM -0200, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB OHCI controller.
>
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
> .../devicetree/bindings/usb/s3c2410-usb.txt | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/s3c2410-usb.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation
From: Sakari Ailus @ 2016-11-30 21:48 UTC (permalink / raw)
To: Rob Herring
Cc: Kevin Hilman, linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Hans Verkuil, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
Alexandre Bailon, David Lechner, g.liakhovetski-Mmb7MZpHnFY,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <CAL_JsqJ3wJnNa=bVN+UT4A-J5XC0jdyGAgWzROScRDLy6T8xHw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob and Kevin,
On Tue, Nov 29, 2016 at 08:41:44AM -0600, Rob Herring wrote:
> On Mon, Nov 28, 2016 at 4:30 PM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> > Hi Rob,
> >
> > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> >
> >> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
> >>> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >>> ---
> >>> .../bindings/media/ti,da850-vpif-capture.txt | 65 ++++++++++++++++++++++
> >>> .../devicetree/bindings/media/ti,da850-vpif.txt | 8 +++
> >>> 2 files changed, 73 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >>> new file mode 100644
> >>> index 000000000000..c447ac482c1d
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >>> @@ -0,0 +1,65 @@
> >>> +Texas Instruments VPIF Capture
> >>> +------------------------------
> >>> +
> >>> +The TI Video Port InterFace (VPIF) capture component is the primary
> >>> +component for video capture on the DA850 family of TI DaVinci SoCs.
> >>> +
> >>> +TI Document number reference: SPRUH82C
> >>> +
> >>> +Required properties:
> >>> +- compatible: must be "ti,da850-vpif-capture"
> >>> +- reg: physical base address and length of the registers set for the device;
> >>> +- interrupts: should contain IRQ line for the VPIF
> >>> +
> >>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
> >>> +channels or a single 16-bit channel. It should contain at least one
> >>> +port child node with child 'endpoint' node. Please refer to the
> >>> +bindings defined in
> >>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> >>> +
> >>> +Example using 2 8-bit input channels, one of which is connected to an
> >>> +I2C-connected TVP5147 decoder:
> >>> +
> >>> + vpif_capture: video-capture@0x00217000 {
> >>> + reg = <0x00217000 0x1000>;
> >>> + interrupts = <92>;
> >>> +
> >>> + port {
> >>> + vpif_ch0: endpoint@0 {
> >>> + reg = <0>;
> >>> + bus-width = <8>;
> >>> + remote-endpoint = <&composite>;
> >>> + };
> >>> +
> >>> + vpif_ch1: endpoint@1 {
> >>
> >> I think probably channels here should be ports rather than endpoints.
> >> AIUI, having multiple endpoints is for cases like a mux or 1 to many
> >> connections. There's only one data flow, but multiple sources or sinks.
> >
> > Looking at this closer... , I used an endpoint because it's bascially a
> > 16-bit parallel bus, that can be configured as (up to) 2 8-bit
> > "channels. So, based on the video-interfaces.txt doc, I configured this
> > as a single port, with (up to) 2 endpoints. That also allows me to
> > connect output of the decoder directly, using the remote-endpoint
> > property.
> >
> > So I guess I'm not fully understanding your suggestion.
>
> NM, looks like video-interfaces.txt actually spells out this case and
> defines doing it as you did.
It's actually the first time I read that portion (at least so that I could
remember) of video-interfaces.txt. I'm not sure if anyone has implemented
that previously, nor how we ended up with the text. The list archive could
probably tell. Cc Guennadi who wrote it. :-) I couldn't immediately find DT
source with this arrangement.
In case of splitting the port into two parallel interfaces, how do you
determine which wires belong to which endpoint? I guess they'd be particular
sets of wires but as there's just a single port it isn't defined by the
port.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/7] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Rob Herring @ 2016-11-30 21:44 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20161125130156.17879-2-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 25, 2016 at 02:01:50PM +0100, Martin Blumenstingl wrote:
> This allows configuring the RGMII TX clock delay. The RGMII clock is
> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
> The configuration depends on the actual hardware (no delay may be
> needed due to the design of the actual circuit, the PHY might add this
> delay, etc.).
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/meson-dwmac.txt b/Documentation/devicetree/bindings/net/meson-dwmac.txt
> index 89e62dd..f8bc540 100644
> --- a/Documentation/devicetree/bindings/net/meson-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/meson-dwmac.txt
> @@ -25,6 +25,20 @@ Required properties on Meson8b and newer:
> - "clkin0" - first parent clock of the internal mux
> - "clkin1" - second parent clock of the internal mux
>
> +Optional properties on Meson8b and newer:
> +- amlogic,tx-delay-ns: The internal RGMII TX clock delay (provided
> + by this driver) in nanoseconds. Allowed values
> + are: 0ns, 2ns, 4ns, 6ns.
> + This must be configured when the phy-mode is
> + "rgmii" (typically a value of 2ns is used in
> + this case).
> + When phy-mode is set to "rgmii-id" or
> + "rgmii-txid" the TX clock delay is already
> + provided by the PHY. In that case this
> + property should be set to 0ns (which disables
> + the TX clock delay in the MAC to prevent the
> + clock from going off because both PHY and MAC
> + are adding a delay).
What's the default? Why can't no property mean 0ns delay?
>
> Example for Meson6:
>
> --
> 2.10.2
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] dt-bindings: document how to setup rockchip timers as clocksource
From: Alexander Kochetkov @ 2016-11-30 21:40 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, wxt, daniel.lezcano, huangtao, devicetree,
linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <20161130213006.p6totahjga7546s6@rob-hp-laptop>
> 1 дек. 2016 г., в 0:30, Rob Herring <robh@kernel.org> написал(а):
>
> 1st and 2nd are ambiguous. Plus this is an OS implementation detail that
> doesn't belong in the binding.
>
>> +If you want to bind specific timer as clockevent (i.e. one from alive subsystem)
>> +and specific timer as clocksource, you can number the timers in "aliases" node.
>
> No.
>
> Use and/or describe what are the features of a timer to make the
> decision. There has to be some reason you care which one. One has an
> interrupt and the other doesn't. One is always on. Etc.
Thank you, Rob.
Eventually I abandoned this decision.
I left only one patch, which you confirmed recently.
And sorry for making noise with duplicate patches.
Alexander.
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: Add DT bindings info for FlexRM mailbox driver
From: Rob Herring @ 2016-11-30 21:38 UTC (permalink / raw)
To: Anup Patel
Cc: Jassi Brar, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
Rob Rice, devicetree, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1480048551-3285-3-git-send-email-anup.patel@broadcom.com>
On Fri, Nov 25, 2016 at 10:05:51AM +0530, Anup Patel wrote:
> This patch adds device tree bindings document for the FlexRM
> mailbox driver.
Bindings document h/w, not drivers.
>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> ---
> .../bindings/mailbox/brcm,flexrm-mbox.txt | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>
> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
> new file mode 100644
> index 0000000..7969b1c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
> @@ -0,0 +1,60 @@
> +Broadcom FlexRM Mailbox Driver
h/w
> +===============================
> +The Broadcom FlexRM ring manager provides a set of rings which can be
> +used to submit work to offload engines. An SoC may have multiple FlexRM
> +hardware blocks. There is one device tree entry per block. The FlexRM
> +mailbox drivers creates a mailbox instance using FlexRM rings where
> +each mailbox channel is a separate FlexRM ring.
> +
> +Required properties:
> +--------------------
> +- compatible: Should be "brcm,flexrm-mbox"
Sounds generic. Add SoC specific compatible strings please.
> +- reg: Specifies base physical address and size of the FlexRM
> + ring registers
> +- msi-parent: Phandles (and potential Device IDs) to MSI controllers
> + The FlexRM engine will send MSIs (instead of wired
> + interrupts) to CPU. There is one MSI for each FlexRM ring.
One ring is one h/w block, right? How many instances is not really
relevant.
> + Refer devicetree/bindings/interrupt-controller/msi.txt
> +- #mbox-cells: Specifies the number of cells needed to encode a mailbox
> + channel. This should be 3.
> +
> + The 1st cell is the mailbox channel number.
> +
> + The 2nd cell contains MSI completion threshold. This is the
> + number of completion messages for which FlexRM will inject
> + one MSI interrupt to CPU.
> +
> + The 3nd cell contains MSI timer value representing time for
> + which FlexRM will wait to accumulate N completion messages
> + where N is the value specified by 2nd cell above. If FlexRM
> + does not get required number of completion messages in time
> + specified by this cell then it will inject one MSI interrupt
> + to CPU provided atleast one completion message is available.
> +
> +Optional properties:
> +--------------------
> +- dma-coherent: Present if DMA operations made by the FlexRM engine (such
> + as DMA descriptor access, access to buffers pointed by DMA
> + descriptors and read/write pointer updates to DDR) are
> + cache coherent with the CPU.
> +
> +Example:
> +--------
> +crypto_mbox: mbox@67000000 {
> + compatible = "brcm,flexrm-mbox";
> + reg = <0x67000000 0x200000>;
> + msi-parent = <&gic_its 0x7f00>;
> + #mbox-cells = <3>;
> +};
> +
> +crypto_client {
Is this a h/w block or purely a driver on top of the mailbox? The latter
doesn't belong in DT.
> + ...
> + mboxes = <&crypto_mbox 0 0x1 0xffff>,
> + <&crypto_mbox 1 0x1 0xffff>,
> + <&crypto_mbox 16 0x1 0xffff>,
> + <&crypto_mbox 17 0x1 0xffff>,
> + <&crypto_mbox 30 0x1 0xffff>,
> + <&crypto_mbox 31 0x1 0xffff>;
> + };
> + ...
> +};
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: document how to setup rockchip timers as clocksource
From: Rob Herring @ 2016-11-30 21:30 UTC (permalink / raw)
To: Alexander Kochetkov
Cc: mark.rutland-5wv7dgnIgG8, wxt-TNX95d0MmH7DzftRWevZcw,
daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
huangtao-TNX95d0MmH7DzftRWevZcw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480025536-6837-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Nov 25, 2016 at 01:12:16AM +0300, Alexander Kochetkov wrote:
> The patch describes how to setup rockchip timers in device tree
> so they can be used as clocksource.
>
> I'm going to implement this feature.
>
> Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../bindings/timer/rockchip,rk-timer.txt | 35 +++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> index 7bc9691..15f8fed 100644
> --- a/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/rockchip,rk-timer.txt
> @@ -16,7 +16,18 @@ Required properties:
> - clock-names : must include the following entries:
> "timer", "pclk"
>
> -Example:
> +Note:
> +If device tree contain only one timer, than the timer will be intialized
> +as clockevent provider. If device tree contain two timers, than first timer
> +will be initialized as clockevent provider and second one as clocksource.
1st and 2nd are ambiguous. Plus this is an OS implementation detail that
doesn't belong in the binding.
> +If you want to bind specific timer as clockevent (i.e. one from alive subsystem)
> +and specific timer as clocksource, you can number the timers in "aliases" node.
No.
Use and/or describe what are the features of a timer to make the
decision. There has to be some reason you care which one. One has an
interrupt and the other doesn't. One is always on. Etc.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] dt-bindings: clarify compatible field usage for rockchip timers
From: Rob Herring @ 2016-11-30 21:25 UTC (permalink / raw)
To: Alexander Kochetkov
Cc: mark.rutland-5wv7dgnIgG8, wxt-TNX95d0MmH7DzftRWevZcw,
daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
huangtao-TNX95d0MmH7DzftRWevZcw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480025455-6797-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Nov 25, 2016 at 01:10:55AM +0300, Alexander Kochetkov wrote:
> rk3036 dtsi file already use compatible field as
> "rockchip,rk3036-timer", "rockchip,rk3288-timer".
>
> The patch clearly shows how that filed should be used on other chips.
>
> Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../bindings/timer/rockchip,rk-timer.txt | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/7] PWM: add pwm-stm32 DT bindings
From: Rob Herring @ 2016-11-30 21:20 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: mark.rutland, devicetree, lars, alexandre.torgue, linux-pwm,
linux-iio, linus.walleij, arnaud.pouliquen, linux-kernel,
thierry.reding, linux-arm-kernel, pmeerw, knaack.h, gerald.baeza,
fabrice.gasnier, lee.jones, linaro-kernel, jic23,
Benjamin Gaignard
In-Reply-To: <1480000463-9625-4-git-send-email-benjamin.gaignard@st.com>
On Thu, Nov 24, 2016 at 04:14:19PM +0100, Benjamin Gaignard wrote:
> Define bindings for pwm-stm32
>
> version 2:
> - use parameters instead of compatible of handle the hardware configuration
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> .../devicetree/bindings/pwm/pwm-stm32.txt | 37 ++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> new file mode 100644
> index 0000000..36263f0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> @@ -0,0 +1,37 @@
> +STMicroelectronics PWM driver bindings for STM32
> +
> +Must be a sub-node of STM32 general purpose timer driver
> +
> +Required parameters:
> +- compatible: Must be "st,stm32-pwm"
> +- pinctrl-names: Set to "default".
> +- pinctrl-0: List of phandles pointing to pin configuration nodes
> + for PWM module.
> + For Pinctrl properties, please refer to [1].
> +
> +Optional parameters:
> +- st,breakinput: Set if the hardware have break input capabilities
> +- st,breakinput-polarity: Set break input polarity. Default is 0
> + The value define the active polarity:
> + - 0 (active LOW)
> + - 1 (active HIGH)
> +- st,pwm-num-chan: Number of available PWM channels. Default is 0.
> +- st,32bits-counter: Set if the hardware have a 32 bits counter
> +- st,complementary: Set if the hardware have complementary output channels
What does complementary mean here?
> +
> +[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +
> +Example:
> + gptimer1: gptimer1@40010000 {
timer@...
> + compatible = "st,stm32-gptimer";
> + reg = <0x40010000 0x400>;
> + clocks = <&rcc 0 160>;
> + clock-names = "clk_int";
> +
> + pwm1@0 {
pwm {
Is there more than one?
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,breakinput;
> + st,complementary;
> + };
> + };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH] rtc: add support for EPSON TOYOCOM RTC-7301SF/DG
From: Alexandre Belloni @ 2016-11-30 21:01 UTC (permalink / raw)
To: Akinobu Mita
Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Alessandro Zummo
In-Reply-To: <1472396118-13424-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi,
Sorry for the very late review!
It seems mostly fine for me, two small comments:
On 28/08/2016 at 23:55:18 +0900, Akinobu Mita wrote :
> diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c
> new file mode 100644
> index 0000000..b1be281
> --- /dev/null
> +++ b/drivers/rtc/rtc-r7301.c
> @@ -0,0 +1,458 @@
> +/*
> + * EPSON TOYOCOM RTC-7301SF/DG Driver
> + *
> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * Based on rtc-rp5c01.c
> + *
> + * Datasheet: http://www5.epsondevice.com/en/products/parallel/rtc7301sf.html
> + */
> +
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/regmap.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +
> +#define DRV_NAME "rtc-r7301"
> +
> +enum {
> + RTC7301_1_SEC = 0x0, /* Bank 0 and Band 1 */
> + RTC7301_10_SEC = 0x1, /* Bank 0 and Band 1 */
> + RTC7301_AE = BIT(3),
> + RTC7301_1_MIN = 0x2, /* Bank 0 and Band 1 */
> + RTC7301_10_MIN = 0x3, /* Bank 0 and Band 1 */
> + RTC7301_1_HOUR = 0x4, /* Bank 0 and Band 1 */
> + RTC7301_10_HOUR = 0x5, /* Bank 0 and Band 1 */
> + RTC7301_DAY_OF_WEEK = 0x6, /* Bank 0 and Band 1 */
> + RTC7301_1_DAY = 0x7, /* Bank 0 and Band 1 */
> + RTC7301_10_DAY = 0x8, /* Bank 0 and Band 1 */
> + RTC7301_1_MONTH = 0x9, /* Bank 0 */
> + RTC7301_10_MONTH = 0xa, /* Bank 0 */
> + RTC7301_1_YEAR = 0xb, /* Bank 0 */
> + RTC7301_10_YEAR = 0xc, /* Bank 0 */
> + RTC7301_100_YEAR = 0xd, /* Bank 0 */
> + RTC7301_1000_YEAR = 0xe, /* Bank 0 */
> + RTC7301_ALARM_CONTROL = 0xe, /* Bank 1 */
> + RTC7301_ALARM_CONTROL_AIE = BIT(0),
> + RTC7301_ALARM_CONTROL_AF = BIT(1),
> + RTC7301_TIMER_CONTROL = 0xe, /* Bank 2 */
> + RTC7301_ALARM_CONTROL_TIE = BIT(0),
> + RTC7301_ALARM_CONTROL_TF = BIT(1),
> + RTC7301_CONTROL = 0xf, /* All banks */
> + RTC7301_CONTROL_BUSY = BIT(0),
> + RTC7301_CONTROL_STOP = BIT(1),
> + RTC7301_CONTROL_BANK_SEL_0 = BIT(2),
> + RTC7301_CONTROL_BANK_SEL_1 = BIT(3),
> +};
> +
Any particular reason why you use an enum instead of the usual #define?
[...]
> +static void rtc7301_init(struct rtc7301_priv *priv)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->lock, flags);
> +
> + rtc7301_select_bank(priv, 1);
> + rtc7301_alarm_irq(priv, false);
> +
If the RTC is battery backed, it may still run with the core power off
and maybe someone will actually expect the alarm to trigger at a later
time.
I don't mind much as you are probably the only user anyway.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v2 4/9] clk: stm32f4: Add lcd-tft clock
From: Rob Herring @ 2016-11-30 20:53 UTC (permalink / raw)
To: gabriel.fernandez
Cc: Mark Rutland, devicetree, daniel.thompson, radoslaw.pietrzyk,
Alexandre Torgue, Arnd Bergmann, Nicolas Pitre, andrea.merello,
Michael Turquette, olivier.bideau, Stephen Boyd, Russell King,
linux-kernel, ludovic.barre, Maxime Coquelin, amelie.delaunay,
linux-clk, linux-arm-kernel, kernel
In-Reply-To: <1479998749-20358-5-git-send-email-gabriel.fernandez@st.com>
On Thu, Nov 24, 2016 at 03:45:44PM +0100, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> This patch introduces lcd-tft clock for stm32f4 soc.
>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> .../devicetree/bindings/clock/st,stm32-rcc.txt | 1 +
> drivers/clk/clk-stm32f4.c | 118 +++++++++++++++++++++
> include/dt-bindings/clock/stm32f4-clock.h | 3 +-
> 3 files changed, 121 insertions(+), 1 deletion(-)
> diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h
> index 56b8e10..1be4a3a 100644
> --- a/include/dt-bindings/clock/stm32f4-clock.h
> +++ b/include/dt-bindings/clock/stm32f4-clock.h
> @@ -27,7 +27,8 @@
> #define CLK_RTC 5
> #define PLL_VCO_I2S 6
> #define PLL_VCO_SAI 7
> +#define CLK_LCD 8
>
> -#define END_PRIMARY_CLK 8
> +#define END_PRIMARY_CLK 9
Do you really need this? Having this change could cause compatibility
problems between dtb and kernel versions.
Please restructure the patch series and put all of the binding changes
including this header into a single patch. Incrementally add s/w
features, not h/w.
Rob
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox