* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Laurent Pinchart @ 2016-12-01 9:16 UTC (permalink / raw)
To: Sakari Ailus
Cc: Kevin Hilman, linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner
In-Reply-To: <20161201075730.GP16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hello,
On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
> > Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> >> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
> >>> 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)
> >>>>> 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...
That's really ugly :-( The problem should be fixed properly instead of adding
one more offender.
> >>>>> #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;
Can subdev_name be made const instead of blindly casting the full_name pointer
? If not this is probably unsafe, and if yes it should be done :-)
> >>>>> + 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 afraid I have to disappoint Hans here, I don't have code for that yet.
> >> 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". ;)
>
> :-)
>
> For the tvp514x, do you need more than a single endpoint on the receiver
> side? Does the input that's selected affect the bus parameters?
>
> If it doesn't, you could create a custom endpoint property for the possible
> input values. The s_routing() really should be fixed though, but that could
> be postponed I guess. There are quite a few drivers using it.
There's two ways to look at s_routing() in my opinion, as the calling driver
should really not hardcode any knowledge specific to a particular subdev. We
can either have the calling driver discover the possible routing options at
runtime through the subdev API, or modify the s_routing() API.
--
Regards,
Laurent Pinchart
--
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 v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Laurent Pinchart @ 2016-12-01 9:19 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201091313.th7nucjmvtuolqza@lukather>
Hi Maxime,
On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> >> More, it is not sure that the bridge/DW code would work with Allwinner's
> >> SoCs.
> >
> > If it doesn't work and can't be made to work in a non-invasive way they it
> > should certainly not be used :-)
>
> Even if the register layout is completely scrambled, as long as the
> bits themselves aren't (and by comparing the two drivers it looks like
> they haven't changed), you can easily deal with that using the
> regmap_fields, with the two implementations (the original one and the
> scrambled one) providing their register map that way, and the driver
> code using whatever has been provided.
Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it
seems that an address remapping function could be used.
> >> Eventually, I went the same way as omap/hdmi5: different driver.
> >
> > I might try to fix that for OMAP5 at some point, we'll see.
>
> For complex drivers that have already a driver written and a lot of
> testing that already happened, I don't think duplication is a smart
> move.
>
> >>> - And finally the fact that we can't have several display engine in
> >>> parallel, if needs be. This has happened in the past already on
> >>> Allwinner SoCs, so it's definitely something we should consider in
> >>> the DT bindings, since we can't break them.
> >>
> >> IIRC, I proposed my driver before yours, and the DE2 is completely
> >> different from the other display engines.
> >> What you are telling is "add more code to already complex code and have
> >> a big driver for all SoCs in each kernels".
> >> I think it should be better to have small modules, each one treating
> >> specific hardware, and to let only the needed code in the kernel memory
> >> at startup time.
> >>
> >>> Until those are fixed, I cannot see how this driver can be merged,
> >>> unfortunately.
> >>
> >> No problem. I just wanted to help people by giving the job I did on the
> >> boards I have. My boards are working for almost one year, fine enough
> >> for I use them as daily desktop computers. I don't want to spend one
> >> more year for having my code in the Linux kernel: there are so much
> >> other exciting things to do...
> >
> > And you're certainly welcome to contribute drivers to the kernel, that's
> > always appreciated. Of course, to ensure a reasonable level of quality and
> > consistency between drivers, the review process often requires changes to
> > be made to the code being submitted. When it comes to drivers I mostly
> > pay attention to DT bindings, userspace APIs and modification to common
> > code. Driver code itself, as long as it's reasonably clean and doesn't
> > impede development of other drivers or impact system security in an
> > adverse way, is still important but maybe slightly less so. I'll defer to
> > Maxime to come to an agreement on the multiple display engines in
> > parallel problem as I'm not familiar with it for the Allwinner platforms.
>
> The earlier Allwinner SoCs (with the old display engine), we had some
> SoCs with multiple instances of the display engine and TCON (the
> display engine roughly implementing the planes, the TCON the
> CRTC. Roughly.). However, those were sharing some encoders (HDMI,
> DSI) after that.
>
> So we need to have a single DRM device taking care of the multiple
> display engines, which essentialy means that we have to decouple the
> DRM device from the display engine. This was done in the earlier
> designs using an additional node with a list of phandles to the
> display engines in the system, and obviously, I'd prefer to have some
> consistency and reuse the same thing.
>
> But the current approach doesn't work and will require some DT
> modifications if that case happens again, which we can't do because of
> the DT ABI.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Maxime Ripard @ 2016-12-01 9:21 UTC (permalink / raw)
To: Emmanuel Vadot
Cc: mark.rutland, devicetree, linux, linux-kernel, wens, robh+dt,
linux-arm-kernel
In-Reply-To: <20161125220752.c989c85e01ed202be0485c78@bidouilliste.com>
[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]
Hi Emmanuel,
On Fri, Nov 25, 2016 at 10:07:52PM +0100, Emmanuel Vadot wrote:
> On Fri, 25 Nov 2016 16:20:47 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > On Thu, Nov 24, 2016 at 09:05:09PM +0100, Emmanuel Vadot wrote:
> > > On Thu, 24 Nov 2016 20:55:17 +0100
> > > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > >
> > > > On Tue, Nov 22, 2016 at 06:06:16PM +0100, Emmanuel Vadot wrote:
> > > > > The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
> > > > > others three only have 1.
> > > > > Add the num-cs property to each node.
> > > > >
> > > > > Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com>
> > > >
> > > > I don't think we have any code that uses it at the moment. What is the
> > > > rationale behind this patch?
> > > >
> > > > Thanks!
> > > > Maxime
> > > >
> > > > --
> > > > Maxime Ripard, Free Electrons
> > > > Embedded Linux and Kernel engineering
> > > > http://free-electrons.com
> > >
> > > Hi Maxime,
> > >
> > > If num-cs isn't present nothing prevent to start a transfer with a
> > > non-valid CS pin, resulting in an error.
> > > num-cs are default property especially made for this and a SPI driver
> > > should try to get the property at probe/attach time.
> >
> > Yes, but as far as I know, our driver doesn't. I'm all in for having
> > support for that in our driver, but without it, that patch is kind of
> > useless.
>
> Yes the Linux driver doesn't use it but my upcoming one for FreeBSD
> uses it. So it is not useless for downstream user of DTS.
Ah, I didn't know this was for FreeBSD. So you started to use our DTs,
or do you have some modifications to it? How does that work?
Anyway, the fact that it isn't used by our driver at the moment and
that it's meant for other OSes should be mentionned in the commit log.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* How should we group related devices in DT ? (was Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support)
From: Laurent Pinchart @ 2016-12-01 9:28 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201091313.th7nucjmvtuolqza@lukather>
Hello,
On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
[snip]
> The earlier Allwinner SoCs (with the old display engine), we had some
> SoCs with multiple instances of the display engine and TCON (the
> display engine roughly implementing the planes, the TCON the
> CRTC. Roughly.). However, those were sharing some encoders (HDMI,
> DSI) after that.
>
> So we need to have a single DRM device taking care of the multiple
> display engines, which essentialy means that we have to decouple the
> DRM device from the display engine. This was done in the earlier
> designs using an additional node with a list of phandles to the
> display engines in the system, and obviously, I'd prefer to have some
> consistency and reuse the same thing.
I believe this problem isn't limited to sunxi and should be addressed in a
more generic way. How should we describe in the device tree that multiple
instances of a device unrelated from a control point of view are related at
the hardware level ? There are multiple reasons why we need this, and here are
a few.
- As described above, unrelated display controller instances that share
encoders at their output need to be exposed to userspace as a single DRM
device. This is also the case on Renesas platforms (where the display engines
are independent except for the "small" detail that output routing is
controlled through the first display engine).
- On Renesas platforms again a radio-related SPI receiver has multiple
independent channels that each have their own registers, clocks and
interrupts, but share the same physical clock and sync pins. They are used to
receive multiple channels of the same data stream and must be exposed as a
single V4L2 device to userspace. A generic DT binding RFC is available at
http://www.spinics.net/lists/devicetree/msg152414.html.
> But the current approach doesn't work and will require some DT
> modifications if that case happens again, which we can't do because of
> the DT ABI.
--
Regards,
Laurent Pinchart
--
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 2/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Maxime Ripard @ 2016-12-01 9:36 UTC (permalink / raw)
To: André Przywara
Cc: Mark Rutland, devicetree@vger.kernel.org, Vishnu Patekar,
Arnd Bergmann, Jonathan Corbet, linux-doc@vger.kernel.org,
Russell King, linux-kernel@vger.kernel.org, Hans de Goede,
Chen-Yu Tsai, Icenowy Zheng, linux-arm-kernel@lists.infradead.org
In-Reply-To: <cdfd3c65-d473-badb-ea6a-035f7ab79217@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 1083 bytes --]
On Mon, Nov 28, 2016 at 12:29:07AM +0000, André Przywara wrote:
> > Something more interesting happened.
> >
> > Xunlong made a add-on board for Orange Pi Zero, which exposes the
> > two USB Controllers exported at expansion bus as USB Type-A
> > connectors.
> >
> > Also it exposes a analog A/V jack and a microphone.
> >
> > Should I enable {e,o}hci{2.3} in the device tree?
>
> Actually we should do this regardless of this extension board. The USB
> pins are not multiplexed and are exposed on user accessible pins (just
> not soldered, but that's a detail), so I think they qualify for DT
> enablement. And even if a user can't use them, it doesn't hurt to have
> them (since they are not multiplexed).
My main concern about this is that we'll leave regulators enabled by
default, for a minority of users. And that minority will prevent to do
a proper power management when the times come since we'll have to keep
that behaviour forever.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Maxime Ripard @ 2016-12-01 9:42 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <2084988.kISO4Quil7@avalon>
[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]
On Thu, Dec 01, 2016 at 11:19:56AM +0200, Laurent Pinchart wrote:
> Hi Maxime,
>
> On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> > On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> > >> More, it is not sure that the bridge/DW code would work with Allwinner's
> > >> SoCs.
> > >
> > > If it doesn't work and can't be made to work in a non-invasive way they it
> > > should certainly not be used :-)
> >
> > Even if the register layout is completely scrambled, as long as the
> > bits themselves aren't (and by comparing the two drivers it looks like
> > they haven't changed), you can easily deal with that using the
> > regmap_fields, with the two implementations (the original one and the
> > scrambled one) providing their register map that way, and the driver
> > code using whatever has been provided.
>
> Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it
> seems that an address remapping function could be used.
Even better.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
From: Ulf Hansson @ 2016-12-01 9:51 UTC (permalink / raw)
To: Yong Mao
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
YH Huang, Nicolas Boichat, Mathias Nyman, srv_heupstream,
Catalin Marinas, Will Deacon, Douglas Anderson,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Chunfeng Yun, Rob Herring, Geert Uytterhoeven,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Philipp Zabel, Matthias Brugger,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eddie Huang,
Chaotian Jing <chaot>
In-Reply-To: <1478585341-6749-2-git-send-email-yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On 8 November 2016 at 07:08, Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> From: yong mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> When initializing EMMC, after switch to HS400,
> it will issue CMD6 to change ext_csd, if first CMD6 got CRC
> error, the repeat CMD6 may get timeout, that's
> because SDCBSY was cleared by msdc_reset_hw()
Sorry for the delay!
We have recently been re-working the sequence for how to deal more
properly with CMD6 in the mmc core.
The changes done so far should mostly concern switches to HS and HS
DDR, but I think you should run a re-test to make sure you still hit
the same problems.
>
> Signed-off-by: Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Chaotian Jing <chaotian.jing-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/mmc/host/mtk-sd.c | 77 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afc..b29683b 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
> return true;
> }
>
> +static int msdc_card_busy(struct mmc_host *mmc)
> +{
> + struct msdc_host *host = mmc_priv(mmc);
> + u32 status = readl(host->base + MSDC_PS);
> +
> + /* check if data0 is low */
> + return !(status & BIT(16));
> +}
> +
> /* It is the core layer's responsibility to ensure card status
> * is correct before issue a request. but host design do below
> * checks recommended.
Hmm. Why?
I think you should rely on the mmc core to invoke the ->card_busy()
ops instead. The core knows better when it's needed.
Perhaps that may also resolve some of these issues for you!?
> @@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> {
> /* The max busy time we can endure is 20ms */
> unsigned long tmo = jiffies + msecs_to_jiffies(20);
> + u32 count = 0;
> +
> + if (in_interrupt()) {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + (count < 1000)) {
> + udelay(1);
> + count++;
This seems like a bad idea, "busy-wait" in irq context is never a good idea.
> + }
> + } else {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + time_before(jiffies, tmo))
> + cpu_relax();
> + }
>
> - while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
> dev_err(host->dev, "CMD bus busy detected\n");
> host->error |= REQ_CMD_BUSY;
> @@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> return false;
> }
>
> - if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
> - tmo = jiffies + msecs_to_jiffies(20);
> - /* R1B or with data, should check SDCBUSY */
> - while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> - if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
> - dev_err(host->dev, "Controller busy detected\n");
> - host->error |= REQ_CMD_BUSY;
> - msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> - return false;
> + if (cmd->opcode != MMC_SEND_STATUS) {
> + count = 0;
> + /* Consider that CMD6 crc error before card was init done,
> + * mmc_retune() will return directly as host->card is null.
> + * and CMD6 will retry 3 times, must ensure card is in transfer
> + * state when retry.
> + */
> + tmo = jiffies + msecs_to_jiffies(60 * 1000);
> + while (1) {
> + if (msdc_card_busy(host->mmc)) {
> + if (in_interrupt()) {
> + udelay(1);
> + count++;
> + } else {
> + msleep_interruptible(10);
> + }
> + } else {
> + break;
> + }
> + /* Timeout if the device never
> + * leaves the program state.
> + */
> + if (count > 1000 || time_after(jiffies, tmo)) {
> + pr_err("%s: Card stuck in programming state! %s\n",
> + mmc_hostname(host->mmc), __func__);
> + host->error |= REQ_CMD_BUSY;
> + msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> + return false;
> + }
This hole new code is a hack, that shouldn't be needed in the host driver.
I think we need to investigate and fix the issue in the mmc core
layer, to make this work for your host driver. That instead of doing a
work around in the host.
> }
> }
> return true;
> @@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
> return ret;
> }
>
> -static int msdc_card_busy(struct mmc_host *mmc)
> -{
> - struct msdc_host *host = mmc_priv(mmc);
> - u32 status = readl(host->base + MSDC_PS);
> -
> - /* check if any pin between dat[0:3] is low */
> - if (((status >> 16) & 0xf) != 0xf)
> - return 1;
> -
> - return 0;
> -}
> -
> static void msdc_request_timeout(struct work_struct *work)
> {
> struct msdc_host *host = container_of(work, struct msdc_host,
> --
> 1.7.9.5
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: Sudeep Holla @ 2016-12-01 9:58 UTC (permalink / raw)
To: Arnd Bergmann, 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: <7823573.FNB8ayVOnQ@wuerfel>
On 30/11/16 22:51, Arnd Bergmann wrote:
> 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.
>
No that's fine.
--
Regards,
Sudeep
--
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] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Emmanuel Vadot @ 2016-12-01 10:24 UTC (permalink / raw)
To: Maxime Ripard
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
wens-jdAy2FN1RRM, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201092150.rlo5skxd6elovlgq@lukather>
Hi Maxime,
On Thu, 1 Dec 2016 10:21:50 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Emmanuel,
>
> On Fri, Nov 25, 2016 at 10:07:52PM +0100, Emmanuel Vadot wrote:
> > On Fri, 25 Nov 2016 16:20:47 +0100
> > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >
> > > On Thu, Nov 24, 2016 at 09:05:09PM +0100, Emmanuel Vadot wrote:
> > > > On Thu, 24 Nov 2016 20:55:17 +0100
> > > > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > >
> > > > > On Tue, Nov 22, 2016 at 06:06:16PM +0100, Emmanuel Vadot wrote:
> > > > > > The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
> > > > > > others three only have 1.
> > > > > > Add the num-cs property to each node.
> > > > > >
> > > > > > Signed-off-by: Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
> > > > >
> > > > > I don't think we have any code that uses it at the moment. What is the
> > > > > rationale behind this patch?
> > > > >
> > > > > Thanks!
> > > > > Maxime
> > > > >
> > > > > --
> > > > > Maxime Ripard, Free Electrons
> > > > > Embedded Linux and Kernel engineering
> > > > > http://free-electrons.com
> > > >
> > > > Hi Maxime,
> > > >
> > > > If num-cs isn't present nothing prevent to start a transfer with a
> > > > non-valid CS pin, resulting in an error.
> > > > num-cs are default property especially made for this and a SPI driver
> > > > should try to get the property at probe/attach time.
> > >
> > > Yes, but as far as I know, our driver doesn't. I'm all in for having
> > > support for that in our driver, but without it, that patch is kind of
> > > useless.
> >
> > Yes the Linux driver doesn't use it but my upcoming one for FreeBSD
> > uses it. So it is not useless for downstream user of DTS.
>
> Ah, I didn't know this was for FreeBSD. So you started to use our DTs,
> or do you have some modifications to it? How does that work?
Yes we use the DTS from linux from quite some times now. We're
currently synced with 4.7-ish.
We either use them directly or modify them according to our needs and
driver support.
> Anyway, the fact that it isn't used by our driver at the moment and
> that it's meant for other OSes should be mentionned in the commit log.
Yeah I understand, I'll send a v2 with this in the commit log.
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> <manu-h+KGxgPPiopAfugRpC6u6w@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
* [PATCH v3 0/5] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).
This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.
Romain Perier (5):
spi: Add basic support for Armada 3700 SPI Controller
spi: armada-3700: Add support for the FIFO mode
spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
arm64: dts: marvell: Add definition of SPI controller for Armada 3700
arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
.../devicetree/bindings/spi/spi-armada-3700.txt | 25 +
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 +
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 1046 ++++++++++++++++++++
6 files changed, 1120 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
create mode 100644 drivers/spi/spi-armada-3700.c
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).
This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Fixed wrong variable passed as MODULE_DEVICE_TABLE
- Added missing null terminated entry in a3700_spi_dt_ids
- Added the tag "Tested-by" by Gregory
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 652 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 660 insertions(+)
create mode 100644 drivers/spi/spi-armada-3700.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
This enables support for the SPI controller present on the
Atheros AR71XX/AR724X/AR913X SoCs.
+config SPI_ARMADA_3700
+ tristate "Marvell Armada 3700 SPI Controller"
+ depends on ARCH_MVEBU && OF
+ help
+ This enables support for the SPI controller present on the
+ Marvell Armada 3700 SoCs.
+
config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_ALTERA) += spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700) += spi-armada-3700.o
obj-$(CONFIG_SPI_ATMEL) += spi-atmel.o
obj-$(CONFIG_SPI_ATH79) += spi-ath79.o
obj-$(CONFIG_SPI_AU1550) += spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..4115685
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,652 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
+ * Author: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME "armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT 10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG 0x00
+#define A3700_SPI_IF_CFG_REG 0x04
+#define A3700_SPI_DATA_OUT_REG 0x08
+#define A3700_SPI_DATA_IN_REG 0x0C
+#define A3700_SPI_IF_INST_REG 0x10
+#define A3700_SPI_IF_ADDR_REG 0x14
+#define A3700_SPI_IF_RMODE_REG 0x18
+#define A3700_SPI_IF_HDR_CNT_REG 0x1C
+#define A3700_SPI_IF_DIN_CNT_REG 0x20
+#define A3700_SPI_IF_TIME_REG 0x24
+#define A3700_SPI_INT_STAT_REG 0x28
+#define A3700_SPI_INT_MASK_REG 0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW BIT(8)
+#define A3700_SPI_WFIFO_FULL BIT(7)
+#define A3700_SPI_WFIFO_EMPTY BIT(6)
+#define A3700_SPI_RFIFO_FULL BIT(5)
+#define A3700_SPI_RFIFO_EMPTY BIT(4)
+#define A3700_SPI_WFIFO_RDY BIT(3)
+#define A3700_SPI_RFIFO_RDY BIT(2)
+#define A3700_SPI_XFER_RDY BIT(1)
+#define A3700_SPI_XFER_DONE BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS BIT(28)
+#define A3700_SPI_RFIFO_THRS BIT(24)
+#define A3700_SPI_AUTO_CS BIT(20)
+#define A3700_SPI_DMA_RD_EN BIT(18)
+#define A3700_SPI_FIFO_MODE BIT(17)
+#define A3700_SPI_SRST BIT(16)
+#define A3700_SPI_XFER_START BIT(15)
+#define A3700_SPI_XFER_STOP BIT(14)
+#define A3700_SPI_INST_PIN BIT(13)
+#define A3700_SPI_ADDR_PIN BIT(12)
+#define A3700_SPI_DATA_PIN1 BIT(11)
+#define A3700_SPI_DATA_PIN0 BIT(10)
+#define A3700_SPI_FIFO_FLUSH BIT(9)
+#define A3700_SPI_RW_EN BIT(8)
+#define A3700_SPI_CLK_POL BIT(7)
+#define A3700_SPI_CLK_PHA BIT(6)
+#define A3700_SPI_BYTE_LEN BIT(5)
+#define A3700_SPI_CLK_PRESCALE BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK (0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT 28
+#define A3700_SPI_RFIFO_THRS_BIT 24
+#define A3700_SPI_FIFO_THRS_MASK 0x7
+
+#define A3700_SPI_DATA_PIN_MASK 0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT 12
+#define A3700_SPI_DUMMY_CNT_MASK 0x7
+#define A3700_SPI_RMODE_CNT_BIT 8
+#define A3700_SPI_RMODE_CNT_MASK 0x3
+#define A3700_SPI_ADDR_CNT_BIT 4
+#define A3700_SPI_ADDR_CNT_MASK 0x7
+#define A3700_SPI_INSTR_CNT_BIT 0
+#define A3700_SPI_INSTR_CNT_MASK 0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+
+struct a3700_spi {
+ struct spi_master *master;
+ void __iomem *base;
+ struct clk *clk;
+ unsigned int irq;
+ unsigned int flags;
+ bool last_xfer;
+ const u8 *tx_buf;
+ u8 *rx_buf;
+ size_t buf_len;
+ u8 byte_len;
+ u32 wait_mask;
+ struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+ return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+ writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_AUTO_CS;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val |= (A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+ unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val &= ~(A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int pin_mode)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+ val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+ switch (pin_mode) {
+ case 1:
+ break;
+ case 2:
+ val |= A3700_SPI_DATA_PIN0;
+ break;
+ case 4:
+ val |= A3700_SPI_DATA_PIN1;
+ break;
+ default:
+ dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+ return -EINVAL;
+ }
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_FIFO_MODE;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int mode_bits)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+ if (mode_bits & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+ else
+ val &= ~A3700_SPI_CLK_POL;
+
+ if (mode_bits & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+ else
+ val &= ~A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+ unsigned int speed_hz, u16 mode)
+{
+ u32 val;
+ u32 prescale;
+
+ prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+ val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ if (prescale <= 2) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+ val |= A3700_SPI_CLK_CAPT_EDGE;
+ spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+ if (mode & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+
+ if (mode & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (len == 4)
+ val |= A3700_SPI_BYTE_LEN;
+ else
+ val &= ~A3700_SPI_BYTE_LEN;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+ struct spi_master *master = a3700_spi->master;
+ u32 val;
+ int i;
+
+ /* Reset SPI unit */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+ udelay(1);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /* Disable AUTO_CS and deactivate all chip-selects */
+ a3700_spi_auto_cs_unset(a3700_spi);
+ for (i = 0; i < master->num_chipselect; i++)
+ a3700_spi_deactivate_cs(a3700_spi, i);
+
+ a3700_spi_pin_mode_set(a3700_spi, 0);
+
+ /* Be sure that FIFO mode is disabled */
+ a3700_spi_fifo_mode_unset(a3700_spi);
+
+ /* Set SPI mode */
+ a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+ /* Reset counters */
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+ /* Mask the interrupts and clear cause bits */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+ return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+ struct spi_master *master = dev_id;
+ struct a3700_spi *a3700_spi;
+ u32 cause;
+
+ a3700_spi = spi_master_get_devdata(master);
+
+ /* Get interrupt causes */
+ cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+ /* mask and acknowledge the SPI interrupts */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+ /* Wake up the transfer */
+ if (a3700_spi->wait_mask & cause)
+ complete(&a3700_spi->done);
+
+ return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+ struct a3700_spi *a3700_spi;
+ unsigned int timeout;
+ unsigned int ctrl_reg;
+ unsigned long timeout_jiffies;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ /* SPI interrupt is edge-triggered, which means an interrupt will
+ * be generated only when detecting a specific status bit changed
+ * from '0' to '1'. So when we start waiting for a interrupt, we
+ * need to check status bit in control reg first, if it is already 1,
+ * then we do not need to wait for interrupt
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ reinit_completion(&a3700_spi->done);
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+ a3700_spi->wait_mask);
+
+ timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+ timeout = wait_for_completion_timeout(&a3700_spi->done,
+ timeout_jiffies);
+
+ a3700_spi->wait_mask = 0;
+
+ if (timeout)
+ return true;
+
+ /* there might be the case that right after we checked the
+ * status bits in this routine and before start to wait for
+ * interrupt by wait_for_completion_timeout, the interrupt
+ * happens, to avoid missing it we need to double check
+ * status bits in control reg, if it is already 1, then
+ * consider that we have the interrupt successfully and
+ * return true.
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+ return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+ unsigned int bit_mask)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+ a3700_spi->wait_mask = bit_mask;
+
+ return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+ u32 val, data;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Read bytes from data in register */
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+ if (a3700_spi->byte_len == 4)
+ data = be32_to_cpu(val);
+ else
+ data = val;
+
+ memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->rx_buf += a3700_spi->byte_len;
+
+ /* Request next 1 or 4 bytes data */
+ if (a3700_spi->buf_len)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+ u32 val = 0;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Write bytes from data out register */
+ if (a3700_spi->byte_len == 4)
+ val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+ else
+ val = a3700_spi->tx_buf[0];
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->tx_buf += a3700_spi->byte_len;
+
+ return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+ if (!enable)
+ a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+ else
+ a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ struct spi_device *spi = message->spi;
+
+ a3700_spi_bytelen_set(a3700_spi, 1);
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* Start READ transfer by writing dummy data to DOUT register */
+ if (xfer->rx_buf)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ while (a3700_spi->buf_len) {
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto err;
+ }
+
+ if (a3700_spi->tx_buf) {
+ ret = a3700_spi_write_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+
+ if (a3700_spi->rx_buf) {
+ ret = a3700_spi_read_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+ }
+
+err:
+ spi_finalize_current_transfer(master);
+ return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct spi_device *spi = message->spi;
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+ { .compatible = "marvell,armada-3700-spi", .data = NULL },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_dt_ids);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *of_node = dev->of_node;
+ struct resource *res;
+ struct spi_master *master;
+ struct a3700_spi *spi;
+ u32 num_cs = 0;
+ int ret = 0;
+
+ master = spi_alloc_master(dev, sizeof(*spi));
+ if (!master) {
+ dev_err(dev, "master allocation failed\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+ dev_err(dev, "could not find num-cs\n");
+ ret = -ENXIO;
+ goto error;
+ }
+
+ master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+ master->dev.of_node = of_node;
+ master->mode_bits = SPI_MODE_3;
+ master->num_chipselect = num_cs;
+ master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ master->set_cs = a3700_spi_set_cs;
+
+ platform_set_drvdata(pdev, master);
+
+ spi = spi_master_get_devdata(master);
+ memset(spi, 0, sizeof(struct a3700_spi));
+
+ spi->master = master;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ spi->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(spi->base)) {
+ ret = PTR_ERR(spi->base);
+ goto error;
+ }
+
+ spi->irq = platform_get_irq(pdev, 0);
+ if (spi->irq < 0) {
+ dev_err(dev, "could not get irq: %d\n", spi->irq);
+ ret = -ENXIO;
+ goto error;
+ }
+
+ init_completion(&spi->done);
+
+ spi->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(spi->clk)) {
+ dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+ goto error;
+ }
+
+ ret = clk_prepare_enable(spi->clk);
+ if (ret) {
+ dev_err(dev, "could not prepare clk: %d\n", ret);
+ goto error;
+ }
+
+ ret = a3700_spi_init(spi);
+ if (ret)
+ goto error_clk;
+
+ ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+ dev_name(dev), master);
+ if (ret) {
+ dev_err(dev, "could not request IRQ: %d\n", ret);
+ goto error_clk;
+ }
+
+ ret = devm_spi_register_master(dev, master);
+ if (ret) {
+ dev_err(dev, "Failed to register master\n");
+ goto error_clk;
+ }
+
+ dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+ (unsigned long)res->start, spi->irq);
+
+ return 0;
+
+error_clk:
+ clk_disable_unprepare(spi->clk);
+error:
+ spi_master_put(master);
+out:
+ return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct a3700_spi *spi = spi_master_get_devdata(master);
+
+ clk_disable_unprepare(spi->clk);
+ spi_master_put(master);
+
+ return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(a3700_spi_dt_ids),
+ },
+ .probe = a3700_spi_probe,
+ .remove = a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
--
2.9.3
--
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 related
* [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.
This commit adds support for the FIFO mode with the CPU.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Don't enable the fifo mode based on the compatible string, we introduce
a module parameter "pio_mode". By default this option is set to zero, so
the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
mode.
- Added tag "Tested-by" by Gregory
Changes in v2:
- Removed a3700_spi_bytelen_set from the setup function, it was accidentally
let here and not required, as it is configured in the prepare callback now
(defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
detection with jedec.
drivers/spi/spi-armada-3700.c | 414 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 404 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index 4115685..27a46cb 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -25,6 +25,11 @@
#include <linux/pinctrl/consumer.h>
#include <linux/spi/spi.h>
+static bool pio_mode;
+
+module_param(pio_mode, bool, 0);
+MODULE_PARM_DESC(pio_mode, "enable the PIO mode");
+
#define DRIVER_NAME "armada_3700_spi"
#define A3700_SPI_TIMEOUT 10
@@ -99,19 +104,28 @@
/* A3700_SPI_IF_TIME_REG */
#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO BIT(0)
+#define A3700_INSTR_CNT 1
+#define A3700_ADDR_CNT 3
+#define A3700_DUMMY_CNT 1
+
struct a3700_spi {
struct spi_master *master;
void __iomem *base;
struct clk *clk;
unsigned int irq;
unsigned int flags;
- bool last_xfer;
+ bool xmit_data;
const u8 *tx_buf;
u8 *rx_buf;
size_t buf_len;
u8 byte_len;
u32 wait_mask;
struct completion done;
+ u32 addr_cnt;
+ u32 instr_cnt;
+ size_t hdr_cnt;
};
static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +194,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
return 0;
}
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
{
u32 val;
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
- val &= ~A3700_SPI_FIFO_MODE;
+ if (a3700_spi->flags & HAS_FIFO)
+ val |= A3700_SPI_FIFO_MODE;
+ else
+ val &= ~A3700_SPI_FIFO_MODE;
spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
}
@@ -255,11 +272,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
a3700_spi->byte_len = len;
}
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_FIFO_FLUSH;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_FIFO_FLUSH))
+ return 0;
+ udelay(1);
+ }
+
+ return -ETIMEDOUT;
+}
+
static int a3700_spi_init(struct a3700_spi *a3700_spi)
{
struct spi_master *master = a3700_spi->master;
u32 val;
- int i;
+ int i, ret = 0;
/* Reset SPI unit */
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +314,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
for (i = 0; i < master->num_chipselect; i++)
a3700_spi_deactivate_cs(a3700_spi, i);
- a3700_spi_pin_mode_set(a3700_spi, 0);
-
- /* Be sure that FIFO mode is disabled */
- a3700_spi_fifo_mode_unset(a3700_spi);
+ /* Enable FIFO mode */
+ a3700_spi_fifo_mode_set(a3700_spi);
/* Set SPI mode */
a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +328,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
- return 0;
+ return ret;
}
static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +414,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
return a3700_spi_wait_completion(spi);
}
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+ unsigned int bytes)
+{
+ u32 val;
+
+ if (a3700_spi->flags & HAS_FIFO) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+ val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+ val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+}
+
static void a3700_spi_transfer_setup(struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi;
+ unsigned int byte_len;
a3700_spi = spi_master_get_devdata(spi->master);
a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+ byte_len = xfer->bits_per_word >> 3;
+
+ a3700_spi_fifo_thres_set(a3700_spi, byte_len);
}
static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +501,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
}
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+ u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+ u32 val = 0;
+
+ /* Clear the header registers */
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+ /* Set header counters */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+ instr_cnt = a3700_spi->buf_len;
+ } else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+ a3700_spi->addr_cnt)) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->buf_len - instr_cnt;
+ } else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->addr_cnt;
+ /* Need to handle the normal write case with 1 byte
+ * data
+ */
+ if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+ dummy_cnt = a3700_spi->buf_len - instr_cnt -
+ addr_cnt;
+ }
+ val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+ << A3700_SPI_INSTR_CNT_BIT);
+ val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+ << A3700_SPI_ADDR_CNT_BIT);
+ val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+ << A3700_SPI_DUMMY_CNT_BIT);
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+ /* Update the buffer length to be transferred */
+ a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+ /* Set Instruction */
+ val = 0;
+ while (instr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+ /* Set Address */
+ val = 0;
+ while (addr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+ int i = 0;
+
+ while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+ val = 0;
+ if (a3700_spi->buf_len >= 4) {
+ val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->tx_buf += 4;
+ } else {
+ /*
+ * If the remained buffer length is less than 4-bytes,
+ * we should pad the write buffer with all ones. So that
+ * it avoids overwrite the unexpected bytes following
+ * the last one.
+ */
+ val = GENMASK(31, 0);
+ while (a3700_spi->buf_len) {
+ val &= ~(0xff << (8 * i));
+ val |= *a3700_spi->tx_buf++ << (8 * i);
+ i++;
+ a3700_spi->buf_len--;
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+ val);
+ }
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+ u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+ return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+ if (a3700_spi->buf_len >= 4) {
+ u32 data = le32_to_cpu(val);
+ memcpy(a3700_spi->rx_buf, &data, 4);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->rx_buf += 4;
+ } else {
+ /*
+ * When remain bytes is not larger than 4, we should
+ * avoid memory overwriting and just write the left rx
+ * buffer bytes.
+ */
+ while (a3700_spi->buf_len) {
+ *a3700_spi->rx_buf = val & 0xff;
+ val >>= 8;
+
+ a3700_spi->buf_len--;
+ a3700_spi->rx_buf++;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ a3700_spi_fifo_flush(a3700_spi);
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
static int a3700_spi_prepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -463,12 +679,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
return 0;
}
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret;
+
+ /* Flush the FIFOs */
+ ret = a3700_spi_fifo_flush(a3700_spi);
+ if (ret)
+ return ret;
+
+ a3700_spi_bytelen_set(a3700_spi, 4);
+
+ return 0;
+}
+
static int a3700_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
- int ret = 0;
+ int ret;
a3700_spi_transfer_setup(spi, xfer);
@@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
return ret;
}
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0, timeout = A3700_SPI_TIMEOUT;
+ unsigned int nbits = 0;
+ u32 val;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* SPI transfer headers */
+ a3700_spi_header_set(a3700_spi);
+
+ if (xfer->tx_buf)
+ nbits = xfer->tx_nbits;
+ else if (xfer->rx_buf)
+ nbits = xfer->rx_nbits;
+
+ a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+ if (xfer->rx_buf) {
+ /* Set read data length */
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+ a3700_spi->buf_len);
+ /* Start READ transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_RW_EN;
+ val |= A3700_SPI_XFER_START;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ } else if (xfer->tx_buf) {
+ /* Start Write transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /*
+ * If there are data to be written to the SPI device, xmit_data
+ * flag is set true; otherwise the instruction in SPI_INSTR does
+ * not require data to be written to the SPI device, then
+ * xmit_data flag is set false.
+ */
+ a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+ }
+
+ while (a3700_spi->buf_len) {
+ if (a3700_spi->tx_buf) {
+ /* Wait wfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait wfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Fill up the wfifo */
+ ret = a3700_spi_fifo_write(a3700_spi);
+ if (ret)
+ goto error;
+ } else if (a3700_spi->rx_buf) {
+ /* Wait rfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_RFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait rfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Drain out the rfifo */
+ ret = a3700_spi_fifo_read(a3700_spi);
+ if (ret)
+ goto error;
+ }
+ }
+
+ /*
+ * Stop a write transfer in fifo mode:
+ * - wait all the bytes in wfifo to be shifted out
+ * - set XFER_STOP bit
+ * - wait XFER_START bit clear
+ * - clear XFER_STOP bit
+ * Stop a read transfer in fifo mode:
+ * - the hardware is to reset the XFER_START bit
+ * after the number of bytes indicated in DIN_CNT
+ * register
+ * - just wait XFER_START bit clear
+ */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->xmit_data) {
+ /*
+ * If there are data written to the SPI device, wait
+ * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+ * transfer out of write FIFO.
+ */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_EMPTY)) {
+ dev_err(&spi->dev, "wait wfifo empty timed out\n");
+ return -ETIMEDOUT;
+ }
+ } else {
+ /*
+ * If the instruction in SPI_INSTR does not require data
+ * to be written to the SPI device, wait until SPI_RDY
+ * is 1 for the SPI interface to be in idle.
+ */
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait xfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ if (timeout == 0) {
+ dev_err(&spi->dev, "wait transfer start clear timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ goto out;
+
+error:
+ a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+ spi_finalize_current_transfer(master);
+
+ return ret;
+}
+
static int a3700_spi_unprepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -593,6 +970,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
goto error;
}
+ if (!pio_mode) {
+ master->prepare_message = a3700_spi_prepare_fifo_message;
+ master->transfer_one = a3700_spi_fifo_transfer_one;
+
+ spi->flags |= HAS_FIFO;
+ spi->instr_cnt = A3700_INSTR_CNT;
+ spi->addr_cnt = A3700_ADDR_CNT;
+ spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+ A3700_DUMMY_CNT;
+ master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+ SPI_RX_QUAD | SPI_TX_QUAD);
+ } else {
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ }
+
ret = a3700_spi_init(spi);
if (ret)
goto error_clk;
--
2.9.3
--
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 related
* [PATCH v3 3/5] spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Added tag "Tested-by" by Gregory
- Fixed commit title, as requested by Mark Brown
.../devicetree/bindings/spi/spi-armada-3700.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+ the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+ spi0: spi@10600 {
+ compatible = "marvell,armada-3700-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10600 0x5d>;
+ clocks = <&nb_perih_clk 7>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+ num-cs = <4>;
+ };
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 related
* [PATCH v3 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Fixed wrong register size for spi0, as suggested by the maintainer
on the ML.
- Added tag "Tested-by" by Gregory
Changes in v2:
- Removed properties max-frequency and clock-frequency, it is no
longer required and not used by the DT-bindings.
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..fcef9a5 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
/* 32M internal register @ 0xd000_0000 */
ranges = <0x0 0x0 0xd0000000 0x2000000>;
+ spi0: spi@10600 {
+ compatible = "marvell,armada-3700-spi";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x10600 0xA00>;
+ clocks = <&nb_periph_clk 7>;
+ interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+ num-cs = <4>;
+ status = "disabled";
+ };
+
uart0: serial@12000 {
compatible = "marvell,armada-3700-uart";
reg = <0x12000 0x400>;
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 related
* [PATCH v3 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Added tag "Tested-by" by Gregory
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
status = "okay";
};
+&spi0 {
+ status = "okay";
+
+ m25p80@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <108000000>;
+ spi-rx-bus-width = <4>;
+ spi-tx-bus-width = <4>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ partition@0 {
+ label = "bootloader";
+ reg = <0x0 0x200000>;
+ };
+ partition@200000 {
+ label = "U-boot Env";
+ reg = <0x200000 0x10000>;
+ };
+ partition@210000 {
+ label = "Linux";
+ reg = <0x210000 0xDF0000>;
+ };
+ };
+ };
+};
+
/* Exported on the micro USB connector CON32 through an FTDI */
&uart0 {
status = "okay";
--
2.9.3
--
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 related
* Re: Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-12-01 10:41 UTC (permalink / raw)
To: Maxime Ripard
Cc: Icenowy Zheng, moinejf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Dave Airlie, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161201085520.tk2kdi33rn6fyld2@lukather>
On Thursday 01 Dec 2016 09:55:20 Maxime Ripard wrote:
> On Thu, Dec 01, 2016 at 01:33:30AM +0800, Icenowy Zheng wrote:
> >>> hdmi-out {
> >>> compatible = "hdmi-connector";
> >>> type = "a";
> >>> /* I2C bus and GPIO references are made up for the
> >>> example */ ddc-i2c-bus = <&i2c4>;
> >>> hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> >>
> >> the "hdmi-connector" is a big piece of software. It must handle a lot
> >> of more and more exotic connectors.
> >> So, I hope that you have written a "simple-hdmi-connector" which does
> >> nothing but setting the connector type.
> >> Where is it?
> >
> > I suddenly thought about something...
> >
> > If a DVI connector instead of a HDMI connector is soldered, how
> > should such a device tree be written?
>
> Use a dvi-connector instead :)
The HDMI encoder DT node doesn't (and certainly shouldn't) report what type of
connector is mounted on the board. Having a connector node in DT makes the
connector type available to the system, allowing the DRM driver to expose the
right connector type to userspace (it would be confusing for the user to
report DRM_MODE_CONNECTOR_HDMIA for a DVI connector).
> > How about solder a HDMI-to-VGA bridge on the board? (Maybe there
> > should be "dumb-hdmi-dvi-bridge" and "dumb-hdmi-vga-bridge"
> > drivers?)
>
> It probably wouldn't be dumb, but yeah, it would definitely be a
> bridge instead of the connector.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH V7 00/10] PM / OPP: Multiple regulator support
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, Stephen Boyd, nm, Vincent Guittot, robh,
d-gerlach, broonie, devicetree, Viresh Kumar
Hi,
Some platforms (like TI) have complex DVFS configuration for CPU
devices, where multiple regulators are required to be configured to
change DVFS state of the device. This was explained well by Nishanth
earlier [1].
One of the major complaints around multiple regulators case was that the
DT isn't responsible in any way to represent the order in which multiple
supplies need to be programmed, before or after a frequency change. It
was considered in this patch and such information is left for the
platform specific OPP driver now, which can register its own
opp_set_rate() callback with the OPP core and the OPP core will then
call it during DVFS.
The patches are tested on Exynos5250 (Dual A15). I have hacked around DT
and code to pass values for multiple regulators and verified that they
are all properly read by the kernel (using debugfs interface).
Dave Gerlach has already tested [2] it on the real TI platforms and it
works well for him.
This is rebased over: linux-next branch in the PM tree.
V6->V7:
- Added RBY from Stephen for the 8th patch as well.
- Rebased over pm/bleeding-edge as the dependency patch is already
applied there.
- s/dev_pm_opp_set_regulator()/dev_pm_opp_set_regulators() in a comment.
- Removed the local 'names' array in cpufreq-dt and used "&name"
instead.
- Only the 6th patch doesn't have a Reviewed-by Tag now..
V5->V6:
- Rebased over a recent fix (and resolved rebase conflicts) that will
get applied before this series:
https://marc.info/?l=linux-kernel&m=148047843010418&w=2
V4->V5:
- Stephen boyd had some minor review comments and gave his Reviewed-by
tag for the rest. Only 2 patches don't have his RBY tag.
- Individual patches contain the version history from V4 to V5.
V3->V4:
- Separate out cpu-supply fix in the binding in a separate patch (Mark).
- Add more documentation to the binding to explain that the relation to
the supplies and the order of programming them is left for the
platform specific bindings and that every platform using multiple
regulators for their devices needs to provide a separate binding
document explaining their implementation (Mark).
- @Rob and Stephen: I have kept your Acks for the bindings as the
bindings only got a bit reworded (improved) since the time you guys
Acked them. Please let me know if you want more improvement in the
bindings now.
- V4 for 10/10 was sent earlier, which added a missing
rcu_read_unlock(). Nothing else changed in it.
- Added some missing Kernel documentation comments
V2->V3:
- The last patch is new
- Removed a debug leftover pr_info() message
- Renamed few names as s/set_rate/set_opp
- Removed a TODO comment (as it is done now with this series)
- created struct for min_uV and max_uV
- kerneldoc comments for structures in pm_opp.h
- s/const char */const char * const
- use kasprintf()
- Some more minor reformatting
- More Ack/RBY tags added
V1->V2:
- Ack from Rob for 1st patch
- Moved the supplies structure to pm_opp.h (Dave)
- Fixed an compilation warning.
--
viresh
[1] https://marc.info/?l=linux-pm&m=145684495832764&w=2
[2] https://marc.info/?l=linux-kernel&m=147924789305276&w=2
Viresh Kumar (10):
PM / OPP: Fix incorrect cpu-supply property in binding
PM / OPP: Reword binding supporting multiple regulators per device
PM / OPP: Don't use OPP structure outside of rcu protected section
PM / OPP: Manage supply's voltage/current in a separate structure
PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
PM / OPP: Add infrastructure to manage multiple regulators
PM / OPP: Separate out _generic_set_opp()
PM / OPP: Allow platform specific custom set_opp() callbacks
PM / OPP: Don't WARN on multiple calls to dev_pm_opp_set_regulators()
PM / OPP: Don't assume platform doesn't have regulators
Documentation/devicetree/bindings/opp/opp.txt | 27 +-
drivers/base/power/opp/core.c | 538 ++++++++++++++++++++------
drivers/base/power/opp/debugfs.c | 52 ++-
drivers/base/power/opp/of.c | 105 +++--
drivers/base/power/opp/opp.h | 22 +-
drivers/cpufreq/cpufreq-dt.c | 6 +-
include/linux/pm_opp.h | 69 +++-
7 files changed, 633 insertions(+), 186 deletions(-)
--
2.7.1.410.g6faf27b
^ permalink raw reply
* [PATCH V7 01/10] PM / OPP: Fix incorrect cpu-supply property in binding
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The regulator bindings allow the "<name>-supply" property to define a
single parent supply and not a list of parents.
Fix the wrong example code present in OPP bindings.
While at it also change the compatible string as Rob pointed out earlier
that none of A7 implementation have multiple supplies for the CPU core.
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index ee91cbdd95ee..f0239f68d186 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -386,10 +386,12 @@ Example 4: Handling multiple regulators
/ {
cpus {
cpu@0 {
- compatible = "arm,cortex-a7";
+ compatible = "vendor,cpu-type";
...
- cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
+ vcc0-supply = <&cpu_supply0>;
+ vcc1-supply = <&cpu_supply1>;
+ vcc2-supply = <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
--
2.7.1.410.g6faf27b
--
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 related
* [PATCH V7 02/10] PM / OPP: Reword binding supporting multiple regulators per device
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, Vincent Guittot, robh, d-gerlach,
broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar@linaro.org>
On certain platforms (like TI), DVFS for a single device (CPU) requires
configuring multiple power supplies.
The OPP bindings already contains binding and example to explain this
case, but it isn't sufficient.
- There is no way for the code parsing these bindings to know which
voltage values belong to which power supply.
- It is not possible to know the order in which the supplies need to be
configured while switching OPPs.
This patch clarifies on those details by mentioning that such
information is left for the implementation specific bindings to explain.
They may want to hardcode such details or implement their own properties
to get such information. All implementations using multiple regulators
for their devices must provide a binding document explaining their
implementation.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index f0239f68d186..9f5ca4457b5f 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -86,8 +86,14 @@ properties.
Single entry is for target voltage and three entries are for <target min max>
voltages.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. The OPP binding doesn't provide any provisions to
+ relate the values to their power supplies or the order in which the supplies
+ need to be configured and that is left for the implementation specific
+ binding.
+
+ Entries for all regulators shall be of the same size, i.e. either all use a
+ single value or triplets.
- opp-microvolt-<name>: Named opp-microvolt property. This is exactly similar to
the above opp-microvolt property, but allows multiple voltage ranges to be
@@ -104,10 +110,13 @@ properties.
Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node. If this property isn't required
- for few regulators, then this should be marked as zero for them. If it isn't
- required for any regulator, then this property need not be present.
+ Entries for multiple regulators shall be provided in the same field separated
+ by angular brackets <>. If current values aren't required for a regulator,
+ then it shall be filled with 0. If current values aren't required for any of
+ the regulators, then this field is not required. The OPP binding doesn't
+ provide any provisions to relate the values to their power supplies or the
+ order in which the supplies need to be configured and that is left for the
+ implementation specific binding.
- opp-microamp-<name>: Named opp-microamp property. Similar to
opp-microvolt-<name> property, but for microamp instead.
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V7 03/10] PM / OPP: Don't use OPP structure outside of rcu protected section
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar, # v4 . 6+
In-Reply-To: <cover.1480564564.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The OPP structure must not be used out of the rcu protected section.
Cache the values to be used in separate variables instead.
Cc: # v4.6+ <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
---
drivers/base/power/opp/core.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 2824d3a5e9f0..6441dfda489f 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -584,6 +584,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct clk *clk;
unsigned long freq, old_freq;
unsigned long u_volt, u_volt_min, u_volt_max;
+ unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
int ret;
if (unlikely(!target_freq)) {
@@ -633,6 +634,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
+ if (IS_ERR(old_opp)) {
+ old_u_volt = 0;
+ } else {
+ old_u_volt = old_opp->u_volt;
+ old_u_volt_min = old_opp->u_volt_min;
+ old_u_volt_max = old_opp->u_volt_max;
+ }
+
u_volt = opp->u_volt;
u_volt_min = opp->u_volt_min;
u_volt_max = opp->u_volt_max;
@@ -677,9 +686,10 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (!IS_ERR(old_opp))
- _set_opp_voltage(dev, reg, old_opp->u_volt,
- old_opp->u_volt_min, old_opp->u_volt_max);
+ if (old_u_volt) {
+ _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
+ old_u_volt_max);
+ }
return ret;
}
--
2.7.1.410.g6faf27b
--
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 related
* [PATCH V7 04/10] PM / OPP: Manage supply's voltage/current in a separate structure
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
This is a preparatory step for multiple regulator per device support.
Move the voltage/current variables to a new structure.
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++++++-------------------
drivers/base/power/opp/debugfs.c | 8 ++++----
drivers/base/power/opp/of.c | 18 ++++++++--------
drivers/base/power/opp/opp.h | 11 +++-------
include/linux/pm_opp.h | 16 +++++++++++++++
5 files changed, 55 insertions(+), 42 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 6441dfda489f..188048d3d203 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -112,7 +112,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
- v = tmp_opp->u_volt;
+ v = tmp_opp->supply.u_volt;
return v;
}
@@ -246,10 +246,10 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
if (!opp->available)
continue;
- if (opp->u_volt_min < min_uV)
- min_uV = opp->u_volt_min;
- if (opp->u_volt_max > max_uV)
- max_uV = opp->u_volt_max;
+ if (opp->supply.u_volt_min < min_uV)
+ min_uV = opp->supply.u_volt_min;
+ if (opp->supply.u_volt_max > max_uV)
+ max_uV = opp->supply.u_volt_max;
}
rcu_read_unlock();
@@ -637,14 +637,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
if (IS_ERR(old_opp)) {
old_u_volt = 0;
} else {
- old_u_volt = old_opp->u_volt;
- old_u_volt_min = old_opp->u_volt_min;
- old_u_volt_max = old_opp->u_volt_max;
+ old_u_volt = old_opp->supply.u_volt;
+ old_u_volt_min = old_opp->supply.u_volt_min;
+ old_u_volt_max = old_opp->supply.u_volt_max;
}
- u_volt = opp->u_volt;
- u_volt_min = opp->u_volt_min;
- u_volt_max = opp->u_volt_max;
+ u_volt = opp->supply.u_volt;
+ u_volt_min = opp->supply.u_volt_min;
+ u_volt_max = opp->supply.u_volt_max;
reg = opp_table->regulator;
@@ -957,10 +957,11 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
struct regulator *reg = opp_table->regulator;
if (!IS_ERR(reg) &&
- !regulator_is_supported_voltage(reg, opp->u_volt_min,
- opp->u_volt_max)) {
+ !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
+ opp->supply.u_volt_max)) {
pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
- __func__, opp->u_volt_min, opp->u_volt_max);
+ __func__, opp->supply.u_volt_min,
+ opp->supply.u_volt_max);
return false;
}
@@ -993,11 +994,12 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
- __func__, opp->rate, opp->u_volt, opp->available,
- new_opp->rate, new_opp->u_volt, new_opp->available);
+ __func__, opp->rate, opp->supply.u_volt,
+ opp->available, new_opp->rate, new_opp->supply.u_volt,
+ new_opp->available);
- return opp->available && new_opp->u_volt == opp->u_volt ?
- 0 : -EEXIST;
+ return opp->available &&
+ new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1064,9 +1066,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
- new_opp->u_volt = u_volt;
- new_opp->u_volt_min = u_volt - tol;
- new_opp->u_volt_max = u_volt + tol;
+ new_opp->supply.u_volt = u_volt;
+ new_opp->supply.u_volt_min = u_volt - tol;
+ new_opp->supply.u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index ef1ae6b52042..c897676ca35f 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -63,16 +63,16 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->u_volt))
+ if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->u_volt_min))
+ if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->u_volt_max))
+ if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
return -ENOMEM;
- if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->u_amp))
+ if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
return -ENOMEM;
if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index 5b3755e49731..bdf409d42126 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -148,14 +148,14 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
return -EINVAL;
}
- opp->u_volt = microvolt[0];
+ opp->supply.u_volt = microvolt[0];
if (count == 1) {
- opp->u_volt_min = opp->u_volt;
- opp->u_volt_max = opp->u_volt;
+ opp->supply.u_volt_min = opp->supply.u_volt;
+ opp->supply.u_volt_max = opp->supply.u_volt;
} else {
- opp->u_volt_min = microvolt[1];
- opp->u_volt_max = microvolt[2];
+ opp->supply.u_volt_min = microvolt[1];
+ opp->supply.u_volt_max = microvolt[2];
}
/* Search for "opp-microamp-<name>" */
@@ -173,7 +173,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
}
if (prop && !of_property_read_u32(opp->np, name, &val))
- opp->u_amp = val;
+ opp->supply.u_amp = val;
return 0;
}
@@ -303,9 +303,9 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
mutex_unlock(&opp_table_lock);
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
- __func__, new_opp->turbo, new_opp->rate, new_opp->u_volt,
- new_opp->u_volt_min, new_opp->u_volt_max,
- new_opp->clock_latency_ns);
+ __func__, new_opp->turbo, new_opp->rate,
+ new_opp->supply.u_volt, new_opp->supply.u_volt_min,
+ new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
/*
* Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 96cd30ac6c1d..8a02516542c2 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,10 +61,7 @@ extern struct list_head opp_tables;
* @turbo: true if turbo (boost) OPP
* @suspend: true if suspend OPP
* @rate: Frequency in hertz
- * @u_volt: Target voltage in microvolts corresponding to this OPP
- * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
- * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
- * @u_amp: Maximum current drawn by the device in microamperes
+ * @supply: Power supply voltage/current values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
* frequency from any other OPP's frequency.
* @opp_table: points back to the opp_table struct this opp belongs to
@@ -83,10 +80,8 @@ struct dev_pm_opp {
bool suspend;
unsigned long rate;
- unsigned long u_volt;
- unsigned long u_volt_min;
- unsigned long u_volt_max;
- unsigned long u_amp;
+ struct dev_pm_opp_supply supply;
+
unsigned long clock_latency_ns;
struct opp_table *opp_table;
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index f6bc76501912..824f7268f687 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -25,6 +25,22 @@ enum dev_pm_opp_event {
OPP_EVENT_ADD, OPP_EVENT_REMOVE, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
};
+/**
+ * struct dev_pm_opp_supply - Power supply voltage/current values
+ * @u_volt: Target voltage in microvolts corresponding to this OPP
+ * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP
+ * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP
+ * @u_amp: Maximum current drawn by the device in microamperes
+ *
+ * This structure stores the voltage/current values for a single power supply.
+ */
+struct dev_pm_opp_supply {
+ unsigned long u_volt;
+ unsigned long u_volt_min;
+ unsigned long u_volt_max;
+ unsigned long u_amp;
+};
+
#if defined(CONFIG_PM_OPP)
unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
--
2.7.1.410.g6faf27b
--
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 related
* [PATCH V7 05/10] PM / OPP: Pass struct dev_pm_opp_supply to _set_opp_voltage()
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Pass the entire supply structure instead of all of its fields.
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
drivers/base/power/opp/core.c | 44 +++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 188048d3d203..46ad8470c438 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -542,8 +542,7 @@ static struct clk *_get_opp_clk(struct device *dev)
}
static int _set_opp_voltage(struct device *dev, struct regulator *reg,
- unsigned long u_volt, unsigned long u_volt_min,
- unsigned long u_volt_max)
+ struct dev_pm_opp_supply *supply)
{
int ret;
@@ -554,14 +553,15 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
return 0;
}
- dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__, u_volt_min,
- u_volt, u_volt_max);
+ dev_dbg(dev, "%s: voltages (mV): %lu %lu %lu\n", __func__,
+ supply->u_volt_min, supply->u_volt, supply->u_volt_max);
- ret = regulator_set_voltage_triplet(reg, u_volt_min, u_volt,
- u_volt_max);
+ ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
+ supply->u_volt, supply->u_volt_max);
if (ret)
dev_err(dev, "%s: failed to set voltage (%lu %lu %lu mV): %d\n",
- __func__, u_volt_min, u_volt, u_volt_max, ret);
+ __func__, supply->u_volt_min, supply->u_volt,
+ supply->u_volt_max, ret);
return ret;
}
@@ -583,8 +583,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
struct regulator *reg;
struct clk *clk;
unsigned long freq, old_freq;
- unsigned long u_volt, u_volt_min, u_volt_max;
- unsigned long old_u_volt, old_u_volt_min, old_u_volt_max;
+ struct dev_pm_opp_supply old_supply, new_supply;
int ret;
if (unlikely(!target_freq)) {
@@ -634,17 +633,12 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
- if (IS_ERR(old_opp)) {
- old_u_volt = 0;
- } else {
- old_u_volt = old_opp->supply.u_volt;
- old_u_volt_min = old_opp->supply.u_volt_min;
- old_u_volt_max = old_opp->supply.u_volt_max;
- }
+ if (IS_ERR(old_opp))
+ old_supply.u_volt = 0;
+ else
+ memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
- u_volt = opp->supply.u_volt;
- u_volt_min = opp->supply.u_volt_min;
- u_volt_max = opp->supply.u_volt_max;
+ memcpy(&new_supply, &opp->supply, sizeof(new_supply));
reg = opp_table->regulator;
@@ -652,8 +646,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling up? Scale voltage before frequency */
if (freq > old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_voltage;
}
@@ -672,8 +665,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
/* Scaling down? Scale voltage after frequency */
if (freq < old_freq) {
- ret = _set_opp_voltage(dev, reg, u_volt, u_volt_min,
- u_volt_max);
+ ret = _set_opp_voltage(dev, reg, &new_supply);
if (ret)
goto restore_freq;
}
@@ -686,10 +678,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
__func__, old_freq);
restore_voltage:
/* This shouldn't harm even if the voltages weren't updated earlier */
- if (old_u_volt) {
- _set_opp_voltage(dev, reg, old_u_volt, old_u_volt_min,
- old_u_volt_max);
- }
+ if (old_supply.u_volt)
+ _set_opp_voltage(dev, reg, &old_supply);
return ret;
}
--
2.7.1.410.g6faf27b
--
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 related
* [PATCH V7 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, Vincent Guittot, robh, d-gerlach,
broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar@linaro.org>
This patch adds infrastructure to manage multiple regulators and updates
the only user (cpufreq-dt) of dev_pm_opp_set{put}_regulator().
This is preparatory work for adding full support for devices with
multiple regulators.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
---
drivers/base/power/opp/core.c | 248 +++++++++++++++++++++++++++------------
drivers/base/power/opp/debugfs.c | 52 ++++++--
drivers/base/power/opp/of.c | 103 +++++++++++-----
drivers/base/power/opp/opp.h | 10 +-
drivers/cpufreq/cpufreq-dt.c | 6 +-
include/linux/pm_opp.h | 8 +-
6 files changed, 300 insertions(+), 127 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 46ad8470c438..b4da31c5a5eb 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -93,6 +93,8 @@ struct opp_table *_find_opp_table(struct device *dev)
* Return: voltage in micro volt corresponding to the opp, else
* return 0
*
+ * This is useful only for devices with single power supply.
+ *
* Locking: This function must be called under rcu_read_lock(). opp is a rcu
* protected pointer. This means that opp which could have been fetched by
* opp_find_freq_{exact,ceil,floor} functions is valid as long as we are
@@ -112,7 +114,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
- v = tmp_opp->supply.u_volt;
+ v = tmp_opp->supplies[0].u_volt;
return v;
}
@@ -210,6 +212,24 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
+static int _get_regulator_count(struct device *dev)
+{
+ struct opp_table *opp_table;
+ int count;
+
+ rcu_read_lock();
+
+ opp_table = _find_opp_table(dev);
+ if (!IS_ERR(opp_table))
+ count = opp_table->regulator_count;
+ else
+ count = 0;
+
+ rcu_read_unlock();
+
+ return count;
+}
+
/**
* dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds
* @dev: device for which we do this operation
@@ -222,34 +242,51 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
{
struct opp_table *opp_table;
struct dev_pm_opp *opp;
- struct regulator *reg;
+ struct regulator *reg, **regulators;
unsigned long latency_ns = 0;
- unsigned long min_uV = ~0, max_uV = 0;
- int ret;
+ int ret, i, count;
+ struct {
+ unsigned long min;
+ unsigned long max;
+ } *uV;
+
+ count = _get_regulator_count(dev);
+
+ /* Regulator may not be required for the device */
+ if (!count)
+ return 0;
+
+ regulators = kmalloc_array(count, sizeof(*regulators), GFP_KERNEL);
+ if (!regulators)
+ return 0;
+
+ uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
+ if (!uV)
+ goto free_regulators;
rcu_read_lock();
opp_table = _find_opp_table(dev);
if (IS_ERR(opp_table)) {
rcu_read_unlock();
- return 0;
+ goto free_uV;
}
- reg = opp_table->regulator;
- if (IS_ERR(reg)) {
- /* Regulator may not be required for device */
- rcu_read_unlock();
- return 0;
- }
+ memcpy(regulators, opp_table->regulators, count * sizeof(*regulators));
- list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
- if (!opp->available)
- continue;
+ for (i = 0; i < count; i++) {
+ uV[i].min = ~0;
+ uV[i].max = 0;
- if (opp->supply.u_volt_min < min_uV)
- min_uV = opp->supply.u_volt_min;
- if (opp->supply.u_volt_max > max_uV)
- max_uV = opp->supply.u_volt_max;
+ list_for_each_entry_rcu(opp, &opp_table->opp_list, node) {
+ if (!opp->available)
+ continue;
+
+ if (opp->supplies[i].u_volt_min < uV[i].min)
+ uV[i].min = opp->supplies[i].u_volt_min;
+ if (opp->supplies[i].u_volt_max > uV[i].max)
+ uV[i].max = opp->supplies[i].u_volt_max;
+ }
}
rcu_read_unlock();
@@ -258,9 +295,16 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
* The caller needs to ensure that opp_table (and hence the regulator)
* isn't freed, while we are executing this routine.
*/
- ret = regulator_set_voltage_time(reg, min_uV, max_uV);
- if (ret > 0)
- latency_ns = ret * 1000;
+ for (i = 0; reg = regulators[i], i < count; i++) {
+ ret = regulator_set_voltage_time(reg, uV[i].min, uV[i].max);
+ if (ret > 0)
+ latency_ns += ret * 1000;
+ }
+
+free_uV:
+ kfree(uV);
+free_regulators:
+ kfree(regulators);
return latency_ns;
}
@@ -580,7 +624,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
struct dev_pm_opp *old_opp, *opp;
- struct regulator *reg;
+ struct regulator *reg = ERR_PTR(-ENXIO);
struct clk *clk;
unsigned long freq, old_freq;
struct dev_pm_opp_supply old_supply, new_supply;
@@ -633,14 +677,23 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
+ if (opp_table->regulators) {
+ /* This function only supports single regulator per device */
+ if (WARN_ON(opp_table->regulator_count > 1)) {
+ dev_err(dev, "multiple regulators not supported\n");
+ rcu_read_unlock();
+ return -EINVAL;
+ }
+
+ reg = opp_table->regulators[0];
+ }
+
if (IS_ERR(old_opp))
old_supply.u_volt = 0;
else
- memcpy(&old_supply, &old_opp->supply, sizeof(old_supply));
-
- memcpy(&new_supply, &opp->supply, sizeof(new_supply));
+ memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
- reg = opp_table->regulator;
+ memcpy(&new_supply, opp->supplies, sizeof(new_supply));
rcu_read_unlock();
@@ -764,9 +817,6 @@ static struct opp_table *_add_opp_table(struct device *dev)
_of_init_opp_table(opp_table, dev);
- /* Set regulator to a non-NULL error value */
- opp_table->regulator = ERR_PTR(-ENXIO);
-
/* Find clk for the device */
opp_table->clk = clk_get(dev, NULL);
if (IS_ERR(opp_table->clk)) {
@@ -815,7 +865,7 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->prop_name)
return;
- if (!IS_ERR(opp_table->regulator))
+ if (opp_table->regulators)
return;
/* Release clk */
@@ -924,35 +974,50 @@ struct dev_pm_opp *_allocate_opp(struct device *dev,
struct opp_table **opp_table)
{
struct dev_pm_opp *opp;
+ int count, supply_size;
+ struct opp_table *table;
- /* allocate new OPP node */
- opp = kzalloc(sizeof(*opp), GFP_KERNEL);
- if (!opp)
+ table = _add_opp_table(dev);
+ if (!table)
return NULL;
- INIT_LIST_HEAD(&opp->node);
+ /* Allocate space for at least one supply */
+ count = table->regulator_count ? table->regulator_count : 1;
+ supply_size = sizeof(*opp->supplies) * count;
- *opp_table = _add_opp_table(dev);
- if (!*opp_table) {
- kfree(opp);
+ /* allocate new OPP node and supplies structures */
+ opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+ if (!opp) {
+ kfree(table);
return NULL;
}
+ /* Put the supplies at the end of the OPP structure as an empty array */
+ opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+ INIT_LIST_HEAD(&opp->node);
+
+ *opp_table = table;
+
return opp;
}
static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
struct opp_table *opp_table)
{
- struct regulator *reg = opp_table->regulator;
-
- if (!IS_ERR(reg) &&
- !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
- opp->supply.u_volt_max)) {
- pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
- __func__, opp->supply.u_volt_min,
- opp->supply.u_volt_max);
- return false;
+ struct regulator *reg;
+ int i;
+
+ for (i = 0; i < opp_table->regulator_count; i++) {
+ reg = opp_table->regulators[i];
+
+ if (!regulator_is_supported_voltage(reg,
+ opp->supplies[i].u_volt_min,
+ opp->supplies[i].u_volt_max)) {
+ pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator\n",
+ __func__, opp->supplies[i].u_volt_min,
+ opp->supplies[i].u_volt_max);
+ return false;
+ }
}
return true;
@@ -984,12 +1049,13 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
- __func__, opp->rate, opp->supply.u_volt,
- opp->available, new_opp->rate, new_opp->supply.u_volt,
- new_opp->available);
+ __func__, opp->rate, opp->supplies[0].u_volt,
+ opp->available, new_opp->rate,
+ new_opp->supplies[0].u_volt, new_opp->available);
+ /* Should we compare voltages for all regulators here ? */
return opp->available &&
- new_opp->supply.u_volt == opp->supply.u_volt ? 0 : -EEXIST;
+ new_opp->supplies[0].u_volt == opp->supplies[0].u_volt ? 0 : -EEXIST;
}
new_opp->opp_table = opp_table;
@@ -1056,9 +1122,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
- new_opp->supply.u_volt = u_volt;
- new_opp->supply.u_volt_min = u_volt - tol;
- new_opp->supply.u_volt_max = u_volt + tol;
+ new_opp->supplies[0].u_volt = u_volt;
+ new_opp->supplies[0].u_volt_min = u_volt - tol;
+ new_opp->supplies[0].u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
@@ -1303,12 +1369,14 @@ void dev_pm_opp_put_prop_name(struct device *dev)
EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
/**
- * dev_pm_opp_set_regulator() - Set regulator name for the device
+ * dev_pm_opp_set_regulators() - Set regulator names for the device
* @dev: Device for which regulator name is being set.
- * @name: Name of the regulator.
+ * @names: Array of pointers to the names of the regulator.
+ * @count: Number of regulators.
*
* In order to support OPP switching, OPP layer needs to know the name of the
- * device's regulator, as the core would be required to switch voltages as well.
+ * device's regulators, as the core would be required to switch voltages as
+ * well.
*
* This must be called before any OPPs are initialized for the device.
*
@@ -1318,11 +1386,13 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
* that this function is *NOT* called under RCU protection or in contexts where
* mutex cannot be locked.
*/
-struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name)
+struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
+ const char * const names[],
+ unsigned int count)
{
struct opp_table *opp_table;
struct regulator *reg;
- int ret;
+ int ret, i;
mutex_lock(&opp_table_lock);
@@ -1338,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)) {
ret = -EBUSY;
goto err;
}
- /* Allocate the regulator */
- reg = regulator_get_optional(dev, name);
- if (IS_ERR(reg)) {
- ret = PTR_ERR(reg);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "%s: no regulator (%s) found: %d\n",
- __func__, name, ret);
+
+ opp_table->regulators = kmalloc_array(count,
+ sizeof(*opp_table->regulators),
+ GFP_KERNEL);
+ if (!opp_table->regulators) {
+ ret = -ENOMEM;
goto err;
}
- opp_table->regulator = reg;
+ for (i = 0; i < count; i++) {
+ reg = regulator_get_optional(dev, names[i]);
+ if (IS_ERR(reg)) {
+ ret = PTR_ERR(reg);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "%s: no regulator (%s) found: %d\n",
+ __func__, names[i], ret);
+ 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:
@@ -1365,11 +1453,11 @@ struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name)
return ERR_PTR(ret);
}
-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
- * @opp_table: OPP table returned from dev_pm_opp_set_regulator().
+ * dev_pm_opp_put_regulators() - Releases resources blocked for regulator
+ * @opp_table: OPP table returned from dev_pm_opp_set_regulators().
*
* Locking: The internal opp_table and opp structures are RCU protected.
* Hence this function internally uses RCU updater strategy with mutex locks
@@ -1377,20 +1465,26 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
* that this function is *NOT* called under RCU protection or in contexts where
* mutex cannot be locked.
*/
-void dev_pm_opp_put_regulator(struct opp_table *opp_table)
+void dev_pm_opp_put_regulators(struct opp_table *opp_table)
{
+ int i;
+
mutex_lock(&opp_table_lock);
- if (IS_ERR(opp_table->regulator)) {
- pr_err("%s: Doesn't have regulator set\n", __func__);
+ if (!opp_table->regulators) {
+ pr_err("%s: Doesn't have regulators set\n", __func__);
goto unlock;
}
/* Make sure there are no concurrent readers while updating opp_table */
WARN_ON(!list_empty(&opp_table->opp_list));
- regulator_put(opp_table->regulator);
- opp_table->regulator = ERR_PTR(-ENXIO);
+ for (i = opp_table->regulator_count - 1; i >= 0; i--)
+ regulator_put(opp_table->regulators[i]);
+
+ kfree(opp_table->regulators);
+ opp_table->regulators = NULL;
+ opp_table->regulator_count = 0;
/* Try freeing opp_table if this was the last blocking resource */
_remove_opp_table(opp_table);
@@ -1398,7 +1492,7 @@ void dev_pm_opp_put_regulator(struct opp_table *opp_table)
unlock:
mutex_unlock(&opp_table_lock);
}
-EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulator);
+EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
/**
* dev_pm_opp_add() - Add an OPP table from a table definitions
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
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/limits.h>
+#include <linux/slab.h>
#include "opp.h"
@@ -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);
+
+ /* Create per-opp directory */
+ d = debugfs_create_dir(name, pdentry);
+
+ kfree(name);
+
+ if (!d)
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d,
+ &opp->supplies[i].u_volt))
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d,
+ &opp->supplies[i].u_volt_min))
+ return false;
+
+ if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d,
+ &opp->supplies[i].u_volt_max))
+ return false;
+
+ if (!debugfs_create_ulong("u_amp", S_IRUGO, d,
+ &opp->supplies[i].u_amp))
+ return false;
+ } while (++i < opp_table->regulator_count);
+
+ return true;
+}
+
int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
{
struct dentry *pdentry = opp_table->dentry;
@@ -63,16 +104,7 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)
if (!debugfs_create_ulong("rate_hz", S_IRUGO, d, &opp->rate))
return -ENOMEM;
- if (!debugfs_create_ulong("u_volt_target", S_IRUGO, d, &opp->supply.u_volt))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_volt_min", S_IRUGO, d, &opp->supply.u_volt_min))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_volt_max", S_IRUGO, d, &opp->supply.u_volt_max))
- return -ENOMEM;
-
- if (!debugfs_create_ulong("u_amp", S_IRUGO, d, &opp->supply.u_amp))
+ if (!opp_debug_create_supplies(opp, opp_table, d))
return -ENOMEM;
if (!debugfs_create_ulong("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/of.c b/drivers/base/power/opp/of.c
index bdf409d42126..3f7d2591b173 100644
--- a/drivers/base/power/opp/of.c
+++ b/drivers/base/power/opp/of.c
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/device.h>
#include <linux/of.h>
+#include <linux/slab.h>
#include <linux/export.h>
#include "opp.h"
@@ -101,16 +102,16 @@ static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
return true;
}
-/* TODO: Support multiple regulators */
static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
struct opp_table *opp_table)
{
- u32 microvolt[3] = {0};
- u32 val;
- int count, ret;
+ u32 *microvolt, *microamp = NULL;
+ int supplies, vcount, icount, ret, i, j;
struct property *prop = NULL;
char name[NAME_MAX];
+ supplies = opp_table->regulator_count ? opp_table->regulator_count : 1;
+
/* Search for "opp-microvolt-<name>" */
if (opp_table->prop_name) {
snprintf(name, sizeof(name), "opp-microvolt-%s",
@@ -128,34 +129,29 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
return 0;
}
- count = of_property_count_u32_elems(opp->np, name);
- if (count < 0) {
+ vcount = of_property_count_u32_elems(opp->np, name);
+ if (vcount < 0) {
dev_err(dev, "%s: Invalid %s property (%d)\n",
- __func__, name, count);
- return count;
+ __func__, name, vcount);
+ return vcount;
}
- /* There can be one or three elements here */
- if (count != 1 && count != 3) {
- dev_err(dev, "%s: Invalid number of elements in %s property (%d)\n",
- __func__, name, count);
+ /* There can be one or three elements per supply */
+ if (vcount != supplies && vcount != supplies * 3) {
+ dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+ __func__, name, vcount, supplies);
return -EINVAL;
}
- ret = of_property_read_u32_array(opp->np, name, microvolt, count);
+ microvolt = kmalloc_array(vcount, sizeof(*microvolt), GFP_KERNEL);
+ if (!microvolt)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(opp->np, name, microvolt, vcount);
if (ret) {
dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret);
- return -EINVAL;
- }
-
- opp->supply.u_volt = microvolt[0];
-
- if (count == 1) {
- opp->supply.u_volt_min = opp->supply.u_volt;
- opp->supply.u_volt_max = opp->supply.u_volt;
- } else {
- opp->supply.u_volt_min = microvolt[1];
- opp->supply.u_volt_max = microvolt[2];
+ ret = -EINVAL;
+ goto free_microvolt;
}
/* Search for "opp-microamp-<name>" */
@@ -172,10 +168,59 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
prop = of_find_property(opp->np, name, NULL);
}
- if (prop && !of_property_read_u32(opp->np, name, &val))
- opp->supply.u_amp = val;
+ if (prop) {
+ icount = of_property_count_u32_elems(opp->np, name);
+ if (icount < 0) {
+ dev_err(dev, "%s: Invalid %s property (%d)\n", __func__,
+ name, icount);
+ ret = icount;
+ goto free_microvolt;
+ }
- return 0;
+ if (icount != supplies) {
+ dev_err(dev, "%s: Invalid number of elements in %s property (%d) with supplies (%d)\n",
+ __func__, name, icount, supplies);
+ ret = -EINVAL;
+ goto free_microvolt;
+ }
+
+ microamp = kmalloc_array(icount, sizeof(*microamp), GFP_KERNEL);
+ if (!microamp) {
+ ret = -EINVAL;
+ goto free_microvolt;
+ }
+
+ ret = of_property_read_u32_array(opp->np, name, microamp,
+ icount);
+ if (ret) {
+ dev_err(dev, "%s: error parsing %s: %d\n", __func__,
+ name, ret);
+ ret = -EINVAL;
+ goto free_microamp;
+ }
+ }
+
+ for (i = 0, j = 0; i < supplies; i++) {
+ opp->supplies[i].u_volt = microvolt[j++];
+
+ if (vcount == supplies) {
+ opp->supplies[i].u_volt_min = opp->supplies[i].u_volt;
+ opp->supplies[i].u_volt_max = opp->supplies[i].u_volt;
+ } else {
+ opp->supplies[i].u_volt_min = microvolt[j++];
+ opp->supplies[i].u_volt_max = microvolt[j++];
+ }
+
+ if (microamp)
+ opp->supplies[i].u_amp = microamp[i];
+ }
+
+free_microamp:
+ kfree(microamp);
+free_microvolt:
+ kfree(microvolt);
+
+ return ret;
}
/**
@@ -304,8 +349,8 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
__func__, new_opp->turbo, new_opp->rate,
- new_opp->supply.u_volt, new_opp->supply.u_volt_min,
- new_opp->supply.u_volt_max, new_opp->clock_latency_ns);
+ new_opp->supplies[0].u_volt, new_opp->supplies[0].u_volt_min,
+ new_opp->supplies[0].u_volt_max, new_opp->clock_latency_ns);
/*
* Notify the changes in the availability of the operable
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 8a02516542c2..5b0f7e53bede 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -61,7 +61,7 @@ extern struct list_head opp_tables;
* @turbo: true if turbo (boost) OPP
* @suspend: true if suspend OPP
* @rate: Frequency in hertz
- * @supply: Power supply voltage/current values
+ * @supplies: Power supplies voltage/current values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
* frequency from any other OPP's frequency.
* @opp_table: points back to the opp_table struct this opp belongs to
@@ -80,7 +80,7 @@ struct dev_pm_opp {
bool suspend;
unsigned long rate;
- struct dev_pm_opp_supply supply;
+ struct dev_pm_opp_supply *supplies;
unsigned long clock_latency_ns;
@@ -139,7 +139,8 @@ enum opp_table_access {
* @supported_hw_count: Number of elements in supported_hw array.
* @prop_name: A name to postfix to many DT properties, while parsing them.
* @clk: Device's clock handle
- * @regulator: Supply regulator
+ * @regulators: Supply regulators
+ * @regulator_count: Number of power supply regulators
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
*
@@ -174,7 +175,8 @@ struct opp_table {
unsigned int supported_hw_count;
const char *prop_name;
struct clk *clk;
- struct regulator *regulator;
+ struct regulator **regulators;
+ unsigned int regulator_count;
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 4d3ec92cbabf..269013311e79 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -188,7 +188,7 @@ 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);
+ 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",
@@ -289,7 +289,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
out_free_opp:
dev_pm_opp_of_cpumask_remove_table(policy->cpus);
if (name)
- dev_pm_opp_put_regulator(opp_table);
+ dev_pm_opp_put_regulators(opp_table);
out_put_clk:
clk_put(cpu_clk);
@@ -304,7 +304,7 @@ static int cpufreq_exit(struct cpufreq_policy *policy)
dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
if (priv->reg_name)
- dev_pm_opp_put_regulator(priv->opp_table);
+ dev_pm_opp_put_regulators(priv->opp_table);
clk_put(policy->clk);
kfree(priv);
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 824f7268f687..9a825ae78653 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -79,8 +79,8 @@ int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
void dev_pm_opp_put_supported_hw(struct device *dev);
int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct device *dev);
-struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name);
-void dev_pm_opp_put_regulator(struct opp_table *opp_table);
+struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
+void dev_pm_opp_put_regulators(struct opp_table *opp_table);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -187,12 +187,12 @@ static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
static inline void dev_pm_opp_put_prop_name(struct device *dev) {}
-static inline struct opp_table *dev_pm_opp_set_regulator(struct device *dev, const char *name)
+static inline struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count)
{
return ERR_PTR(-ENOTSUPP);
}
-static inline void dev_pm_opp_put_regulator(struct opp_table *opp_table) {}
+static inline void dev_pm_opp_put_regulators(struct opp_table *opp_table) {}
static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V7 07/10] PM / OPP: Separate out _generic_set_opp()
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Later patches would add support for custom set_opp() callbacks. This
patch separates out the code for _generic_set_opp() handler in order to
prepare for that.
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Tested-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
drivers/base/power/opp/core.c | 181 +++++++++++++++++++++++++++++-------------
drivers/base/power/opp/opp.h | 3 +
include/linux/pm_opp.h | 35 ++++++++
3 files changed, 166 insertions(+), 53 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index b4da31c5a5eb..e33198ce41b4 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -610,6 +610,69 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
return ret;
}
+static inline int
+_generic_set_opp_clk_only(struct device *dev, struct clk *clk,
+ unsigned long old_freq, unsigned long freq)
+{
+ int ret;
+
+ ret = clk_set_rate(clk, freq);
+ if (ret) {
+ dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
+ ret);
+ }
+
+ return ret;
+}
+
+static int _generic_set_opp(struct dev_pm_set_opp_data *data)
+{
+ struct dev_pm_opp_supply *old_supply = data->old_opp.supplies;
+ struct dev_pm_opp_supply *new_supply = data->new_opp.supplies;
+ unsigned long old_freq = data->old_opp.rate, freq = data->new_opp.rate;
+ struct regulator *reg = data->regulators[0];
+ struct device *dev= data->dev;
+ int ret;
+
+ /* This function only supports single regulator per device */
+ if (WARN_ON(data->regulator_count > 1)) {
+ dev_err(dev, "multiple regulators are not supported\n");
+ return -EINVAL;
+ }
+
+ /* Scaling up? Scale voltage before frequency */
+ if (freq > old_freq) {
+ ret = _set_opp_voltage(dev, reg, new_supply);
+ if (ret)
+ goto restore_voltage;
+ }
+
+ /* Change frequency */
+ ret = _generic_set_opp_clk_only(dev, data->clk, old_freq, freq);
+ if (ret)
+ goto restore_voltage;
+
+ /* Scaling down? Scale voltage after frequency */
+ if (freq < old_freq) {
+ ret = _set_opp_voltage(dev, reg, new_supply);
+ if (ret)
+ goto restore_freq;
+ }
+
+ return 0;
+
+restore_freq:
+ if (_generic_set_opp_clk_only(dev, data->clk, freq, old_freq))
+ dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+ __func__, old_freq);
+restore_voltage:
+ /* This shouldn't harm even if the voltages weren't updated earlier */
+ if (old_supply->u_volt)
+ _set_opp_voltage(dev, reg, old_supply);
+
+ return ret;
+}
+
/**
* dev_pm_opp_set_rate() - Configure new OPP based on frequency
* @dev: device for which we do this operation
@@ -623,12 +686,12 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
+ unsigned long freq, old_freq;
struct dev_pm_opp *old_opp, *opp;
- struct regulator *reg = ERR_PTR(-ENXIO);
+ struct regulator **regulators;
+ struct dev_pm_set_opp_data *data;
struct clk *clk;
- unsigned long freq, old_freq;
- struct dev_pm_opp_supply old_supply, new_supply;
- int ret;
+ int ret, size;
if (unlikely(!target_freq)) {
dev_err(dev, "%s: Invalid target frequency %lu\n", __func__,
@@ -677,64 +740,36 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return ret;
}
- if (opp_table->regulators) {
- /* This function only supports single regulator per device */
- if (WARN_ON(opp_table->regulator_count > 1)) {
- dev_err(dev, "multiple regulators not supported\n");
- rcu_read_unlock();
- return -EINVAL;
- }
+ dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", __func__,
+ old_freq, freq);
- reg = opp_table->regulators[0];
+ regulators = opp_table->regulators;
+
+ /* Only frequency scaling */
+ if (!regulators) {
+ rcu_read_unlock();
+ return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
+ data = opp_table->set_opp_data;
+ data->regulators = regulators;
+ data->regulator_count = opp_table->regulator_count;
+ data->clk = clk;
+ data->dev = dev;
+
+ data->old_opp.rate = old_freq;
+ size = sizeof(*opp->supplies) * opp_table->regulator_count;
if (IS_ERR(old_opp))
- old_supply.u_volt = 0;
+ memset(data->old_opp.supplies, 0, size);
else
- memcpy(&old_supply, old_opp->supplies, sizeof(old_supply));
+ memcpy(data->old_opp.supplies, old_opp->supplies, size);
- memcpy(&new_supply, opp->supplies, sizeof(new_supply));
+ data->new_opp.rate = freq;
+ memcpy(data->new_opp.supplies, opp->supplies, size);
rcu_read_unlock();
- /* Scaling up? Scale voltage before frequency */
- if (freq > old_freq) {
- ret = _set_opp_voltage(dev, reg, &new_supply);
- if (ret)
- goto restore_voltage;
- }
-
- /* Change frequency */
-
- dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",
- __func__, old_freq, freq);
-
- ret = clk_set_rate(clk, freq);
- if (ret) {
- dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
- ret);
- goto restore_voltage;
- }
-
- /* Scaling down? Scale voltage after frequency */
- if (freq < old_freq) {
- ret = _set_opp_voltage(dev, reg, &new_supply);
- if (ret)
- goto restore_freq;
- }
-
- return 0;
-
-restore_freq:
- if (clk_set_rate(clk, old_freq))
- dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
- __func__, old_freq);
-restore_voltage:
- /* This shouldn't harm even if the voltages weren't updated earlier */
- if (old_supply.u_volt)
- _set_opp_voltage(dev, reg, &old_supply);
-
- return ret;
+ return _generic_set_opp(data);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
@@ -1368,6 +1403,38 @@ void dev_pm_opp_put_prop_name(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name);
+static int _allocate_set_opp_data(struct opp_table *opp_table)
+{
+ struct dev_pm_set_opp_data *data;
+ int len, count = opp_table->regulator_count;
+
+ if (WARN_ON(!count))
+ return -EINVAL;
+
+ /* space for set_opp_data */
+ len = sizeof(*data);
+
+ /* space for old_opp.supplies and new_opp.supplies */
+ len += 2 * sizeof(struct dev_pm_opp_supply) * count;
+
+ data = kzalloc(len, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->old_opp.supplies = (void *)(data + 1);
+ data->new_opp.supplies = data->old_opp.supplies + count;
+
+ opp_table->set_opp_data = data;
+
+ return 0;
+}
+
+static void _free_set_opp_data(struct opp_table *opp_table)
+{
+ kfree(opp_table->set_opp_data);
+ opp_table->set_opp_data = NULL;
+}
+
/**
* dev_pm_opp_set_regulators() - Set regulator names for the device
* @dev: Device for which regulator name is being set.
@@ -1437,6 +1504,11 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
opp_table->regulator_count = count;
+ /* Allocate block only once to pass to set_opp() routines */
+ ret = _allocate_set_opp_data(opp_table);
+ if (ret)
+ goto free_regulators;
+
mutex_unlock(&opp_table_lock);
return opp_table;
@@ -1446,6 +1518,7 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
kfree(opp_table->regulators);
opp_table->regulators = NULL;
+ opp_table->regulator_count = 0;
err:
_remove_opp_table(opp_table);
unlock:
@@ -1482,6 +1555,8 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
for (i = opp_table->regulator_count - 1; i >= 0; i--)
regulator_put(opp_table->regulators[i]);
+ _free_set_opp_data(opp_table);
+
kfree(opp_table->regulators);
opp_table->regulators = NULL;
opp_table->regulator_count = 0;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 5b0f7e53bede..a05e43912c6b 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
* @clk: Device's clock handle
* @regulators: Supply regulators
* @regulator_count: Number of power supply regulators
+ * @set_opp_data: Data to be passed to set_opp callback
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
*
@@ -178,6 +179,8 @@ struct opp_table {
struct regulator **regulators;
unsigned int regulator_count;
+ struct dev_pm_set_opp_data *set_opp_data;
+
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
char dentry_name[NAME_MAX];
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 9a825ae78653..779b40a9287d 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -17,6 +17,8 @@
#include <linux/err.h>
#include <linux/notifier.h>
+struct clk;
+struct regulator;
struct dev_pm_opp;
struct device;
struct opp_table;
@@ -41,6 +43,39 @@ struct dev_pm_opp_supply {
unsigned long u_amp;
};
+/**
+ * struct dev_pm_opp_info - OPP freq/voltage/current values
+ * @rate: Target clk rate in hz
+ * @supplies: Array of voltage/current values for all power supplies
+ *
+ * This structure stores the freq/voltage/current values for a single OPP.
+ */
+struct dev_pm_opp_info {
+ unsigned long rate;
+ struct dev_pm_opp_supply *supplies;
+};
+
+/**
+ * struct dev_pm_set_opp_data - Set OPP data
+ * @old_opp: Old OPP info
+ * @new_opp: New OPP info
+ * @regulators: Array of regulator pointers
+ * @regulator_count: Number of regulators
+ * @clk: Pointer to clk
+ * @dev: Pointer to the struct device
+ *
+ * This structure contains all information required for setting an OPP.
+ */
+struct dev_pm_set_opp_data {
+ struct dev_pm_opp_info old_opp;
+ struct dev_pm_opp_info new_opp;
+
+ struct regulator **regulators;
+ unsigned int regulator_count;
+ struct clk *clk;
+ struct device *dev;
+};
+
#if defined(CONFIG_PM_OPP)
unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp);
--
2.7.1.410.g6faf27b
--
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 related
* [PATCH V7 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Viresh Kumar @ 2016-12-01 10:58 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, Vincent Guittot, robh, d-gerlach,
broonie, devicetree, Viresh Kumar
In-Reply-To: <cover.1480564564.git.viresh.kumar@linaro.org>
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>
---
drivers/base/power/opp/core.c | 114 +++++++++++++++++++++++++++++++++++++++++-
drivers/base/power/opp/opp.h | 2 +
include/linux/pm_opp.h | 10 ++++
3 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index e33198ce41b4..eceebef36f21 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -687,6 +687,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
{
struct opp_table *opp_table;
unsigned long freq, old_freq;
+ int (*set_opp)(struct dev_pm_set_opp_data *data);
struct dev_pm_opp *old_opp, *opp;
struct regulator **regulators;
struct dev_pm_set_opp_data *data;
@@ -751,6 +752,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
+ if (opp_table->set_opp)
+ set_opp = opp_table->set_opp;
+ else
+ set_opp = _generic_set_opp;
+
data = opp_table->set_opp_data;
data->regulators = regulators;
data->regulator_count = opp_table->regulator_count;
@@ -769,7 +775,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
rcu_read_unlock();
- return _generic_set_opp(data);
+ return set_opp(data);
}
EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
@@ -903,6 +909,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->regulators)
return;
+ if (opp_table->set_opp)
+ return;
+
/* Release clk */
if (!IS_ERR(opp_table->clk))
clk_put(opp_table->clk);
@@ -1570,6 +1579,109 @@ void dev_pm_opp_put_regulators(struct opp_table *opp_table)
EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
/**
+ * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
+ * @dev: Device for which the helper is getting registered.
+ * @set_opp: Custom set OPP helper.
+ *
+ * This is useful to support complex platforms (like platforms with multiple
+ * regulators per device), instead of the generic OPP set rate helper.
+ *
+ * This must be called before any OPPs are initialized for the device.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_register_set_opp_helper(struct device *dev,
+ int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+ struct opp_table *opp_table;
+ int ret;
+
+ if (!set_opp)
+ return -EINVAL;
+
+ mutex_lock(&opp_table_lock);
+
+ opp_table = _add_opp_table(dev);
+ if (!opp_table) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ /* This should be called before OPPs are initialized */
+ if (WARN_ON(!list_empty(&opp_table->opp_list))) {
+ ret = -EBUSY;
+ goto err;
+ }
+
+ /* Already have custom set_opp helper */
+ if (WARN_ON(opp_table->set_opp)) {
+ ret = -EBUSY;
+ goto err;
+ }
+
+ opp_table->set_opp = set_opp;
+
+ mutex_unlock(&opp_table_lock);
+ return 0;
+
+err:
+ _remove_opp_table(opp_table);
+unlock:
+ mutex_unlock(&opp_table_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_set_opp_helper);
+
+/**
+ * dev_pm_opp_register_put_opp_helper() - Releases resources blocked for
+ * set_opp helper
+ * @dev: Device for which custom set_opp helper has to be cleared.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_register_put_opp_helper(struct device *dev)
+{
+ struct opp_table *opp_table;
+
+ mutex_lock(&opp_table_lock);
+
+ /* Check for existing table for 'dev' first */
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ dev_err(dev, "Failed to find opp_table: %ld\n",
+ PTR_ERR(opp_table));
+ goto unlock;
+ }
+
+ if (!opp_table->set_opp) {
+ dev_err(dev, "%s: Doesn't have custom set_opp helper set\n",
+ __func__);
+ goto unlock;
+ }
+
+ /* Make sure there are no concurrent readers while updating opp_table */
+ WARN_ON(!list_empty(&opp_table->opp_list));
+
+ opp_table->set_opp = NULL;
+
+ /* Try freeing opp_table if this was the last blocking resource */
+ _remove_opp_table(opp_table);
+
+unlock:
+ mutex_unlock(&opp_table_lock);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_put_opp_helper);
+
+/**
* dev_pm_opp_add() - Add an OPP table from a table definitions
* @dev: device for which we do this operation
* @freq: Frequency in Hz for this OPP
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index a05e43912c6b..af9f2b849a66 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -141,6 +141,7 @@ enum opp_table_access {
* @clk: Device's clock handle
* @regulators: Supply regulators
* @regulator_count: Number of power supply regulators
+ * @set_opp: Platform specific set_opp callback
* @set_opp_data: Data to be passed to set_opp callback
* @dentry: debugfs dentry pointer of the real device directory (not links).
* @dentry_name: Name of the real dentry.
@@ -179,6 +180,7 @@ struct opp_table {
struct regulator **regulators;
unsigned int regulator_count;
+ int (*set_opp)(struct dev_pm_set_opp_data *data);
struct dev_pm_set_opp_data *set_opp_data;
#ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 779b40a9287d..0edd88f93904 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -116,6 +116,8 @@ int dev_pm_opp_set_prop_name(struct device *dev, const char *name);
void dev_pm_opp_put_prop_name(struct device *dev);
struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * const names[], unsigned int count);
void dev_pm_opp_put_regulators(struct opp_table *opp_table);
+int dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
+void dev_pm_opp_register_put_opp_helper(struct device *dev);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
@@ -215,6 +217,14 @@ static inline int dev_pm_opp_set_supported_hw(struct device *dev,
static inline void dev_pm_opp_put_supported_hw(struct device *dev) {}
+static inline int dev_pm_opp_register_set_opp_helper(struct device *dev,
+ int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+ return -ENOTSUPP;
+}
+
+static inline void dev_pm_opp_register_put_opp_helper(struct device *dev) {}
+
static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name)
{
return -ENOTSUPP;
--
2.7.1.410.g6faf27b
^ permalink raw reply related
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