* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Kevin Hilman @ 2016-12-01 0:14 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <20161130225136.GO16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> Hi Kevin,
>
> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
>> Hi Sakari,
>>
>> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
>>
>> > On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
>> >> Allow getting of subdevs from DT ports and endpoints.
>> >>
>> >> The _get_pdata() function was larely inspired by (i.e. stolen from)
>> >
>> > vpif_capture_get_pdata and "largely"?
>>
>> Yes, thanks.
>>
>> >> am437x-vpfe.c
>> >>
>> >> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> >> ---
>> >> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
>> >> include/media/davinci/vpif_types.h | 9 +-
>> >> 2 files changed, 133 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> >> index 94ee6cf03f02..47a4699157e7 100644
>> >> --- a/drivers/media/platform/davinci/vpif_capture.c
>> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> >> @@ -26,6 +26,8 @@
>> >> #include <linux/slab.h>
>> >>
>> >> #include <media/v4l2-ioctl.h>
>> >> +#include <media/v4l2-of.h>
>> >> +#include <media/i2c/tvp514x.h>
>> >
>> > Do you need this header?
>> >
>>
>> Yes, based on discussion with Hans, since there is no DT binding for
>> selecting the input pins of the TVP514x, I have to select it in the
>> driver, so I need the defines from this header. More on this below...
>>
>> >>
>> >> #include "vpif.h"
>> >> #include "vpif_capture.h"
>> >> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
>> >>
>> >> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>> >>
>> >> + if (!chan_cfg)
>> >> + return -1;
>> >> + if (input_index >= chan_cfg->input_count)
>> >> + return -1;
>> >> subdev_name = chan_cfg->inputs[input_index].subdev_name;
>> >> if (subdev_name == NULL)
>> >> return -1;
>> >> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
>> >> /* loop through the sub device list to get the sub device info */
>> >> for (i = 0; i < vpif_cfg->subdev_count; i++) {
>> >> subdev_info = &vpif_cfg->subdev_info[i];
>> >> - if (!strcmp(subdev_info->name, subdev_name))
>> >> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>> >> return i;
>> >> }
>> >> return -1;
>> >> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
>> >> {
>> >> int i;
>> >>
>> >> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
>> >> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
>> >> + const struct device_node *node = _asd->match.of.node;
>> >> +
>> >> + if (node == subdev->of_node) {
>> >> + vpif_obj.sd[i] = subdev;
>> >> + vpif_obj.config->chan_config->inputs[i].subdev_name =
>> >> + (char *)subdev->of_node->full_name;
>> >> + vpif_dbg(2, debug,
>> >> + "%s: setting input %d subdev_name = %s\n",
>> >> + __func__, i, subdev->of_node->full_name);
>> >> + return 0;
>> >> + }
>> >> + }
>> >> +
>> >> for (i = 0; i < vpif_obj.config->subdev_count; i++)
>> >> if (!strcmp(vpif_obj.config->subdev_info[i].name,
>> >> subdev->name)) {
>> >> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
>> >> return vpif_probe_complete();
>> >> }
>> >>
>> >> +static struct vpif_capture_config *
>> >> +vpif_capture_get_pdata(struct platform_device *pdev)
>> >> +{
>> >> + struct device_node *endpoint = NULL;
>> >> + struct v4l2_of_endpoint bus_cfg;
>> >> + struct vpif_capture_config *pdata;
>> >> + struct vpif_subdev_info *sdinfo;
>> >> + struct vpif_capture_chan_config *chan;
>> >> + unsigned int i;
>> >> +
>> >> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
>> >> +
>> >> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> >> + return pdev->dev.platform_data;
>> >> +
>> >> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> >> + if (!pdata)
>> >> + return NULL;
>> >> + pdata->subdev_info =
>> >> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
>> >> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
>> >> +
>> >> + if (!pdata->subdev_info)
>> >> + return NULL;
>> >> + dev_dbg(&pdev->dev, "%s\n", __func__);
>> >> +
>> >> + for (i = 0; ; i++) {
>> >> + struct device_node *rem;
>> >> + unsigned int flags;
>> >> + int err;
>> >> +
>> >> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
>> >> + endpoint);
>> >> + if (!endpoint)
>> >> + break;
>> >> +
>> >> + sdinfo = &pdata->subdev_info[i];
>> >
>> > subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
>> >
>>
>> Right, I need to make the loop only go for a max of
>> VPIF_CAPTURE_MAX_CHANNELS iterations.
>>
>> >> + chan = &pdata->chan_config[i];
>> >> + chan->inputs = devm_kzalloc(&pdev->dev,
>> >> + sizeof(*chan->inputs) *
>> >> + VPIF_DISPLAY_MAX_CHANNELS,
>> >> + GFP_KERNEL);
>> >> +
>> >> + chan->input_count++;
>> >> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
>> >
>> > I wonder what's the purpose of using index i on this array as well.
>>
>> The number of endpoints in DT is the number of input channels configured
>> (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
>>
>> > If you use that to access a corresponding entry in a different array, I'd
>> > just create a struct that contains the port configuration and the async
>> > sub-device. The omap3isp driver does that, for instance; see
>> > isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
>> > interested. Up to you.
>>
>> OK, I'll have a look at that driver. The goal here with this series is
>> just to get this working with DT, but also not break the existing legacy
>> platform_device support, so I'm trying not to mess with the
>> driver-interal data structures too much.
>
> Ack.
>
>>
>> >> + chan->inputs[i].input.std = V4L2_STD_ALL;
>> >> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
>> >> +
>> >> + /* FIXME: need a new property? ch0:composite ch1: s-video */
>> >> + if (i == 0)
>> >
>> > Can you assume that the first endopoint has got a particular kind of input?
>> > What if it's not connected?
>>
>> On all the boards I know of (there aren't many using this SoC), it's a
>> safe assumption.
>>
>> > If this is a different physical port (not in the meaning another) in the
>> > device, I'd use the reg property for this. Please see
>> > Documentation/devicetree/bindings/media/video-interfaces.txt .
>>
>> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
>> that it's not physically a different port. Instead, it's just telling
>> the TVP514x which pin(s) will be active inputs (and what kind of signal
>> will be present.)
>>
>> I'm open to a better way to describe this input select from DT, but
>> based on what I heard from Hans, there isn't currently a good way to do
>> that except for in the driver:
>> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
>>
>> Based on further discussion in that thread, it sounds like there may be
>> a way forward coming soon, and I'll be glad to switch to that when it
>> arrives.
>
> I'm not sure that properly supporting connectors will provide any help here.
>
> Looking at the s_routing() API, it's the calling driver that has to be aware
> of sub-device specific function parameters. As such it's not a very good
> idea to require that a driver is aware of the value range of another
> driver's parameter. I wonder if a simple enumeration interface would help
> here --- if I understand correctly, the purpose is just to provide a way to
> choose the input using VIDIOC_S_INPUT.
>
> I guess that's somehow ok as long as you have no other combinations of these
> devices but this is hardly future-proof. (And certainly not a problem
> created by this patch.)
Yeah, this is far from future proof.
> It'd be still nice to fix that as presumably we don't have the option of
> reworking how we expect the device tree to look like.
Agreed.
I'm just hoping someone can shed som light on "how we expect the device
tree to look". ;)
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V6 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks
From: Viresh Kumar @ 2016-12-01 0:27 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rafael Wysocki, Viresh Kumar, Nishanth Menon,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
robh-DgEjT+Ai2ygdnm+yROfE0A, d-gerlach-l0cyMroinI0,
broonie-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161130220419.GL6095-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 30-11-16, 14:04, Stephen Boyd wrote:
> On 11/30, Viresh Kumar wrote:
> > The generic set_opp() handler isn't sufficient for platforms with
> > complex DVFS. For example, some TI platforms have multiple regulators
> > for a CPU device. The order in which various supplies need to be
> > programmed is only known to the platform code and its best to leave it
> > to it.
> >
> > This patch implements APIs to register platform specific set_opp()
> > callback.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar-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>
Thanks.
> So this one has the same set/put problem the other APIs has?
Yes.
> Presumably we're going to need to fix and change the API that is
> introduced here. Wouldn't it be better to do that first though?
Returning opp_table is one of the solutions and I am not sure if it is
the best one. I am working on the cleanup series and will modify all
the routine the same way very soon.
--
viresh
--
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 6/6] pinctrl: mt8173: set GPIO16 to usb iddig mode
From: Hongzhou Yang @ 2016-12-01 0:43 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, Maoguang Meng, Linus Walleij, Mathias Nyman,
Alan Stern, chunfeng yun, Yingjoe Chen, Felipe Balbi,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pawel Moll,
Ian Campbell, Biao Huang, Sascha Hauer,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
moderated list:ARM/Mediatek SoC support,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Sergei Shtylyov, Greg Kroah-Hartman <gregkh>
In-Reply-To: <42f5df18-2edd-08f2-a833-9c92ac85e87c-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, 2016-11-23 at 19:32 +0100, Matthias Brugger wrote:
> Hi Hongzhou,
>
> On 12/05/16 04:55, Hongzhou Yang wrote:
> > On Wed, 2016-05-11 at 19:09 -0700, Hongzhou Yang wrote:
> >> On Thu, 2016-05-12 at 09:41 +0800, chunfeng yun wrote:
> >>> Hi,
> >>>
> >>> On Wed, 2016-05-11 at 11:32 -0700, Hongzhou Yang wrote:
> >>>> On Wed, 2016-05-11 at 13:56 +0200, Linus Walleij wrote:
> >>>>> On Tue, May 10, 2016 at 10:23 AM, Chunfeng Yun
> >>>>> <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>>>>
> >>>>>> the default mode of GPIO16 pin is gpio, when set EINT16 to
> >>>>>> IRQ_TYPE_LEVEL_HIGH, no interrupt is triggered, it can be
> >>>>>> fixed when set its default mode as usb iddig.
> >>>>>>
> >>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>>
> >>>>
> >>>> Chunfeng, GPIO16 can be used as EINT16 mode, but the pinmux should be 0.
> >>>> If you want to set its default mode to iddig, you should set it in dts.
> >>>>
> >>> I set it in DTS, but it didn't work, because when usb driver requested
> >>> IRQ, pinmux was switched back to default mode set by
> >>> MTK_EINT_FUNCTION().
> >>>
> >>
> >> After confirmed, there are something wrong with data sheet and pinmux
> >> table, and GPIO16 can only receive interrupt by mode 1. So
> >>
> >> Acked-by: Hongzhou Yang <hongzhou.yang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>
> >
> > Linus,
> >
> > We find there are some other pins still have the same problem, so please
> > hold on it. Sorry for so much noise.
> >
>
> Did you made any progress on this? I didn't see any patch on the mailing
> list.
>
> Regards,
> Matthias
Hi Matthias,
Sorry for the late reply.
I have double confirmed with HW designer, other special EINTs are
built-in and they are using internal signal, they are not triggered
by GPIO, only GPIO16 should set to mode 1.
And, Chunfeng already re-sent this patch.
Thank you very much.
Yours,
Hongzhou
^ permalink raw reply
* [resend V2:PATCH 0/2] add reset-hi3660
From: Zhangfei Gao @ 2016-12-01 0:48 UTC (permalink / raw)
To: Rob Herring, Philipp Zabel, Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Zhangfei Gao
Considering Arnd and Philipp suggestions,
move reset register to dts as table instead of dts header in case of ABI issue
Zhangfei Gao (2):
dt-bindings: Document the hi3660 reset bindings
reset: hisilicon: add reset-hi3660
.../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++
drivers/reset/hisilicon/Kconfig | 7 +
drivers/reset/hisilicon/Makefile | 1 +
drivers/reset/hisilicon/reset-hi3660.c | 144 +++++++++++++++++++++
4 files changed, 203 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
create mode 100644 drivers/reset/hisilicon/reset-hi3660.c
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [resend v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Zhangfei Gao @ 2016-12-01 0:48 UTC (permalink / raw)
To: Rob Herring, Philipp Zabel, Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Zhangfei Gao
In-Reply-To: <1480553321-17400-1-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Add DT bindings documentation for hi3660 SoC reset controller.
Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../bindings/reset/hisilicon,hi3660-reset.txt | 51 ++++++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
new file mode 100644
index 0000000..250daf2
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
@@ -0,0 +1,51 @@
+Hisilicon System Reset Controller
+======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+The reset controller registers are part of the system-ctl block on
+hi3660 SoC.
+
+Required properties:
+- compatible: should be
+ "hisilicon,hi3660-reset"
+- #reset-cells: 1, see below
+- hisi,rst-syscon: phandle of the reset's syscon.
+- hisi,reset-bits: Contains the reset control register information
+ Should contain 2 cells for each reset exposed to
+ consumers, defined as:
+ Cell #1 : offset from the syscon register base
+ Cell #2 : bits position of the control register
+
+Example:
+ iomcu: iomcu@ffd7e000 {
+ compatible = "hisilicon,hi3660-iomcu", "syscon";
+ reg = <0x0 0xffd7e000 0x0 0x1000>;
+ };
+
+ iomcu_rst: iomcu_rst_controller {
+ compatible = "hisilicon,hi3660-reset";
+ #reset-cells = <1>;
+ hisi,rst-syscon = <&iomcu>;
+ hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
+ 0x20 0x10 /* 1: i2c1 */
+ 0x20 0x20 /* 2: i2c2 */
+ 0x20 0x8000000>; /* 3: i2c6 */
+ };
+
+Specifying reset lines connected to IP modules
+==============================================
+example:
+
+ i2c0: i2c@..... {
+ ...
+ resets = <&iomcu_rst 0>;
+ ...
+ };
+
+ i2c1: i2c@..... {
+ ...
+ resets = <&iomcu_rst 1>;
+ ...
+ };
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [resend V2: PATCH 2/2] reset: hisilicon: add reset-hi3660
From: Zhangfei Gao @ 2016-12-01 0:48 UTC (permalink / raw)
To: Rob Herring, Philipp Zabel, Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Zhangfei Gao
In-Reply-To: <1480553321-17400-1-git-send-email-zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Add hi3660 reset driver
Reset offset & bits should be listed in dts
Like:
iomcu_rst: iomcu_rst_controller {
compatible = "hisilicon,hi3660-reset";
#reset-cells = <1>;
hisi,rst-syscon = <&iomcu>;
hisi,reset-bits = <0x20 0x8 /* 0: i2c0 */
0x20 0x10 /* 1: i2c1 */
0x20 0x20 /* 2: i2c2 */
0x20 0x8000000>; /* 3: i2c6 */
};
Signed-off-by: Zhangfei Gao <zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/reset/hisilicon/Kconfig | 7 ++
drivers/reset/hisilicon/Makefile | 1 +
drivers/reset/hisilicon/reset-hi3660.c | 144 +++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
create mode 100644 drivers/reset/hisilicon/reset-hi3660.c
diff --git a/drivers/reset/hisilicon/Kconfig b/drivers/reset/hisilicon/Kconfig
index 1ff8b0c..10134dc 100644
--- a/drivers/reset/hisilicon/Kconfig
+++ b/drivers/reset/hisilicon/Kconfig
@@ -1,3 +1,10 @@
+config COMMON_RESET_HI3660
+ tristate "Hi3660 Reset Driver"
+ depends on ARCH_HISI || COMPILE_TEST
+ default ARCH_HISI
+ help
+ Build the Hisilicon Hi3660 reset driver.
+
config COMMON_RESET_HI6220
tristate "Hi6220 Reset Driver"
depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
index c932f86..ab8a7bf 100644
--- a/drivers/reset/hisilicon/Makefile
+++ b/drivers/reset/hisilicon/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
+obj-$(CONFIG_COMMON_RESET_HI3660) += reset-hi3660.o
diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c
new file mode 100644
index 0000000..3307252
--- /dev/null
+++ b/drivers/reset/hisilicon/reset-hi3660.c
@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+struct hi3660_reset_data {
+ unsigned int off;
+ unsigned int bits;
+};
+
+struct hi3660_reset_controller {
+ struct reset_controller_dev rst;
+ struct regmap *map;
+ const struct hi3660_reset_data *datas;
+};
+
+#define to_hi3660_reset_controller(_rst) \
+ container_of(_rst, struct hi3660_reset_controller, rst)
+
+static int hi3660_reset_program_hw(struct reset_controller_dev *rcdev,
+ unsigned long idx, bool assert)
+{
+ struct hi3660_reset_controller *rc = to_hi3660_reset_controller(rcdev);
+ const struct hi3660_reset_data *d;
+
+ if (idx >= rcdev->nr_resets)
+ return -EINVAL;
+
+ d = &rc->datas[idx];
+
+ if (assert)
+ return regmap_write(rc->map, d->off, d->bits);
+ else
+ return regmap_write(rc->map, d->off + 4, d->bits);
+}
+
+static int hi3660_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long idx)
+{
+ return hi3660_reset_program_hw(rcdev, idx, true);
+}
+
+static int hi3660_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long idx)
+{
+ return hi3660_reset_program_hw(rcdev, idx, false);
+}
+
+static int hi3660_reset_dev(struct reset_controller_dev *rcdev,
+ unsigned long idx)
+{
+ int err;
+
+ err = hi3660_reset_assert(rcdev, idx);
+ if (err)
+ return err;
+
+ return hi3660_reset_deassert(rcdev, idx);
+}
+
+static struct reset_control_ops hi3660_reset_ops = {
+ .reset = hi3660_reset_dev,
+ .assert = hi3660_reset_assert,
+ .deassert = hi3660_reset_deassert,
+};
+
+static int hi3660_reset_probe(struct platform_device *pdev)
+{
+ struct hi3660_reset_controller *rc;
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct hi3660_reset_data *datas;
+ const __be32 *list;
+ int size, nr, i;
+
+ rc = devm_kzalloc(dev, sizeof(*rc), GFP_KERNEL);
+ if (!rc)
+ return -ENOMEM;
+
+ rc->map = syscon_regmap_lookup_by_phandle(np, "hisi,rst-syscon");
+ if (IS_ERR(rc->map)) {
+ dev_err(dev, "failed to get hi3660,rst-syscon\n");
+ return PTR_ERR(rc->map);
+ }
+
+ list = of_get_property(np, "hisi,reset-bits", &size);
+ if (!list || (size / sizeof(*list)) % 2 != 0) {
+ dev_err(dev, "invalid DT reset description\n");
+ return -EINVAL;
+ }
+
+ nr = (size / sizeof(*list)) / 2;
+ datas = devm_kzalloc(dev, nr * sizeof(*datas), GFP_KERNEL);
+ if (!datas)
+ return -ENOMEM;
+
+ for (i = 0; i < nr; i++) {
+ datas[i].off = be32_to_cpup(list++);
+ datas[i].bits = be32_to_cpup(list++);
+ }
+
+ rc->rst.ops = &hi3660_reset_ops,
+ rc->rst.of_node = np;
+ rc->rst.nr_resets = nr;
+ rc->datas = datas;
+
+ return reset_controller_register(&rc->rst);
+}
+
+static const struct of_device_id hi3660_reset_match[] = {
+ { .compatible = "hisilicon,hi3660-reset", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, hi3660_reset_match);
+
+static struct platform_driver hi3660_reset_driver = {
+ .probe = hi3660_reset_probe,
+ .driver = {
+ .name = "hi3660-reset",
+ .of_match_table = hi3660_reset_match,
+ },
+};
+
+static int __init hi3660_reset_init(void)
+{
+ return platform_driver_register(&hi3660_reset_driver);
+}
+arch_initcall(hi3660_reset_init);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi3660-reset");
+MODULE_DESCRIPTION("HiSilicon Hi3660 Reset Driver");
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH V6 06/10] PM / OPP: Add infrastructure to manage multiple regulators
From: Viresh Kumar @ 2016-12-01 0:51 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rafael Wysocki, Viresh Kumar, Nishanth Menon, linaro-kernel,
linux-pm, Vincent Guittot, robh, d-gerlach, broonie, devicetree
In-Reply-To: <20161130223712.GM6095@codeaurora.org>
On 30-11-16, 14:37, Stephen Boyd wrote:
> > +static bool opp_debug_create_supplies(struct dev_pm_opp *opp,
> > + struct opp_table *opp_table,
> > + struct dentry *pdentry)
> > +{
> > + struct dentry *d;
> > + int i = 0;
> > + char *name;
> > +
> > + /* Always create at least supply-0 directory */
> > + do {
> > + name = kasprintf(GFP_KERNEL, "supply-%d", i);
>
> Sorry I haven't noticed this before. Wouldn't we want to put the
> supply name here instead of a number?
The OPP core provides a way for platforms to use the set-OPP
functionality, but that is not compulsory for its user. Which makes it
very much possible that the OPP core wouldn't know the names of the
supplies at all. And so I used the numbers here. Anyway, the numbers
matches the order in which they are passed from DT, so that should be
readable.
Here is the new rebased version of the patch. I will send the complete
series again later on. Please see if it looks fine.
-------------------------8<-------------------------
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 4 Oct 2016 17:07:28 +0530
Subject: [PATCH] PM / OPP: Add infrastructure to manage multiple regulators
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 | 9 +-
include/linux/pm_opp.h | 8 +-
6 files changed, 303 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..50669e0466c8 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -188,7 +188,10 @@ static int cpufreq_init(struct cpufreq_policy *policy)
*/
name = find_supply_name(cpu_dev);
if (name) {
- opp_table = dev_pm_opp_set_regulator(cpu_dev, name);
+ const char *names[] = {name};
+
+ opp_table = dev_pm_opp_set_regulators(cpu_dev, &name,
+ ARRAY_SIZE(names));
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 +292,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 +307,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)
{
^ permalink raw reply related
* Re: [PATCH 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
From: John Youn @ 2016-12-01 0:54 UTC (permalink / raw)
To: Felipe Balbi, Raviteja Garimella
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, devicetree,
linux-kernel, BCM Kernel Feedback, linux-usb, John Youn
In-Reply-To: <87k2blm8yx.fsf@linux.intel.com>
On 11/30/2016 4:47 AM, Felipe Balbi wrote:
>
> Hi,
>
> Raviteja Garimella <raviteja.garimella@broadcom.com> writes:
>> Hi Balbi,
>>
>> On Wed, Nov 30, 2016 at 4:10 PM, Felipe Balbi <balbi@kernel.org> wrote:
>>>
>>> Hi,
>>>
>>> Raviteja Garimella <raviteja.garimella@broadcom.com> writes:
>>>> This is driver for Synopsys Designware Cores USB Device
>>>> Controller (UDC) Subsystem with the AMBA Advanced High-Performance
>>>> Bus (AHB). This driver works with Synopsys UDC20 products.
>>>>
>>>> Signed-off-by: Raviteja Garimella <raviteja.garimella@broadcom.com>
>>>
>>> use drivers/usb/dwc2 instead of duplicating it.
>>
>> The ones we have in drivers/usb/dwc2 is for Designware high speed OTG
>> controller IP. The one that I submitted for review is for USB Device
>> controller IP (UDC). The IPs are different.
>
> I'll wait for John's confirmation that this really isn't compatible with
> dwc2. John?
>
Hi Felipe,
This is our older UDC IP, not compatible with HSOTG.
It is also no longer supported by Synopsys and considered EOL.
Regards,
John
^ permalink raw reply
* [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Brian Norris @ 2016-12-01 1:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Caesar Wang, linux-rockchip, Rob Herring, linux-input, devicetree,
linux-kernel, Dmitry Torokhov, Mark Rutland, Doug Anderson,
Brian Norris
From: Caesar Wang <wxt@rock-chips.com>
Add a compatible string and regulator property for Wacom W9103
digitizer. Its VDD supply may need to be enabled before using it.
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: linux-input@vger.kernel.org
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
v1 was a few months back. I finally got around to rewriting it based on
DT binding feedback.
v2:
* add compatible property for wacom
* name the regulator property specifically (VDD)
Documentation/devicetree/bindings/input/hid-over-i2c.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index 488edcb264c4..eb98054e60c9 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -11,12 +11,16 @@ If this binding is used, the kernel module i2c-hid will handle the communication
with the device and the generic hid core layer will handle the protocol.
Required properties:
-- compatible: must be "hid-over-i2c"
+- compatible: must be "hid-over-i2c", or a device-specific string like:
+ * "wacom,w9013"
- reg: i2c slave address
- hid-descr-addr: HID descriptor address
- interrupt-parent: the phandle for the interrupt controller
- interrupts: interrupt line
+Optional properties:
+- vdd-supply: phandle of the regulator that provides the supply voltage.
+
Example:
i2c-hid-dev@2c {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 2/2] HID: i2c-hid: support Wacom digitizer + regulator
From: Brian Norris @ 2016-12-01 1:21 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Caesar Wang, linux-rockchip, Rob Herring, linux-input, devicetree,
linux-kernel, Dmitry Torokhov, Mark Rutland, Doug Anderson,
Brian Norris
In-Reply-To: <1480555288-142791-1-git-send-email-briannorris@chromium.org>
We need to power on the digitizer before using it, and it's also nice to
save power in suspend by disabling it. Support an optional "vdd-supply"
and wire it up for the new Wacom device.
Wacom recommended waiting up to 100ms after powering on before trying to
access this device.
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: linux-input@vger.kernel.org
---
v1 was a few months back. I finally got around to rewriting it based on
DT binding feedback.
v2:
* support compatible property for wacom, with specific "vdd-supply" name
* support the 100ms delay needed for this digitizer
* target regulator support only at specific device
Documentation/devicetree/bindings/input/hid-over-i2c.txt | 6 +++++-
drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++++++++++++++++++++++++++++-
include/linux/i2c/i2c-hid.h | 6 ++++
2 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index b3ec4f2de875..1bc174f3a788 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -37,7 +37,9 @@
#include <linux/mutex.h>
#include <linux/acpi.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
#include <linux/i2c/i2c-hid.h>
@@ -918,10 +920,25 @@ static inline int i2c_hid_acpi_pdata(struct i2c_client *client,
#endif
#ifdef CONFIG_OF
+
+/* of_device_id match data */
+struct i2c_hid_of_data {
+ /* Name of supply regulator. */
+ const char *supply_name;
+ /* Delay required after powering on device before it is usable. */
+ int init_delay_ms;
+};
+
+static const struct i2c_hid_of_data wacom_w9013_data = {
+ .supply_name = "vdd",
+ .init_delay_ms = 100,
+};
+
static int i2c_hid_of_probe(struct i2c_client *client,
struct i2c_hid_platform_data *pdata)
{
struct device *dev = &client->dev;
+ const struct i2c_hid_of_data *data = of_device_get_match_data(dev);
u32 val;
int ret;
@@ -937,10 +954,33 @@ static int i2c_hid_of_probe(struct i2c_client *client,
}
pdata->hid_descriptor_address = val;
+ if (data) {
+ pdata->init_delay_ms = data->init_delay_ms;
+ if (data->supply_name) {
+ pdata->supply = devm_regulator_get_optional(&client->dev,
+ data->supply_name);
+ if (IS_ERR(pdata->supply)) {
+ ret = PTR_ERR(pdata->supply);
+ pdata->supply = NULL;
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ if (ret == -ENODEV)
+ return 0;
+ dev_err(dev, "Failed to get %s regulator: %d\n",
+ data->supply_name, ret);
+ return ret;
+ }
+ }
+ }
+
return 0;
}
static const struct of_device_id i2c_hid_of_match[] = {
+ {
+ .compatible = "wacom,w9013",
+ .data = &wacom_w9013_data,
+ },
{ .compatible = "hid-over-i2c" },
{},
};
@@ -983,6 +1023,17 @@ static int i2c_hid_probe(struct i2c_client *client,
ihid->pdata = *platform_data;
}
+ if (ihid->pdata.supply) {
+ ret = regulator_enable(ihid->pdata.supply);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to enable regulator: %d\n",
+ ret);
+ return ret;
+ }
+ if (ihid->pdata.init_delay_ms)
+ msleep(ihid->pdata.init_delay_ms);
+ }
+
if (client->irq > 0) {
ihid->irq = client->irq;
} else if (ACPI_COMPANION(&client->dev)) {
@@ -1100,6 +1151,9 @@ static int i2c_hid_remove(struct i2c_client *client)
if (ihid->desc)
gpiod_put(ihid->desc);
+ if (ihid->pdata.supply)
+ regulator_disable(ihid->pdata.supply);
+
kfree(ihid);
acpi_dev_remove_driver_gpios(ACPI_COMPANION(&client->dev));
@@ -1152,6 +1206,11 @@ static int i2c_hid_suspend(struct device *dev)
else
hid_warn(hid, "Failed to enable irq wake: %d\n",
wake_status);
+ } else if (ihid->pdata.supply) {
+ ret = regulator_disable(ihid->pdata.supply);
+ if (ret < 0)
+ hid_warn(hid, "Failed to disable supply: %d\n",
+ ret);
}
return 0;
@@ -1165,7 +1224,16 @@ static int i2c_hid_resume(struct device *dev)
struct hid_device *hid = ihid->hid;
int wake_status;
- if (device_may_wakeup(&client->dev) && ihid->irq_wake_enabled) {
+ if (!device_may_wakeup(&client->dev)) {
+ if (ihid->pdata.supply) {
+ ret = regulator_enable(ihid->pdata.supply);
+ if (ret < 0)
+ hid_warn(hid, "Failed to enable supply: %d\n",
+ ret);
+ if (ihid->pdata.init_delay_ms)
+ msleep(ihid->pdata.init_delay_ms);
+ }
+ } else if (ihid->irq_wake_enabled) {
wake_status = disable_irq_wake(ihid->irq);
if (!wake_status)
ihid->irq_wake_enabled = false;
diff --git a/include/linux/i2c/i2c-hid.h b/include/linux/i2c/i2c-hid.h
index 7aa901d92058..97688cde4a91 100644
--- a/include/linux/i2c/i2c-hid.h
+++ b/include/linux/i2c/i2c-hid.h
@@ -14,9 +14,13 @@
#include <linux/types.h>
+struct regulator;
+
/**
* struct i2chid_platform_data - used by hid over i2c implementation.
* @hid_descriptor_address: i2c register where the HID descriptor is stored.
+ * @supply: regulator for powering on the device.
+ * @init_delay_ms: delay after powering on before device is usable.
*
* Note that it is the responsibility of the platform driver (or the acpi 5.0
* driver, or the flattened device tree) to setup the irq related to the gpio in
@@ -31,6 +35,8 @@
*/
struct i2c_hid_platform_data {
u16 hid_descriptor_address;
+ struct regulator *supply;
+ int init_delay_ms;
};
#endif /* __LINUX_I2C_HID_H */
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH v6 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Jun Nie @ 2016-12-01 1:34 UTC (permalink / raw)
To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland
Cc: Ulf Hansson, Jaehoon Chung, Jason Liu, chen.chaokai, lai.binz,
linux-mmc, Jun Nie, devicetree
In-Reply-To: <CABymUCOvpQv7D2fi7EhWUZb_B8iwvQQ7-uW0gPrSdsFabMcPbA@mail.gmail.com>
2016-11-24 10:17 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> 2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> Document the device-tree binding of ZTE MMC host on
>> ZX296718 SoC.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>> .../devicetree/bindings/mmc/zx-dw-mshc.txt | 35 ++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> new file mode 100644
>> index 0000000..c175c4b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>> @@ -0,0 +1,35 @@
>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>> + Host Controller
>> +
>> +The Synopsys designware mobile storage host controller is used to interface
>> +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>> +differences between the core Synopsys dw mshc controller properties described
>> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be
>> + - "zte,zx296718-dw-mshc": for ZX SoCs
>> +
>> +Example:
>> +
>> + mmc1: mmc@1110000 {
>> + compatible = "zte,zx296718-dw-mshc";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x01110000 0x1000>;
>> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>> + fifo-depth = <32>;
>> + data-addr = <0x200>;
>> + fifo-watermark-aligned;
>> + bus-width = <4>;
>> + clock-frequency = <50000000>;
>> + clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>> + clock-names = "biu", "ciu";
>> + num-slots = <1>;
>> + max-frequency = <50000000>;
>> + cap-sdio-irq;
>> + cap-sd-highspeed;
>> + status = "disabled";
>> + };
>> --
>> 1.9.1
>>
>
> Hi Rob & Mark,
>
> Could you help review and act this patch if you think it is OK? Thank you!
>
> Jun
Hi Rob & Mark,
Could you help comment or act on this patch? Thank you!
Jun
^ permalink raw reply
* Re: [PATCH v6 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Jun Nie @ 2016-12-01 1:35 UTC (permalink / raw)
To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland-5wv7dgnIgG8
Cc: Ulf Hansson, Jaehoon Chung, Jason Liu,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
lai.binz-Th6q7B73Y6EnDS1+zs4M5A, linux-mmc, Jun Nie,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CABymUCN_+kJC2NZaPuqOg1C_Q2ASmiGwbwWCtfiLjGwSqPjgkQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-24 10:19 GMT+08:00 Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> 2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
>> Add fifo-addr property and fifo-watermark-quirk property to
>> synopsys-dw-mshc bindings. It is intended to provide more
>> dt interface to support SoCs specific configuration.
>>
>> Signed-off-by: Jun Nie <jun.nie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> index 4e00e85..8bf2e41 100644
>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> @@ -76,6 +76,17 @@ Optional properties:
>>
>> * broken-cd: as documented in mmc core bindings.
>>
>> +* data-addr: Override fifo address with value provided by DT. The default FIFO reg
>> + offset is assumed as 0x100 (version < 0x240A) and 0x200(version >= 0x240A) by
>> + driver. If the controller does not follow this rule, please use this property
>> + to set fifo address in device tree.
>> +
>> +* fifo-watermark-aligned: Data done irq is expected if data length is less than
>> + watermark in PIO mode. But fifo watermark is requested to be aligned with data
>> + length in some SoC so that TX/RX irq can be generated with data done irq. Add this
>> + watermark quirk to mark this requirement and force fifo watermark setting
>> + accordingly.
>> +
>> * vmmc-supply: The phandle to the regulator to use for vmmc. If this is
>> specified we'll defer probe until we can find this regulator.
>>
>> @@ -103,6 +114,8 @@ board specific portions as listed below.
>> interrupts = <0 75 0>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>> + data-addr = <0x200>;
>> + fifo-watermark-aligned;
>> };
>>
>> [board specific internal DMA resources]
>> --
>> 1.9.1
>>
> Hi Rob & Mark,
>
> Could you help review and act this patch if you think it is OK? Thank you!
>
> Jun
Hi Rob & Mark,
Could you help comment or act on this patch in revsion 6? Thank you!
Jun
--
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: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-12-01 1:43 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rob Herring, Linux-ALSA, Linux-DT, Michael Turquette,
Russell King, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <874m2pbwsn.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Hi Stephen, again
Can I confirm ??
Was I misunderstanding ??
> I understand your point, but I think devm_get_clk_from_child()
> needs new DT setings, and it can't keep compatibility, or
> it makes driver complex.
> I think it is nice to have. but, I want to keep current style.
> Thus, I will try to use current of_clk_get() as-is, and
> call clk_free() somehow in this driver.
------ Pattern1 -----------
sound_soc {
clocks = <&xxx>, <&xxx>;
clock-names = "cpu", "codec";
...
cpu { /* of_cpu_node */
...
};
codec { /* of_codec_node */
...
};
};
----------------------------
Do you mean, this case we can use
devm_get_clk_from_child(dev, of_cpu_node, "cpu");
devm_get_clk_from_child(dev, of_codec_node, "codec");
------ Pattern2 -----------
sound_soc {
...
cpu { /* of_cpu_node */
clocks = <&xxx>;
...
};
codec { /* of_codec_node */
clocks = <&xxx>;
...
};
};
----------------------------
And, this case, we can use
devm_get_clk_from_child(dev, of_cpu_node, NULL);
devm_get_clk_from_child(dev, of_codec_node, NULL);
If so, I can use it without DT change.
--
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 v8] mtd: nand: add tango NAND flash controller support
From: Brian Norris @ 2016-12-01 1:44 UTC (permalink / raw)
To: Marc Gonzalez
Cc: linux-mtd, Boris Brezillon, Richard Weinberger, Mark Rutland, DT,
Rob Herring, Sebastian Frias, Mason
In-Reply-To: <580F8407.5070706-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
Hi,
On Tue, Oct 25, 2016 at 06:10:47PM +0200, Marc Gonzalez wrote:
> This driver supports the NAND Flash controller embedded in recent
> Tango chips, such as SMP8758 and SMP8759.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
> ---
...
> diff --git a/drivers/mtd/nand/tango_nand.c b/drivers/mtd/nand/tango_nand.c
> new file mode 100644
> index 000000000000..74e39a92771c
> --- /dev/null
> +++ b/drivers/mtd/nand/tango_nand.c
> @@ -0,0 +1,654 @@
This driver is being introduced with no copyright or license header. At
least it has an appropriate MODULE_LICENSE().
Can the original author(s) please provide a follow-up patch with one?
Thanks.
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mtd/nand.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/platform_device.h>
...
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Sigma Designs");
> +MODULE_DESCRIPTION("Tango4 NAND Flash controller driver");
Brian
--
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] arm64: dts: zx: add zx296718's topcrm node
From: Jun Nie @ 2016-12-01 1:54 UTC (permalink / raw)
To: Baoyou Xie
Cc: Rob Herring, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, Will Deacon, Shawn Guo,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, xie.baoyou,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1480499418-13905-1-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-11-30 17:50 GMT+08:00 Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> Enable topcrm clock node for zx296718, which is used for
> CPU's frequency change.
Please reference other device tree patches title to add a simple title
with category information. Such as
arm64: dts: uniphier: change MIO node to SD control node
>
> Furthermore, this patch adds the CPU clock phandle in CPU's node
> and uses operating-points-v2 to register operating points.
>
> So it can be used by cpufreq-dt driver.
Detail comment should provide more information to support title. So
topcrm and cpu freq changes shall be split into two patches.
>
> Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> arch/arm64/boot/dts/zte/zx296718.dtsi | 43 +++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
> index 6b239a3..992158a 100644
> --- a/arch/arm64/boot/dts/zte/zx296718.dtsi
> +++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
> @@ -44,6 +44,7 @@
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/clock/zx296718-clock.h>
>
> / {
> compatible = "zte,zx296718";
> @@ -81,6 +82,8 @@
> compatible = "arm,cortex-a53","arm,armv8";
> reg = <0x0 0x0>;
> enable-method = "psci";
> + clocks = <&topcrm A53_GATE>;
> + operating-points-v2 = <&cluster0_opp>;
> };
>
> cpu1: cpu@1 {
> @@ -88,6 +91,7 @@
> compatible = "arm,cortex-a53","arm,armv8";
> reg = <0x0 0x1>;
> enable-method = "psci";
> + operating-points-v2 = <&cluster0_opp>;
> };
>
> cpu2: cpu@2 {
> @@ -95,6 +99,7 @@
> compatible = "arm,cortex-a53","arm,armv8";
> reg = <0x0 0x2>;
> enable-method = "psci";
> + operating-points-v2 = <&cluster0_opp>;
> };
>
> cpu3: cpu@3 {
> @@ -102,6 +107,38 @@
> compatible = "arm,cortex-a53","arm,armv8";
> reg = <0x0 0x3>;
> enable-method = "psci";
> + operating-points-v2 = <&cluster0_opp>;
> + };
> + };
> +
> + cluster0_opp: opp_table0 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp@500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + opp-microvolt = <857000>;
> + clock-latency-ns = <500000>;
> + };
> + opp@648000000 {
> + opp-hz = /bits/ 64 <648000000>;
> + opp-microvolt = <857000>;
> + clock-latency-ns = <500000>;
> + };
> + opp@800000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <882000>;
> + clock-latency-ns = <500000>;
> + };
> + opp@1000000000 {
> + opp-hz = /bits/ 64 <1000000000>;
> + opp-microvolt = <892000>;
> + clock-latency-ns = <500000>;
> + };
> + opp@1188000000 {
> + opp-hz = /bits/ 64 <1188000000>;
> + opp-microvolt = <1009000>;
> + clock-latency-ns = <500000>;
> };
I see 1600m and 1800m for a53 clock source in clk driver. Aren't they
supported by product chip?
> };
>
> @@ -279,6 +316,12 @@
> dma-requests = <32>;
> };
>
> + topcrm: clock-controller@1461000 {
> + compatible = "zte,zx296718-topcrm";
> + reg = <0x01461000 0x1000>;
> + #clock-cells = <1>;
> + };
> +
Top clock nodes patch is just merged into linux-next. You can prepare
your patch based on it.
https://www.spinics.net/lists/arm-kernel/msg535883.html
> sysctrl: sysctrl@1463000 {
> compatible = "zte,zx296718-sysctrl", "syscon";
> reg = <0x1463000 0x1000>;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4] clkdev: add devm_of_clk_get()
From: Kuninori Morimoto @ 2016-12-01 1:56 UTC (permalink / raw)
To: Stephen Boyd
Cc: Russell King - ARM Linux, Rob Herring, Linux-ALSA, Linux-DT,
Michael Turquette, Linux-Kernel, Mark Brown,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux-ARM
In-Reply-To: <20161129212600.GH6095-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen
> > Current Linux has of_clk_get(), but doesn't have devm_of_clk_get().
> > This patch adds it. It is implemeted in clk-devres.c to share
> > devm_clk_release().
>
> Please add an explanation of why we want this sort of API. The
> example you gave for audio sound card is useful. We're not going
> to remember 5 months from now why we did something, so we should
> put that here instead of digging through mailing list archives.
OK, will do
> > +struct clk *devm_of_clk_get(struct device *dev,
> > + struct device_node *np, int index)
>
> Please call this devm_get_clk_from_child() instead. Also, replace
> the index argument with a string called con_id. Then call
> of_clk_get_by_name() instead of of_clk_get().
I guess we want to have _of_ on function name ?
devm_get_clk_from_child() ?
devm_of_get_clk_from_child ?
Best regards
---
Kuninori Morimoto
--
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: Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Icenowy Zheng @ 2016-12-01 2:02 UTC (permalink / raw)
To: Maxime Ripard, Jernej Škrabec
Cc: Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
wens-jdAy2FN1RRM@public.gmane.org, linux-kernel, linux-sunxi,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161130205233.mwfqlfuqg4cefink@lukather>
01.12.2016, 04:52, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> On Wed, Nov 30, 2016 at 09:41:26PM +0100, Jernej Škrabec wrote:
>> > > > > The only
>> > > > > code left from you is for DE2. HDMI stuff is basically copied from
>> > > > > Rockhip
>> > > > > driver (including EDID reading), TCON code is now reverted to the same
>> > > > > as
>> > > > > it is in sunxi_display.c. I think it is worth to take a look at EDID
>> > > > > code
>> > > > > and compare it.
>> > > >
>> > > > So is the TCON of DE 2.0 identical to the original TCON?
>> > > >
>> > > > If so, we should reuse sun4i-tcon ...
>> > >
>> > > Well, TCON is splitted in two parts (two base addresses), one for HDMI and
>> > > one for TV. However, register offsets are same as before, so I guess
>> > > driver reusage make sense. I think that there are few additional
>> > > registers, but they can be ignored for simplefb.
>> >
>> > The TCON1 of the H3 is not usable (no ckock). Analog TV has its own
>> > clock and I/O area.
>> >
>>
>> True, H3 user manual can be misleading sometimes. But this doesn't change the
>> fact that TCON0 has same register offsets with same meaning.
>
> Then yes, we should definitely share the drivers too. So, in the end,
> the only thing that is actually new is the display-engine?
And HDMI PHY on H3 ;-)
In my opinion, we should just put sun8i-de2-drm related code into drivers/gpu/drm/sun4i/ .
(Or rename the directory to sunxi)
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH v2 0/2] mmc: sdhci: one expansion for SDHCI base code and add Cadence SDHCI driver
From: Masahiro Yamada @ 2016-12-01 3:36 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: Adrian Hunter, Ulf Hansson, Masahiro Yamada,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren, Rob Herring,
Al Cooper, Wolfram Sang, Andrei Pistirica, Mark Rutland,
Simon Horman, Eric Anholt
1/2 tweaks sdhci_execute_tuning(), which I want to use for 2/2.
2/2 adds a new driver for Cadence's controller IP.
Changes in v2:
- Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"
Masahiro Yamada (2):
mmc: sdhci: continue normal tuning if unsupported by platform tuning
mmc: sdhci-cadence: add Cadence SD4HC support
.../devicetree/bindings/mmc/sdhci-cadence.txt | 31 +++
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-cadence.c | 279 +++++++++++++++++++++
drivers/mmc/host/sdhci.c | 4 +-
5 files changed, 325 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
create mode 100644 drivers/mmc/host/sdhci-cadence.c
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
From: Masahiro Yamada @ 2016-12-01 3:36 UTC (permalink / raw)
To: linux-mmc-u79uwXL29TY76Z2rM5mHXA
Cc: Adrian Hunter, Ulf Hansson, Masahiro Yamada,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren, Rob Herring,
Al Cooper, Wolfram Sang, Andrei Pistirica, Mark Rutland,
Simon Horman, Eric Anholt
In-Reply-To: <1480563366-7259-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Add a driver for the Cadence SD4HC SD/SDIO/eMMC Controller.
For SD, it basically relies on the SDHCI standard code.
For eMMC, this driver provides some callbacks to support the
hardware part that is specific to this IP design.
Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
Changes in v2:
- Remove unnecessary "select MMC_SDHCI_IO_ACCESSORS"
.../devicetree/bindings/mmc/sdhci-cadence.txt | 31 +++
drivers/mmc/host/Kconfig | 11 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-cadence.c | 279 +++++++++++++++++++++
4 files changed, 322 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
create mode 100644 drivers/mmc/host/sdhci-cadence.c
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
new file mode 100644
index 0000000..c18aaee
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
@@ -0,0 +1,31 @@
+* Cadence SD/SDIO/eMMC Host Controller
+
+Required properties:
+- compatible: should be "cdns,sd4hc".
+- reg: offset and length of the register set for the device. The region should
+ contain both Host Register Set (HRS) and Slot Register Set (SRS).
+- interrupts: a single interrupt specifier.
+- clocks: phandle to the input clock.
+
+Optional properties:
+For eMMC configuration, supported speed modes are not provided by the SDHCI
+Capabilities Register. Instead, the following properties should be specified
+if supported. See mmc.txt for details.
+- mmc-ddr-1_8v
+- mmc-ddr-1_2v
+- mmc-hs200-1_8v
+- mmc-hs200-1_2v
+- mmc-hs400-1_8v
+- mmc-hs400-1_2v
+
+Example:
+ emmc: sdhci@5a000000 {
+ compatible = "cdns,sd4hc";
+ reg = <0x5a000000 0x400>;
+ interrupts = <0 78 4>;
+ clocks = <&clk 4>;
+ bus-width = <8>;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ };
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 580b5a1..39f6f96 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -165,6 +165,17 @@ config MMC_SDHCI_OF_HLWD
If unsure, say N.
+config MMC_SDHCI_CADENCE
+ tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
+ depends on MMC_SDHCI_PLTFM
+ depends on OF
+ help
+ This selects the Cadence SD/SDIO/eMMC driver.
+
+ If you have a controller with this interface, say Y or M here.
+
+ If unsure, say N.
+
config MMC_SDHCI_CNS3XXX
tristate "SDHCI support on the Cavium Networks CNS3xxx SoC"
depends on ARCH_CNS3XXX
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e49a82a..55f7193 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_MMC_REALTEK_PCI) += rtsx_pci_sdmmc.o
obj-$(CONFIG_MMC_REALTEK_USB) += rtsx_usb_sdmmc.o
obj-$(CONFIG_MMC_SDHCI_PLTFM) += sdhci-pltfm.o
+obj-$(CONFIG_MMC_SDHCI_CADENCE) += sdhci-cadence.o
obj-$(CONFIG_MMC_SDHCI_CNS3XXX) += sdhci-cns3xxx.o
obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
new file mode 100644
index 0000000..a7daf00
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -0,0 +1,279 @@
+/*
+ * Copyright (C) 2016 Socionext Inc.
+ * Author: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@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 as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/bitops.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mmc/host.h>
+
+#include "sdhci-pltfm.h"
+
+/* HRS - Host Register Set (specific to Cadence) */
+#define SDHCI_CDNS_HRS04 0x10 /* PHY access port */
+#define SDHCI_CDNS_HRS04_ACK BIT(26)
+#define SDHCI_CDNS_HRS04_RD BIT(25)
+#define SDHCI_CDNS_HRS04_WR BIT(24)
+#define SDHCI_CDNS_HRS04_RDATA_SHIFT 12
+#define SDHCI_CDNS_HRS04_WDATA_SHIFT 8
+#define SDHCI_CDNS_HRS04_ADDR_SHIFT 0
+
+#define SDHCI_CDNS_HRS06 0x18 /* eMMC control */
+#define SDHCI_CDNS_HRS06_TUNE_UP BIT(15)
+#define SDHCI_CDNS_HRS06_TUNE_SHIFT 8
+#define SDHCI_CDNS_HRS06_TUNE_MASK 0x3f
+#define SDHCI_CDNS_HRS06_MODE_MASK 0x7
+#define SDHCI_CDNS_HRS06_MODE_SD 0x0
+#define SDHCI_CDNS_HRS06_MODE_MMC_SDR 0x2
+#define SDHCI_CDNS_HRS06_MODE_MMC_DDR 0x3
+#define SDHCI_CDNS_HRS06_MODE_MMC_HS200 0x4
+#define SDHCI_CDNS_HRS06_MODE_MMC_HS400 0x5
+
+/* SRS - Slot Register Set (SDHCI-compatible) */
+#define SDHCI_CDNS_SRS_BASE 0x200
+
+/* PHY */
+#define SDHCI_CDNS_PHY_DLY_SD_HS 0x00
+#define SDHCI_CDNS_PHY_DLY_SD_DEFAULT 0x01
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR12 0x02
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR25 0x03
+#define SDHCI_CDNS_PHY_DLY_UHS_SDR50 0x04
+#define SDHCI_CDNS_PHY_DLY_UHS_DDR50 0x05
+#define SDHCI_CDNS_PHY_DLY_EMMC_LEGACY 0x06
+#define SDHCI_CDNS_PHY_DLY_EMMC_SDR 0x07
+#define SDHCI_CDNS_PHY_DLY_EMMC_DDR 0x08
+
+/*
+ * The tuned val register is 6 bit-wide, but not the whole of the range is
+ * available. The range 0-42 seems to be available (then 43 wraps around to 0)
+ * but I am not quite sure if it is official. Use only 0 to 39 for safety.
+ */
+#define SDHCI_CDNS_MAX_TUNING_LOOP 40
+
+struct sdhci_cdns_priv {
+ void __iomem *hrs_addr;
+};
+
+static void sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
+ u8 addr, u8 data)
+{
+ void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS04;
+ u32 tmp;
+
+ tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) |
+ (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT);
+ writel(tmp, reg);
+
+ tmp |= SDHCI_CDNS_HRS04_WR;
+ writel(tmp, reg);
+
+ tmp &= ~SDHCI_CDNS_HRS04_WR;
+ writel(tmp, reg);
+}
+
+static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
+{
+ sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4);
+ sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4);
+ sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 9);
+ sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2);
+ sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3);
+}
+
+static inline void *sdhci_cdns_priv(struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+ return sdhci_pltfm_priv(pltfm_host);
+}
+
+static unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
+{
+ /*
+ * Cadence's spec says the Timeout Clock Frequency is the same as the
+ * Base Clock Frequency. Divide it by 1000 to return a value in kHz.
+ */
+ return host->max_clk / 1000;
+}
+
+static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
+{
+ struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+ void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS06;
+ u32 tmp;
+
+ if (WARN_ON(val > SDHCI_CDNS_HRS06_TUNE_MASK))
+ return -EINVAL;
+
+ tmp = readl(reg);
+ tmp &= ~(SDHCI_CDNS_HRS06_TUNE_MASK << SDHCI_CDNS_HRS06_TUNE_SHIFT);
+ tmp |= val << SDHCI_CDNS_HRS06_TUNE_SHIFT;
+ tmp |= SDHCI_CDNS_HRS06_TUNE_UP;
+ writel(tmp, reg);
+
+ return readl_poll_timeout(reg, tmp, !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
+ 0, 1);
+}
+
+static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+ int max_streak = 0;
+ int cur_streak = 0;
+ int end_of_streak, i;
+
+ /*
+ * This handler only implements the eMMC tuning that is specific to
+ * this controller. Fall back to the standard method for SD timing.
+ */
+ if (host->timing != MMC_TIMING_MMC_HS200)
+ return -ENOTSUPP;
+
+ if (WARN_ON(opcode != MMC_SEND_TUNING_BLOCK_HS200))
+ return -EINVAL;
+
+ for (i = 0; i < SDHCI_CDNS_MAX_TUNING_LOOP; i++) {
+ if (sdhci_cdns_set_tune_val(host, i) ||
+ mmc_send_tuning(host->mmc, opcode, NULL)) { /* bad */
+ cur_streak = 0;
+ } else { /* good */
+ cur_streak++;
+ max_streak = max(max_streak, cur_streak);
+ end_of_streak = i;
+ }
+ }
+
+ if (!max_streak) {
+ dev_err(mmc_dev(host->mmc), "no tuning point found\n");
+ return -EIO;
+ }
+
+ return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
+}
+
+static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
+ unsigned int timing)
+{
+ struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+ u32 mode, tmp;
+
+ switch (timing) {
+ case MMC_TIMING_MMC_HS:
+ mode = SDHCI_CDNS_HRS06_MODE_MMC_SDR;
+ break;
+ case MMC_TIMING_MMC_DDR52:
+ mode = SDHCI_CDNS_HRS06_MODE_MMC_DDR;
+ break;
+ case MMC_TIMING_MMC_HS200:
+ mode = SDHCI_CDNS_HRS06_MODE_MMC_HS200;
+ break;
+ case MMC_TIMING_MMC_HS400:
+ mode = SDHCI_CDNS_HRS06_MODE_MMC_HS400;
+ break;
+ default:
+ mode = SDHCI_CDNS_HRS06_MODE_SD;
+ break;
+ }
+
+ /* The speed mode for eMMC is selected by HRS06 register */
+ tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06);
+ tmp &= ~SDHCI_CDNS_HRS06_MODE_MASK;
+ tmp |= mode;
+ writel(tmp, priv->hrs_addr + SDHCI_CDNS_HRS06);
+
+ /* For SD, fall back to the default handler */
+ if (mode == SDHCI_CDNS_HRS06_MODE_SD)
+ sdhci_set_uhs_signaling(host, timing);
+}
+
+static const struct sdhci_ops sdhci_cdns_ops = {
+ .set_clock = sdhci_set_clock,
+ .get_timeout_clock = sdhci_cdns_get_timeout_clock,
+ .set_bus_width = sdhci_set_bus_width,
+ .reset = sdhci_reset,
+ .platform_execute_tuning = sdhci_cdns_execute_tuning,
+ .set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
+};
+
+static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
+ .ops = &sdhci_cdns_ops,
+};
+
+static int sdhci_cdns_probe(struct platform_device *pdev)
+{
+ struct sdhci_host *host;
+ struct sdhci_pltfm_host *pltfm_host;
+ struct sdhci_cdns_priv *priv;
+ struct clk *clk;
+ int ret;
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
+ host = sdhci_pltfm_init(pdev, &sdhci_cdns_pltfm_data, sizeof(*priv));
+ if (IS_ERR(host)) {
+ ret = PTR_ERR(host);
+ goto disable_clk;
+ }
+
+ pltfm_host = sdhci_priv(host);
+ pltfm_host->clk = clk;
+
+ priv = sdhci_cdns_priv(host);
+ priv->hrs_addr = host->ioaddr;
+ host->ioaddr += SDHCI_CDNS_SRS_BASE;
+
+ ret = mmc_of_parse(host->mmc);
+ if (ret)
+ goto free;
+
+ sdhci_cdns_phy_init(priv);
+
+ ret = sdhci_add_host(host);
+ if (ret)
+ goto free;
+
+ return 0;
+free:
+ sdhci_pltfm_free(pdev);
+disable_clk:
+ clk_disable_unprepare(clk);
+
+ return ret;
+}
+
+static const struct of_device_id sdhci_cdns_match[] = {
+ { .compatible = "cdns,sd4hc" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_cdns_match);
+
+static struct platform_driver sdhci_cdns_driver = {
+ .driver = {
+ .name = "sdhci-cdns",
+ .pm = &sdhci_pltfm_pmops,
+ .of_match_table = sdhci_cdns_match,
+ },
+ .probe = sdhci_cdns_probe,
+ .remove = sdhci_pltfm_unregister,
+};
+module_platform_driver(sdhci_cdns_driver);
+
+MODULE_AUTHOR("Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>");
+MODULE_DESCRIPTION("Cadence SD/SDIO/eMMC Host Controller Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2] arm64: dts: zx: add zx296718's topcrm node
From: Baoyou Xie @ 2016-12-01 4:42 UTC (permalink / raw)
To: Jun Nie
Cc: Rob Herring, mark.rutland, catalin.marinas, Will Deacon,
Shawn Guo, devicetree, linux-arm-kernel,
Linux Kernel Mailing List, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <CABymUCMo=ADmZLhQp4HoUM5DD2AfQH8gzSLaNWTNt0Hxd9tBLA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5562 bytes --]
cause of topcrm node has been add by another patch(
https://www.spinics.net/lists/arm-kernel/msg535883.html)
<https://wx2.qq.com/cgi-bin/mmwebwx-bin/webwxcheckurl?requrl=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-kernel%2Fmsg535883.html&skey=%40crypt_180b6516_03c09cd122d18259116435c82637657d&deviceid=e227231113535420&pass_ticket=LcwX1HPKOyjV9bHO4rred18DfLeMWaWCRblxaVRH93blUR8NJfEP5tB259bk6RcB&opcode=2&scene=1&username=@7fde1b1ad7cef9d06704c35a39c01d2d292783bde3d0e6e10590ae244337bcc0>
so I will send another patch to support cpu-freq on zx296718.
everyone can ignore this patch.
On 1 December 2016 at 09:54, Jun Nie <jun.nie@linaro.org> wrote:
> 2016-11-30 17:50 GMT+08:00 Baoyou Xie <baoyou.xie@linaro.org>:
> > Enable topcrm clock node for zx296718, which is used for
> > CPU's frequency change.
>
> Please reference other device tree patches title to add a simple title
> with category information. Such as
> arm64: dts: uniphier: change MIO node to SD control node
>
> >
> > Furthermore, this patch adds the CPU clock phandle in CPU's node
> > and uses operating-points-v2 to register operating points.
> >
> > So it can be used by cpufreq-dt driver.
>
> Detail comment should provide more information to support title. So
> topcrm and cpu freq changes shall be split into two patches.
>
> >
> > Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> > ---
> > arch/arm64/boot/dts/zte/zx296718.dtsi | 43
> +++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi
> b/arch/arm64/boot/dts/zte/zx296718.dtsi
> > index 6b239a3..992158a 100644
> > --- a/arch/arm64/boot/dts/zte/zx296718.dtsi
> > +++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
> > @@ -44,6 +44,7 @@
> > #include <dt-bindings/input/input.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > #include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/clock/zx296718-clock.h>
> >
> > / {
> > compatible = "zte,zx296718";
> > @@ -81,6 +82,8 @@
> > compatible = "arm,cortex-a53","arm,armv8";
> > reg = <0x0 0x0>;
> > enable-method = "psci";
> > + clocks = <&topcrm A53_GATE>;
> > + operating-points-v2 = <&cluster0_opp>;
> > };
> >
> > cpu1: cpu@1 {
> > @@ -88,6 +91,7 @@
> > compatible = "arm,cortex-a53","arm,armv8";
> > reg = <0x0 0x1>;
> > enable-method = "psci";
> > + operating-points-v2 = <&cluster0_opp>;
> > };
> >
> > cpu2: cpu@2 {
> > @@ -95,6 +99,7 @@
> > compatible = "arm,cortex-a53","arm,armv8";
> > reg = <0x0 0x2>;
> > enable-method = "psci";
> > + operating-points-v2 = <&cluster0_opp>;
> > };
> >
> > cpu3: cpu@3 {
> > @@ -102,6 +107,38 @@
> > compatible = "arm,cortex-a53","arm,armv8";
> > reg = <0x0 0x3>;
> > enable-method = "psci";
> > + operating-points-v2 = <&cluster0_opp>;
> > + };
> > + };
> > +
> > + cluster0_opp: opp_table0 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp@500000000 {
> > + opp-hz = /bits/ 64 <500000000>;
> > + opp-microvolt = <857000>;
> > + clock-latency-ns = <500000>;
> > + };
> > + opp@648000000 {
> > + opp-hz = /bits/ 64 <648000000>;
> > + opp-microvolt = <857000>;
> > + clock-latency-ns = <500000>;
> > + };
> > + opp@800000000 {
> > + opp-hz = /bits/ 64 <800000000>;
> > + opp-microvolt = <882000>;
> > + clock-latency-ns = <500000>;
> > + };
> > + opp@1000000000 {
> > + opp-hz = /bits/ 64 <1000000000>;
> > + opp-microvolt = <892000>;
> > + clock-latency-ns = <500000>;
> > + };
> > + opp@1188000000 {
> > + opp-hz = /bits/ 64 <1188000000>;
> > + opp-microvolt = <1009000>;
> > + clock-latency-ns = <500000>;
> > };
> I see 1600m and 1800m for a53 clock source in clk driver. Aren't they
> supported by product chip?
> > };
> >
> > @@ -279,6 +316,12 @@
> > dma-requests = <32>;
> > };
> >
> > + topcrm: clock-controller@1461000 {
> > + compatible = "zte,zx296718-topcrm";
> > + reg = <0x01461000 0x1000>;
> > + #clock-cells = <1>;
> > + };
> > +
>
> Top clock nodes patch is just merged into linux-next. You can prepare
> your patch based on it.
>
> https://www.spinics.net/lists/arm-kernel/msg535883.html
>
> > sysctrl: sysctrl@1463000 {
> > compatible = "zte,zx296718-sysctrl", "syscon";
> > reg = <0x1463000 0x1000>;
> > --
> > 2.7.4
> >
>
[-- Attachment #2: Type: text/html, Size: 7968 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/2] Support for Axentia TSE-850
From: Peter Rosin @ 2016-12-01 7:02 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Mark Rutland, devicetree, linux-kernel, Russell King, Rob Herring,
Alexandre Belloni
In-Reply-To: <5198068.r9o6jcztHW@wuerfel>
On 2016-11-30 23:25, Arnd Bergmann wrote:
> On Wednesday, November 30, 2016 1:48:20 PM CET Peter Rosin wrote:
>> Hi!
>>
>> changes v2 -> v3
>> - document the new compatible strings prefixed with "axentia,".
>>
>> changes v1 -> v2
>> - squash the fixup into the correct patch, sorry for the noise.
>>
>> After finally having all essintial drivers upstreamed (the
>> last ones are currently in -next) I would like to have the
>> dts and the defconfig also upstreamed.
>>
>> Cheers,
>> Peter
>>
>> Peter Rosin (2):
>> ARM: dts: add devicetree for the Axentia TSE-850
>> ARM: tse850_defconfig: add Axentia TSE-850
>>
>> Documentation/devicetree/bindings/arm/axentia.txt | 19 ++
>> MAINTAINERS | 8 +
>> arch/arm/boot/dts/Makefile | 1 +
>> arch/arm/boot/dts/axentia-linea.dtsi | 53 +++++
>> arch/arm/boot/dts/axentia-tse850-3.dts | 276 ++++++++++++++++++++++
>> arch/arm/configs/tse850_defconfig | 223 +++++++++++++++++
>> 6 files changed, 580 insertions(+)
>
> Hi Peter,
>
> I'm a bit confused. Are these just boards using the sama5d31 SoC,
> or something else?
No no, it's just what it seems, a cpu module with a sama5d31 and a
board using it (a couple more boards using the cpu module coming
later). I just didn't know about the naming conventions.
> Normally, dts files are picked up by the SoC platform maintainers
> and named with a prefix for that soc.
I was starting to wonder about the deafening silence, now I know
the reason...
> Also, we don't normally add a defconfig file for a specific
> machine, just add the options you want to sama5_defconfig
> and multi_v7_defconfig, and send all patches to the
> at91 maitainers.
I'll make some changes, thank you very much for the pointers!
Cheers,
Peter
^ permalink raw reply
* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Sakari Ailus @ 2016-12-01 7:57 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <m2zikgh5f0.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
>
> > Hi Kevin,
> >
> > On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
> >> Hi Sakari,
> >>
> >> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> >>
> >> > On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
> >> >> Allow getting of subdevs from DT ports and endpoints.
> >> >>
> >> >> The _get_pdata() function was larely inspired by (i.e. stolen from)
> >> >
> >> > vpif_capture_get_pdata and "largely"?
> >>
> >> Yes, thanks.
> >>
> >> >> am437x-vpfe.c
> >> >>
> >> >> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >> >> ---
> >> >> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++++++++++++-
> >> >> include/media/davinci/vpif_types.h | 9 +-
> >> >> 2 files changed, 133 insertions(+), 6 deletions(-)
> >> >>
> >> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> >> >> index 94ee6cf03f02..47a4699157e7 100644
> >> >> --- a/drivers/media/platform/davinci/vpif_capture.c
> >> >> +++ b/drivers/media/platform/davinci/vpif_capture.c
> >> >> @@ -26,6 +26,8 @@
> >> >> #include <linux/slab.h>
> >> >>
> >> >> #include <media/v4l2-ioctl.h>
> >> >> +#include <media/v4l2-of.h>
> >> >> +#include <media/i2c/tvp514x.h>
> >> >
> >> > Do you need this header?
> >> >
> >>
> >> Yes, based on discussion with Hans, since there is no DT binding for
> >> selecting the input pins of the TVP514x, I have to select it in the
> >> driver, so I need the defines from this header. More on this below...
> >>
> >> >>
> >> >> #include "vpif.h"
> >> >> #include "vpif_capture.h"
> >> >> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
> >> >>
> >> >> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
> >> >>
> >> >> + if (!chan_cfg)
> >> >> + return -1;
> >> >> + if (input_index >= chan_cfg->input_count)
> >> >> + return -1;
> >> >> subdev_name = chan_cfg->inputs[input_index].subdev_name;
> >> >> if (subdev_name == NULL)
> >> >> return -1;
> >> >> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
> >> >> /* loop through the sub device list to get the sub device info */
> >> >> for (i = 0; i < vpif_cfg->subdev_count; i++) {
> >> >> subdev_info = &vpif_cfg->subdev_info[i];
> >> >> - if (!strcmp(subdev_info->name, subdev_name))
> >> >> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
> >> >> return i;
> >> >> }
> >> >> return -1;
> >> >> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct v4l2_async_notifier *notifier,
> >> >> {
> >> >> int i;
> >> >>
> >> >> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> >> >> + struct v4l2_async_subdev *_asd = vpif_obj.config->asd[i];
> >> >> + const struct device_node *node = _asd->match.of.node;
> >> >> +
> >> >> + if (node == subdev->of_node) {
> >> >> + vpif_obj.sd[i] = subdev;
> >> >> + vpif_obj.config->chan_config->inputs[i].subdev_name =
> >> >> + (char *)subdev->of_node->full_name;
> >> >> + vpif_dbg(2, debug,
> >> >> + "%s: setting input %d subdev_name = %s\n",
> >> >> + __func__, i, subdev->of_node->full_name);
> >> >> + return 0;
> >> >> + }
> >> >> + }
> >> >> +
> >> >> for (i = 0; i < vpif_obj.config->subdev_count; i++)
> >> >> if (!strcmp(vpif_obj.config->subdev_info[i].name,
> >> >> subdev->name)) {
> >> >> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct v4l2_async_notifier *notifier)
> >> >> return vpif_probe_complete();
> >> >> }
> >> >>
> >> >> +static struct vpif_capture_config *
> >> >> +vpif_capture_get_pdata(struct platform_device *pdev)
> >> >> +{
> >> >> + struct device_node *endpoint = NULL;
> >> >> + struct v4l2_of_endpoint bus_cfg;
> >> >> + struct vpif_capture_config *pdata;
> >> >> + struct vpif_subdev_info *sdinfo;
> >> >> + struct vpif_capture_chan_config *chan;
> >> >> + unsigned int i;
> >> >> +
> >> >> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> >> >> +
> >> >> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> >> >> + return pdev->dev.platform_data;
> >> >> +
> >> >> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> >> >> + if (!pdata)
> >> >> + return NULL;
> >> >> + pdata->subdev_info =
> >> >> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
> >> >> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> >> >> +
> >> >> + if (!pdata->subdev_info)
> >> >> + return NULL;
> >> >> + dev_dbg(&pdev->dev, "%s\n", __func__);
> >> >> +
> >> >> + for (i = 0; ; i++) {
> >> >> + struct device_node *rem;
> >> >> + unsigned int flags;
> >> >> + int err;
> >> >> +
> >> >> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
> >> >> + endpoint);
> >> >> + if (!endpoint)
> >> >> + break;
> >> >> +
> >> >> + sdinfo = &pdata->subdev_info[i];
> >> >
> >> > subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
> >> >
> >>
> >> Right, I need to make the loop only go for a max of
> >> VPIF_CAPTURE_MAX_CHANNELS iterations.
> >>
> >> >> + chan = &pdata->chan_config[i];
> >> >> + chan->inputs = devm_kzalloc(&pdev->dev,
> >> >> + sizeof(*chan->inputs) *
> >> >> + VPIF_DISPLAY_MAX_CHANNELS,
> >> >> + GFP_KERNEL);
> >> >> +
> >> >> + chan->input_count++;
> >> >> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> >> >
> >> > I wonder what's the purpose of using index i on this array as well.
> >>
> >> The number of endpoints in DT is the number of input channels configured
> >> (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
> >>
> >> > If you use that to access a corresponding entry in a different array, I'd
> >> > just create a struct that contains the port configuration and the async
> >> > sub-device. The omap3isp driver does that, for instance; see
> >> > isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if you're
> >> > interested. Up to you.
> >>
> >> OK, I'll have a look at that driver. The goal here with this series is
> >> just to get this working with DT, but also not break the existing legacy
> >> platform_device support, so I'm trying not to mess with the
> >> driver-interal data structures too much.
> >
> > Ack.
> >
> >>
> >> >> + chan->inputs[i].input.std = V4L2_STD_ALL;
> >> >> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> >> >> +
> >> >> + /* FIXME: need a new property? ch0:composite ch1: s-video */
> >> >> + if (i == 0)
> >> >
> >> > Can you assume that the first endopoint has got a particular kind of input?
> >> > What if it's not connected?
> >>
> >> On all the boards I know of (there aren't many using this SoC), it's a
> >> safe assumption.
> >>
> >> > If this is a different physical port (not in the meaning another) in the
> >> > device, I'd use the reg property for this. Please see
> >> > Documentation/devicetree/bindings/media/video-interfaces.txt .
> >>
> >> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
> >> that it's not physically a different port. Instead, it's just telling
> >> the TVP514x which pin(s) will be active inputs (and what kind of signal
> >> will be present.)
> >>
> >> I'm open to a better way to describe this input select from DT, but
> >> based on what I heard from Hans, there isn't currently a good way to do
> >> that except for in the driver:
> >> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
> >>
> >> Based on further discussion in that thread, it sounds like there may be
> >> a way forward coming soon, and I'll be glad to switch to that when it
> >> arrives.
> >
> > I'm not sure that properly supporting connectors will provide any help here.
> >
> > Looking at the s_routing() API, it's the calling driver that has to be aware
> > of sub-device specific function parameters. As such it's not a very good
> > idea to require that a driver is aware of the value range of another
> > driver's parameter. I wonder if a simple enumeration interface would help
> > here --- if I understand correctly, the purpose is just to provide a way to
> > choose the input using VIDIOC_S_INPUT.
> >
> > I guess that's somehow ok as long as you have no other combinations of these
> > devices but this is hardly future-proof. (And certainly not a problem
> > created by this patch.)
>
> Yeah, this is far from future proof.
>
> > It'd be still nice to fix that as presumably we don't have the option of
> > reworking how we expect the device tree to look like.
>
> Agreed.
>
> I'm just hoping someone can shed som light on "how we expect the device
> tree to look". ;)
:-)
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.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] drivers/of: fix missing pr_cont()s in of_print_phandle_args
From: Marcin Nowakowski @ 2016-12-01 8:00 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Since the KERN_CONT changes, the current debug printks have a lot of
empty lines making the log messages very hard to read.
Signed-off-by: Marcin Nowakowski <marcin.nowakowski-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
drivers/of/base.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 0810c5e..bae07f5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1566,9 +1566,12 @@ void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
{
int i;
printk("%s %s", msg, of_node_full_name(args->np));
- for (i = 0; i < args->args_count; i++)
- printk(i ? ",%08x" : ":%08x", args->args[i]);
- printk("\n");
+ for (i = 0; i < args->args_count; i++) {
+ const char delim = i ? ',' : ':';
+
+ pr_cont("%c%08x", delim, args->args[i]);
+ }
+ pr_cont("\n");
}
int of_phandle_iterator_init(struct of_phandle_iterator *it,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v3 4/4] [media] dt-bindings: add TI VPIF documentation
From: Sakari Ailus @ 2016-12-01 8:01 UTC (permalink / raw)
To: Kevin Hilman
Cc: Rob Herring, linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Hans Verkuil, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sekhar Nori, Axel Haslam, Bartosz Gołaszewski,
Alexandre Bailon, David Lechner, g.liakhovetski-Mmb7MZpHnFY,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
In-Reply-To: <m27f7kil5o.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Hi Kevin,
On Wed, Nov 30, 2016 at 03:48:51PM -0800, Kevin Hilman wrote:
> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
>
> > Hi Rob and Kevin,
> >
> > On Tue, Nov 29, 2016 at 08:41:44AM -0600, Rob Herring wrote:
> >> On Mon, Nov 28, 2016 at 4:30 PM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> >> > Hi Rob,
> >> >
> >> > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> >> >
> >> >> On Tue, Nov 22, 2016 at 07:52:44AM -0800, Kevin Hilman wrote:
> >> >>> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >> >>> ---
> >> >>> .../bindings/media/ti,da850-vpif-capture.txt | 65 ++++++++++++++++++++++
> >> >>> .../devicetree/bindings/media/ti,da850-vpif.txt | 8 +++
> >> >>> 2 files changed, 73 insertions(+)
> >> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >> >>> create mode 100644 Documentation/devicetree/bindings/media/ti,da850-vpif.txt
> >> >>>
> >> >>> diff --git a/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >> >>> new file mode 100644
> >> >>> index 000000000000..c447ac482c1d
> >> >>> --- /dev/null
> >> >>> +++ b/Documentation/devicetree/bindings/media/ti,da850-vpif-capture.txt
> >> >>> @@ -0,0 +1,65 @@
> >> >>> +Texas Instruments VPIF Capture
> >> >>> +------------------------------
> >> >>> +
> >> >>> +The TI Video Port InterFace (VPIF) capture component is the primary
> >> >>> +component for video capture on the DA850 family of TI DaVinci SoCs.
> >> >>> +
> >> >>> +TI Document number reference: SPRUH82C
> >> >>> +
> >> >>> +Required properties:
> >> >>> +- compatible: must be "ti,da850-vpif-capture"
> >> >>> +- reg: physical base address and length of the registers set for the device;
> >> >>> +- interrupts: should contain IRQ line for the VPIF
> >> >>> +
> >> >>> +VPIF capture has a 16-bit parallel bus input, supporting 2 8-bit
> >> >>> +channels or a single 16-bit channel. It should contain at least one
> >> >>> +port child node with child 'endpoint' node. Please refer to the
> >> >>> +bindings defined in
> >> >>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> >> >>> +
> >> >>> +Example using 2 8-bit input channels, one of which is connected to an
> >> >>> +I2C-connected TVP5147 decoder:
> >> >>> +
> >> >>> + vpif_capture: video-capture@0x00217000 {
> >> >>> + reg = <0x00217000 0x1000>;
> >> >>> + interrupts = <92>;
> >> >>> +
> >> >>> + port {
> >> >>> + vpif_ch0: endpoint@0 {
> >> >>> + reg = <0>;
> >> >>> + bus-width = <8>;
> >> >>> + remote-endpoint = <&composite>;
> >> >>> + };
> >> >>> +
> >> >>> + vpif_ch1: endpoint@1 {
> >> >>
> >> >> I think probably channels here should be ports rather than endpoints.
> >> >> AIUI, having multiple endpoints is for cases like a mux or 1 to many
> >> >> connections. There's only one data flow, but multiple sources or sinks.
> >> >
> >> > Looking at this closer... , I used an endpoint because it's bascially a
> >> > 16-bit parallel bus, that can be configured as (up to) 2 8-bit
> >> > "channels. So, based on the video-interfaces.txt doc, I configured this
> >> > as a single port, with (up to) 2 endpoints. That also allows me to
> >> > connect output of the decoder directly, using the remote-endpoint
> >> > property.
> >> >
> >> > So I guess I'm not fully understanding your suggestion.
> >>
> >> NM, looks like video-interfaces.txt actually spells out this case and
> >> defines doing it as you did.
> >
> > It's actually the first time I read that portion (at least so that I could
> > remember) of video-interfaces.txt. I'm not sure if anyone has implemented
> > that previously, nor how we ended up with the text. The list archive could
> > probably tell. Cc Guennadi who wrote it. :-) I couldn't immediately find DT
> > source with this arrangement.
> >
> > In case of splitting the port into two parallel interfaces, how do you
> > determine which wires belong to which endpoint? I guess they'd be particular
> > sets of wires but as there's just a single port it isn't defined by the
> > port.
>
> Isn't that the point of data-shift?
Right.
>
> e.g. it's a single 16-bit parallel bus, where the lower 8 bits are for
> channel 0 and the upper 8 bits are for channel 1. Alternately, the port
> can also be configured as a single 16-bit channel (e.g. for raw
> capture.)
>
> If you want more details on this hardware, it's pretty well described in
> Chapter 35 of http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf.
>
> FWIW, I'm not really picky about how to do this. I'm trying to learn
> "the right way" and am happy to do that, but the feedback so far has
> been confusing (at least for someone relatively new to the DT side of
> the media framework.)
I have to admit I'm more familiar with cameras and serial busses than TV
tuners and parallel busses. Yeah, the data-shift is there for that purpose.
I think you should document which properties are expected to be found in the
port / endpoint nodes, the video-interfaces.txt has very many of them and I
don't think many of those are even relevant in this case.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
From: Vivek Gautam @ 2016-12-01 8:42 UTC (permalink / raw)
To: Stephen Boyd
Cc: kishon, robh+dt, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Srinivas Kandagatla, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161128231424.GN6095-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen,
On Tue, Nov 29, 2016 at 4:44 AM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 11/22, Vivek Gautam wrote:
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f2..f1dcec1 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -430,6 +430,17 @@ config PHY_STIH407_USB
>> Enable this support to enable the picoPHY device used by USB2
>> and USB3 controllers on STMicroelectronics STiH407 SoC families.
>>
>> +config PHY_QCOM_QUSB2
>> + tristate "Qualcomm QUSB2 PHY Driver"
>> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> + select GENERIC_PHY
>> + select RESET_CONTROLLER
>
> This shouldn't be necessary. We only need to select it if we're
> providing resets.
Ok, will drop this.
>
>> + help
>> + Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
>> + controllers on Qualcomm chips. This driver supports the high-speed
>> + PHY which is usually paired with either the ChipIdea or Synopsys DWC3
>> + USB IPs on MSM SOCs.
>> +
>> config PHY_QCOM_UFS
>> tristate "Qualcomm UFS PHY driver"
>> depends on OF && ARCH_QCOM
>> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
>> new file mode 100644
>> index 0000000..d3f9657
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qusb2.c
>> @@ -0,0 +1,549 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/nvmem-consumer.h>
>> +#include <linux/of.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
>> +#define QUSB2PHY_PLL_TEST 0x04
>> +#define CLK_REF_SEL BIT(7)
>> +
>> +#define QUSB2PHY_PLL_TUNE 0x08
>> +#define QUSB2PHY_PLL_USER_CTL1 0x0c
>> +#define QUSB2PHY_PLL_USER_CTL2 0x10
>> +#define QUSB2PHY_PLL_AUTOPGM_CTL1 0x1c
>> +#define QUSB2PHY_PLL_PWR_CTRL 0x18
>> +
>> +#define QUSB2PHY_PLL_STATUS 0x38
>> +#define PLL_LOCKED BIT(5)
>> +
>> +#define QUSB2PHY_PORT_TUNE1 0x80
>> +#define QUSB2PHY_PORT_TUNE2 0x84
>> +#define QUSB2PHY_PORT_TUNE3 0x88
>> +#define QUSB2PHY_PORT_TUNE4 0x8C
>> +#define QUSB2PHY_PORT_TUNE5 0x90
>> +#define QUSB2PHY_PORT_TEST2 0x9c
>
> Please use lowercase or uppercase consistently (I'd prefer
> lowercase).
Sure, lowercase.
>
>> +
>> +#define QUSB2PHY_PORT_POWERDOWN 0xB4
>> +#define CLAMP_N_EN BIT(5)
>> +#define FREEZIO_N BIT(1)
>> +#define POWER_DOWN BIT(0)
>> +
>> +#define QUSB2PHY_REFCLK_ENABLE BIT(0)
>> +
>> +#define PHY_CLK_SCHEME_SEL BIT(0)
>> +
>> +struct qusb2_phy_init_tbl {
>> + unsigned int reg_offset;
>> + unsigned int cfg_val;
>> +};
>> +#define QCOM_QUSB2_PHY_INIT_CFG(reg, val) \
>> + { \
>> + .reg_offset = reg, \
>> + .cfg_val = val, \
>> + }
>> +
>> +static struct qusb2_phy_init_tbl msm8996_phy_init_tbl[] = {
>
> const?
argh! shouldn't have missed these 'const' and 'static' assignments.
will update for all instances.
>
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xF8),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xB3),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xC0),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9F),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>> +};
>> +
>> +struct qusb2_phy_init_cfg {
>> + struct qusb2_phy_init_tbl *phy_init_tbl;
>> + int phy_init_tbl_sz;
>> + /* offset to PHY_CLK_SCHEME register in TCSR map. */
>> + unsigned int clk_scheme_offset;
>> +};
>> +
>> +const struct qusb2_phy_init_cfg msm8996_phy_init_cfg = {
>
> static?
>
>> + .phy_init_tbl = msm8996_phy_init_tbl,
>> + .phy_init_tbl_sz = ARRAY_SIZE(msm8996_phy_init_tbl),
>> +};
>> +
>> +/**
>> + * struct qusb2_phy: Structure holding qusb2 phy attributes.
>> + *
>> + * @phy: pointer to generic phy.
>> + * @base: pointer to iomapped memory space for qubs2 phy.
>> + *
>> + * @cfg_ahb_clk: pointer to AHB2PHY interface clock.
>> + * @ref_clk: pointer to reference clock.
>> + * @ref_clk_src: pointer to source to reference clock.
>> + * @iface_src: pointer to phy interface clock.
>> + *
>> + * @phy_reset: Pointer to phy reset control
>> + *
>> + * @vdda_phy: vdd supply to the phy core block.
>> + * @vdda_pll: 1.8V vdd supply to ref_clk block.
>> + * @vdda_phy_dpdm: 3.1V vdd supply to Dp/Dm port signals.
>> + * @tcsr: pointer to TCSR syscon register map.
>
> Drop all the full stops on these comments please.
sure.
>
>> + *
>> + * @cfg: phy initialization config data
>> + * @has_se_clk_scheme: indicate if PHY has Single-ended ref clock scheme
>
> Why is single capitalized?
ok, 'single-ended'
>
>> + */
>> +struct qusb2_phy {
>> + struct phy *phy;
>> + void __iomem *base;
>> +
>> + struct clk *cfg_ahb_clk;
>> + struct clk *ref_clk;
>> + struct clk *ref_clk_src;
>> + struct clk *iface_clk;
>> +
>> + struct reset_control *phy_reset;
>> +
>> + struct regulator *vdd_phy;
>> + struct regulator *vdda_pll;
>> + struct regulator *vdda_phy_dpdm;
>> +
>> + struct regmap *tcsr;
>> +
>> + const struct qusb2_phy_init_cfg *cfg;
>> + bool has_se_clk_scheme;
>> +};
>> +
>> +static inline void qusb2_setbits(void __iomem *reg, u32 val)
>> +{
>> + u32 reg_val;
>> +
>> + reg_val = readl_relaxed(reg);
>> + reg_val |= val;
>> + writel_relaxed(reg_val, reg);
>> +
>> + /* Ensure above write is completed */
>> + mb();
>> +}
>> +
>> +static inline void qusb2_clrbits(void __iomem *reg, u32 val)
>> +{
>> + u32 reg_val;
>> +
>> + reg_val = readl_relaxed(reg);
>> + reg_val &= ~val;
>> + writel_relaxed(reg_val, reg);
>> +
>> + /* Ensure above write is completed */
>> + mb();
>> +}
>> +
>> +static void qcom_qusb2_phy_configure(void __iomem *base,
>> + struct qusb2_phy_init_tbl init_tbl[],
>> + int init_tbl_sz)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < init_tbl_sz; i++) {
>> + writel_relaxed(init_tbl[i].cfg_val,
>> + base + init_tbl[i].reg_offset);
>> + }
>> +
>> + /* flush buffered writes */
>> + mb();
>> +}
>> +
>> +static void qusb2_phy_enable_clocks(struct qusb2_phy *qphy, bool on)
>
> Maybe s/enable/toggle/ because we're not always enabling.
yea, toggle is better; will modify.
>
>> +{
>> + if (on) {
>> + clk_prepare_enable(qphy->iface_clk);
>> + clk_prepare_enable(qphy->ref_clk_src);
>> + } else {
>> + clk_disable_unprepare(qphy->ref_clk_src);
>> + clk_disable_unprepare(qphy->iface_clk);
>> + }
>> +
>> + dev_vdbg(&qphy->phy->dev, "%s(): clocks enabled\n", __func__);
>
> Heh or disabled!
yup, a check for 'on' and relevant string - enabled/disabled. will do it.
>
>> +}
>> +
>> +static int qusb2_phy_enable_power(struct qusb2_phy *qphy, bool on)
>
> Maybe s/enable/toggle/ because we're not always enabling.
Yea, will update it to 'toggle'.
>
>> +{
>> + int ret;
>> + struct device *dev = &qphy->phy->dev;
>> +
>> + if (!on)
>> + goto disable_vdda_phy_dpdm;
>> +
>> + ret = regulator_enable(qphy->vdd_phy);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdd-phy:%d\n", ret);
>> + goto err_vdd_phy;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_pll);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdda-pll:%d\n", ret);
>> + goto disable_vdd_phy;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_phy_dpdm);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdda-phy-dpdm:%d\n", ret);
>> + goto disable_vdda_pll;
>> + }
>> +
>> + dev_vdbg(dev, "%s() regulators are turned on.\n", __func__);
>
> Drop the full stop please.
ok.
>
>> +
>> + return ret;
>> +
>> +disable_vdda_phy_dpdm:
>> + regulator_disable(qphy->vdda_phy_dpdm);
>> +disable_vdda_pll:
>> + regulator_disable(qphy->vdda_pll);
>> +disable_vdd_phy:
>> + regulator_disable(qphy->vdd_phy);
>> +err_vdd_phy:
>> + dev_vdbg(dev, "%s() regulators are turned off.\n", __func__);
>> + return ret;
>> +}
>> +
>> +/*
>> + * Fetches HS Tx tuning value from e-fuse and sets QUSB2PHY_PORT_TUNE2
>> + * register.
>> + * For any error case, skip setting the value and use the default value.
>> + */
>> +static int qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
>> +{
>> + struct device *dev = &qphy->phy->dev;
>> + struct nvmem_cell *cell;
>> + ssize_t len;
>> + u8 *val;
>> +
>> + /*
>> + * Read EFUSE register having TUNE2 parameter's high nibble.
>> + * If efuse register shows value as 0x0, or if we fail to find
>> + * a valid efuse register settings, then use default value
>> + * as 0xB for high nibble that we have already set while
>> + * configuring phy.
>> + */
>> + cell = devm_nvmem_cell_get(dev, "tune2_hstx_trim_efuse");
>> + if (IS_ERR(cell)) {
>> + if (PTR_ERR(cell) == -EPROBE_DEFER)
>> + return PTR_ERR(cell);
>> + goto skip;
>
> Why do we get the nvmem cell here? Wouldn't we want to get it
> during probe? Returning probe defer here during init would be
> odd.
Yea, my bad. This should be moved to probe().
>
>> + }
>> +
>> + /*
>> + * we need to read only one byte here, since the required
>> + * parameter value fits in one nibble
>> + */
>> + val = (u8 *)nvmem_cell_read(cell, &len);
>
> Shouldn't need the cast here. Also it would be nice if
> nvmem_cell_read() didn't require a second argument if we don't
> care for it. We should update the API to allow NULL there.
Will remove the u8 pointer cast.
Correct, it makes sense to allow the length pointer to be passed as NULL.
We don't care about this length. Will update the nvmem API, to allow this.
Also, we should add a check for 'cell' as well. This pointer can be
NULL, and the first thing that nvmem_cell_read does is - deference
the pointer 'cell'
>
>> + if (!IS_ERR(val)) {
>> + /* Fused TUNE2 value is the higher nibble only */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_TUNE2,
>> + val[0] << 0x4);
>> + } else {
>> + dev_dbg(dev, "failed reading hs-tx trim value: %ld\n",
>> + PTR_ERR(val));
>> + }
>> +
>> +skip:
>> + return 0;
>> +}
>> +
> [...]
>> +
>> +static int qusb2_phy_init(struct phy *phy)
>> +{
>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
>> + unsigned int reset_val;
>> + unsigned int clk_scheme;
>> + int ret;
>> +
>> + dev_vdbg(&phy->dev, "Initializing QUSB2 phy\n");
>> +
>> + /* enable ahb interface clock to program phy */
>> + clk_prepare_enable(qphy->cfg_ahb_clk);
>
> What if that fails?
Yea, will add the necessary checks for failure here and in the rest of the patch
wherever necessary.
>
>> +
>> + /* Perform phy reset */
>> + ret = reset_control_assert(qphy->phy_reset);
>> + if (ret) {
>> + dev_err(&phy->dev, "Failed to assert phy_reset\n");
>> + return ret;
>> + }
>> + /* 100 us delay to keep PHY in reset mode */
>> + usleep_range(100, 150);
>> + ret = reset_control_deassert(qphy->phy_reset);
>> + if (ret) {
>> + dev_err(&phy->dev, "Failed to de-assert phy_reset\n");
>> + return ret;
>> + }
>> +
>> + /* Disable the PHY */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
>> + CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
>> +
>> + /* save reset value to override based on clk scheme */
>> + reset_val = readl_relaxed(qphy->base + QUSB2PHY_PLL_TEST);
>> +
>> + qcom_qusb2_phy_configure(qphy->base, qphy->cfg->phy_init_tbl,
>> + qphy->cfg->phy_init_tbl_sz);
>> +
>> + /* Check for efuse value for tuning the PHY */
>> + ret = qusb2_phy_set_tune2_param(qphy);
>> + if (ret)
>> + return ret;
>> +
>> + /* Enable the PHY */
>> + qusb2_clrbits(qphy->base + QUSB2PHY_PORT_POWERDOWN, POWER_DOWN);
>> +
>> + /* Require to get phy pll lock successfully */
>> + usleep_range(150, 160);
>> +
>> + /* Default is Single-ended clock on msm8996 */
>> + qphy->has_se_clk_scheme = true;
>> + /*
>> + * read TCSR_PHY_CLK_SCHEME register to check if Single-ended
>
> Capital Single again?
will use lowercase.
>
>> + * clock scheme is selected. If yes, then disable differential
>> + * ref_clk and use single-ended clock, otherwise use differential
>> + * ref_clk only.
>> + */
>> + if (qphy->tcsr) {
>> + ret = regmap_read(qphy->tcsr, qphy->cfg->clk_scheme_offset,
>> + &clk_scheme);
>> + /* is it a differential clock scheme ? */
>> + if (!(clk_scheme & PHY_CLK_SCHEME_SEL)) {
>> + dev_vdbg(&phy->dev, "%s: select differential clk src\n",
>> + __func__);
>> + qphy->has_se_clk_scheme = false;
>> + } else {
>> + dev_vdbg(&phy->dev, "%s: select single-ended clk src\n",
>> + __func__);
>> + }
>> + }
>> +
>> + if (!qphy->has_se_clk_scheme) {
>> + reset_val &= ~CLK_REF_SEL;
>> + clk_prepare_enable(qphy->ref_clk);
>
> And if that fails?
will add the check.
>
>> + } else {
>> + reset_val |= CLK_REF_SEL;
>> + }
>> +
>> + writel_relaxed(reset_val, qphy->base + QUSB2PHY_PLL_TEST);
>> +
>> + /* Make sure that above write is completed to get PLL source clock */
>> + wmb();
>> +
>> + /* Required to get PHY PLL lock successfully */
>> + usleep_range(100, 110);
>> +
>> + if (!(readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS) &
>> + PLL_LOCKED)) {
>> + dev_err(&phy->dev, "QUSB PHY PLL LOCK fails:%x\n",
>> + readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS));
>
> Would be pretty funny if this was locked now when the error
> printk runs. Are there other bits in there that are helpful?
This is the only bit that's there to check the PLL locking status.
Should we rather poll ?
>
>> + return -EBUSY;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qusb2_phy_exit(struct phy *phy)
>> +{
>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
>> +
>> + /* Disable the PHY */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
>> + CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
>> +
>> + if (!qphy->has_se_clk_scheme)
>> + clk_disable_unprepare(qphy->ref_clk);
>> +
>> + clk_disable_unprepare(qphy->cfg_ahb_clk);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops qusb2_phy_gen_ops = {
>> + .init = qusb2_phy_init,
>> + .exit = qusb2_phy_exit,
>> + .power_on = qusb2_phy_poweron,
>> + .power_off = qusb2_phy_poweroff,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct of_device_id qusb2_phy_of_match_table[] = {
>> + {
>> + .compatible = "qcom,msm8996-qusb2-phy",
>> + .data = &msm8996_phy_init_cfg,
>> + },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qusb2_phy_of_match_table);
>> +
>> +static int qusb2_phy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct qusb2_phy *qphy;
>> + struct phy_provider *phy_provider;
>> + struct phy *generic_phy;
>> + const struct of_device_id *match;
>> + struct resource *res;
>> + int ret;
>> +
>> + qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
>> + if (!qphy)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + qphy->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(qphy->base))
>> + return PTR_ERR(qphy->base);
>> +
>> + qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb_clk");
>> + if (IS_ERR(qphy->cfg_ahb_clk)) {
>> + ret = PTR_ERR(qphy->cfg_ahb_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "failed to get cfg_ahb_clk\n");
>> + return ret;
>> + }
>> +
>> + qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src");
>> + if (IS_ERR(qphy->ref_clk_src)) {
>> + ret = PTR_ERR(qphy->ref_clk_src);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "clk get failed for ref_clk_src\n");
>> + return ret;
>> + }
>> +
>> + qphy->ref_clk = devm_clk_get(dev, "ref_clk");
>> + if (IS_ERR(qphy->ref_clk)) {
>> + ret = PTR_ERR(qphy->ref_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "clk get failed for ref_clk\n");
>> + return ret;
>> + } else {
>> + clk_set_rate(qphy->ref_clk, 19200000);
>
> Drop the else. Also what if clk_set_rate() fails?
Will drop the else.
we should fail in case clk_set_rate() fails.
>
>> + }
>> +
>> + qphy->iface_clk = devm_clk_get(dev, "iface_clk");
>> + if (IS_ERR(qphy->iface_clk)) {
>> + ret = PTR_ERR(qphy->iface_clk);
>> + if (ret != -EPROBE_DEFER) {
>> + qphy->iface_clk = NULL;
>> + dev_dbg(dev, "clk get failed for iface_clk\n");
>> + } else {
>> + return ret;
>> + }
>
> if (PTR_ERR(qphy->iface_clk) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> qphy->iface_clk = NULL;
> dev_dbg(dev, "clk get failed for iface_clk\n");
>
> Is shorter.
Sure, will modify this as suggested.
>
>> + }
>> +
>> + qphy->phy_reset = devm_reset_control_get(&pdev->dev, "phy");
>> + if (IS_ERR(qphy->phy_reset)) {
>> + dev_err(dev, "failed to get phy core reset\n");
>> + return PTR_ERR(qphy->phy_reset);
>> + }
>> +
>> + qphy->vdd_phy = devm_regulator_get(dev, "vdd-phy");
>> + if (IS_ERR(qphy->vdd_phy)) {
>> + dev_err(dev, "unable to get vdd-phy supply\n");
>> + return PTR_ERR(qphy->vdd_phy);
>> + }
>> +
>> + qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
>> + if (IS_ERR(qphy->vdda_pll)) {
>> + dev_err(dev, "unable to get vdda-pll supply\n");
>> + return PTR_ERR(qphy->vdda_pll);
>> + }
>> +
>> + qphy->vdda_phy_dpdm = devm_regulator_get(dev, "vdda-phy-dpdm");
>> + if (IS_ERR(qphy->vdda_phy_dpdm)) {
>> + dev_err(dev, "unable to get vdda-phy-dpdm supply\n");
>> + return PTR_ERR(qphy->vdda_phy_dpdm);
>> + }
>> +
>> + /* Get the specific init parameters of QMP phy */
>> + match = of_match_node(qusb2_phy_of_match_table, dev->of_node);
>> + qphy->cfg = match->data;
>
> Use of_device_get_match_data() instead.
Okay.
>
>> +
>> + qphy->tcsr = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "qcom,tcsr-syscon");
>> + if (IS_ERR(qphy->tcsr)) {
>> + dev_dbg(dev, "Failed to lookup TCSR regmap\n");
>> + qphy->tcsr = NULL;
>> + }
>> +
>> + generic_phy = devm_phy_create(dev, NULL, &qusb2_phy_gen_ops);
>> + if (IS_ERR(generic_phy)) {
>> + ret = PTR_ERR(generic_phy);
>> + dev_err(dev, "%s: failed to create phy %d\n", __func__, ret);
>> + return ret;
>> + }
>> + qphy->phy = generic_phy;
>> +
>> + dev_set_drvdata(dev, qphy);
>> + phy_set_drvdata(generic_phy, qphy);
>> +
>> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> + if (IS_ERR(phy_provider)) {
>> + ret = PTR_ERR(phy_provider);
>> + dev_err(dev, "%s: failed to register phy %d\n", __func__, ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
Thanks for a thorough review. Will respin the patch soon.
regards
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
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