* Re: [PATCH] Documentation: dt: reset: Revise typos in TI syscon reset example
From: Suman Anna @ 2017-01-11 21:49 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Andrew Davis, Philipp Zabel,
Santosh Shilimkar, linux-arm-kernel
In-Reply-To: <20170111211520.w3sy5pkvjydiwvcs@rob-hp-laptop>
On 01/11/2017 03:15 PM, Rob Herring wrote:
> On Mon, Jan 09, 2017 at 01:28:14PM -0600, Suman Anna wrote:
>> Fix couple of typos in the example given in the TI syscon reset
>> binding. The ti,reset-bits used for DSP0 are corrected to match
>> the values that will be used in the actual DT node.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> Hi Philipp,
>>
>> This is the Documentation part fix that goes along with
>> the ti-syscon-reset fix that you have on your next branch.
>> I will be submitting the DT nodes very soon
>>
>> regards
>> Suman
>>
>> Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
>> index 164c7f34c451..21ba739b162e 100644
>> --- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
>> +++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
>> @@ -63,7 +63,7 @@ Example:
>> --------
>> The following example demonstrates a syscon node, the reset controller node
>> using the syscon node, and a consumer (a DSP device) on the TI Keystone 2
>> -Edison SoC.
>> +66AK2E SoC.
>>
>> / {
>> soc {
>> @@ -71,13 +71,13 @@ Edison SoC.
>> compatible = "syscon", "simple-mfd";
>> reg = <0x02350000 0x1000>;
>>
>> - pscrst: psc-reset {
>> + pscrst: psc-reset-controller {
>
> Really, this should be just 'reset-controller'.
Thanks Rob, I will fix this patch and the DTS patches as well.
regards
Suman
^ permalink raw reply
* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
From: Yegor Yefremov @ 2017-01-11 21:50 UTC (permalink / raw)
To: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Andrey Skvortsov, hs-ynQEQJNshbs, Yegor Yefremov
In-Reply-To: <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Wed, Jan 11, 2017 at 3:05 PM, <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> This is an attempt to revive DT support for TI HECC that was started in 2015.
>
> I haven't changed much because not all questions could be fully answered:
>
> * Should HECC use "am3505" as compatible?
I mean "ti,am3505-hecc"
> * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
First submission and related comments:
http://comments.gmane.org/gmane.linux.can/8616
Yegor
--
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 0/2] usb: gadget: s3c2410: add device tree support
From: Sergio Prado @ 2017-01-11 22:02 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, balbi, linux-usb, devicetree,
linux-kernel
Cc: Sergio Prado
This series adds support for configuring Samsung's s3c2410 and
compatible USB device controller via device tree.
Tested on FriendlyARM mini2440, based on s3c2440 SoC.
Changes since v1:
- change the USB bus clock name to "uclk" to better reflect the name
used in the datasheet
- improve samsung,vbus-gpio optional property description
- specify the active state in the GPIOs properties
Sergio Prado (2):
dt-bindings: usb: add DT binding for s3c2410 USB device controller
usb: gadget: s3c2410: allow probing from device tree
.../devicetree/bindings/usb/s3c2410-usb.txt | 28 ++++
drivers/usb/gadget/udc/s3c2410_udc.c | 144 +++++++++++++++++----
drivers/usb/gadget/udc/s3c2410_udc.h | 4 +
3 files changed, 152 insertions(+), 24 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller
From: Sergio Prado @ 2017-01-11 22:02 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
balbi-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Sergio Prado
In-Reply-To: <1484172150-32075-1-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
Adds the device tree bindings description for Samsung S3C2410 and
compatible USB device controller.
Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
---
.../devicetree/bindings/usb/s3c2410-usb.txt | 28 ++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
index e45b38ce2986..28353eea31fd 100644
--- a/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
+++ b/Documentation/devicetree/bindings/usb/s3c2410-usb.txt
@@ -20,3 +20,31 @@ usb0: ohci@49000000 {
clocks = <&clocks UCLK>, <&clocks HCLK_USBH>;
clock-names = "usb-bus-host", "usb-host";
};
+
+Samsung S3C2410 and compatible USB device controller
+
+Required properties:
+ - compatible: Should be one of the following
+ "samsung,s3c2410-udc"
+ "samsung,s3c2440-udc"
+ - reg: address and length of the controller memory mapped region
+ - interrupts: interrupt number for the USB device controller
+ - clocks: Should reference the bus and host clocks
+ - clock-names: Should contain two strings
+ "uclk" for the USB bus clock
+ "usb-device" for the USB device clock
+
+Optional properties:
+ - samsung,vbus-gpio: specifies a gpio that allows to detect whether
+ vbus is present - USB is connected (active high, input).
+ - samsung,pullup-gpio: If present, specifies a gpio to control the
+ USB D+ pullup (active high, output).
+
+usb1: udc@52000000 {
+ compatible = "samsung,s3c2440-udc";
+ reg = <0x52000000 0x100000>;
+ interrupts = <0 0 25 3>;
+ clocks = <&clocks UCLK>, <&clocks HCLK_USBD>;
+ clock-names = "usb-bus-gadget", "usb-device";
+ samsung,pullup-gpio = <&gpc 5 GPIO_ACTIVE_HIGH>;
+};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 2/2] usb: gadget: s3c2410: allow probing from device tree
From: Sergio Prado @ 2017-01-11 22:02 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
balbi-DgEjT+Ai2ygdnm+yROfE0A, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Sergio Prado
In-Reply-To: <1484172150-32075-1-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
Allows configuring Samsung's s3c2410 and compatible USB device
controller using a devicetree.
Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
---
drivers/usb/gadget/udc/s3c2410_udc.c | 144 +++++++++++++++++++++++++++++------
drivers/usb/gadget/udc/s3c2410_udc.h | 4 +
2 files changed, 124 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.c b/drivers/usb/gadget/udc/s3c2410_udc.c
index 4643a01262b4..165a510b1971 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.c
+++ b/drivers/usb/gadget/udc/s3c2410_udc.c
@@ -30,6 +30,9 @@
#include <linux/gpio.h>
#include <linux/prefetch.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
@@ -55,6 +58,18 @@
#define DRIVER_AUTHOR "Herbert Pötzl <herbert-dBHVzrDq9nF4Lj/PQRBjDg@public.gmane.org>, " \
"Arnaud Patard <arnaud.patard-dQbF7i+pzddAfugRpC6u6w@public.gmane.org>"
+struct s3c2410_udc_drv_data {
+ int ep_fifo_size;
+};
+
+static const struct s3c2410_udc_drv_data s3c2410_udc_2410_drv_data = {
+ .ep_fifo_size = EP_FIFO_SIZE,
+};
+
+static const struct s3c2410_udc_drv_data s3c2410_udc_2440_drv_data = {
+ .ep_fifo_size = S3C2440_EP_FIFO_SIZE,
+};
+
static const char gadget_name[] = "s3c2410_udc";
static const char driver_desc[] = DRIVER_DESC;
@@ -62,8 +77,6 @@
static struct clk *udc_clock;
static struct clk *usb_bus_clock;
static void __iomem *base_addr;
-static u64 rsrc_start;
-static u64 rsrc_len;
static struct dentry *s3c2410_udc_debugfs_root;
static inline u32 udc_read(u32 reg)
@@ -997,7 +1010,7 @@ static irqreturn_t s3c2410_udc_irq(int dummy, void *_dev)
}
}
- dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", IRQ_USBD);
+ dprintk(DEBUG_VERBOSE, "irq: %d s3c2410_udc_done.\n", dev->irq);
/* Restore old index */
udc_write(idx, S3C2410_UDC_INDEX_REG);
@@ -1757,6 +1770,49 @@ static int s3c2410_udc_stop(struct usb_gadget *g)
};
+static int s3c2410_udc_probe_dt(struct s3c2410_udc *udc)
+{
+ const struct s3c2410_udc_drv_data *drvdata;
+ struct platform_device *pdev = udc->pdev;
+ struct s3c2410_udc_mach_info *pdata;
+ int gpio;
+
+ drvdata = of_device_get_match_data(&pdev->dev);
+
+ if (drvdata)
+ udc->ep_fifo_size = drvdata->ep_fifo_size;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ gpio = of_get_named_gpio(pdev->dev.of_node, "samsung,vbus-gpio", 0);
+ if (gpio_is_valid(gpio))
+ pdata->vbus_pin = gpio;
+
+ gpio = of_get_named_gpio(pdev->dev.of_node, "samsung,pullup-gpio", 0);
+ if (gpio_is_valid(gpio))
+ pdata->pullup_pin = gpio;
+
+ pdev->dev.platform_data = pdata;
+
+ return 0;
+}
+
+static int s3c2410_udc_probe_pdata(struct s3c2410_udc *udc)
+{
+ const struct s3c2410_udc_drv_data *drvdata;
+ struct platform_device *pdev = udc->pdev;
+
+ drvdata = (struct s3c2410_udc_drv_data *)
+ platform_get_device_id(pdev)->driver_data;
+
+ if (drvdata)
+ udc->ep_fifo_size = drvdata->ep_fifo_size;
+
+ return 0;
+}
+
/*
* probe - binds to the platform device
*/
@@ -1769,7 +1825,17 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
dev_dbg(dev, "%s()\n", __func__);
- usb_bus_clock = clk_get(NULL, "usb-bus-gadget");
+ udc->pdev = pdev;
+
+ if (pdev->dev.of_node)
+ retval = s3c2410_udc_probe_dt(udc);
+ else
+ retval = s3c2410_udc_probe_pdata(udc);
+
+ if (retval)
+ return retval;
+
+ usb_bus_clock = clk_get(NULL, "uclk");
if (IS_ERR(usb_bus_clock)) {
dev_err(dev, "failed to get usb bus clock source\n");
return PTR_ERR(usb_bus_clock);
@@ -1789,24 +1855,27 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
dev_dbg(dev, "got and enabled clocks\n");
- if (strncmp(pdev->name, "s3c2440", 7) == 0) {
- dev_info(dev, "S3C2440: increasing FIFO to 128 bytes\n");
- memory.ep[1].fifo_size = S3C2440_EP_FIFO_SIZE;
- memory.ep[2].fifo_size = S3C2440_EP_FIFO_SIZE;
- memory.ep[3].fifo_size = S3C2440_EP_FIFO_SIZE;
- memory.ep[4].fifo_size = S3C2440_EP_FIFO_SIZE;
+ if (udc->ep_fifo_size) {
+ dev_info(dev, "setting FIFO to %d bytes\n", udc->ep_fifo_size);
+ memory.ep[1].fifo_size = udc->ep_fifo_size;
+ memory.ep[2].fifo_size = udc->ep_fifo_size;
+ memory.ep[3].fifo_size = udc->ep_fifo_size;
+ memory.ep[4].fifo_size = udc->ep_fifo_size;
}
spin_lock_init(&udc->lock);
udc_info = dev_get_platdata(&pdev->dev);
- rsrc_start = S3C2410_PA_USBDEV;
- rsrc_len = S3C24XX_SZ_USBDEV;
+ udc->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!udc->mem) {
+ dev_err(dev, "failed to get I/O memory region\n");
+ return -ENOENT;
+ }
- if (!request_mem_region(rsrc_start, rsrc_len, gadget_name))
+ if (!request_mem_region(udc->mem->start, resource_size(udc->mem), gadget_name))
return -EBUSY;
- base_addr = ioremap(rsrc_start, rsrc_len);
+ base_addr = ioremap(udc->mem->start, resource_size(udc->mem));
if (!base_addr) {
retval = -ENOMEM;
goto err_mem;
@@ -1818,17 +1887,24 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
s3c2410_udc_disable(udc);
s3c2410_udc_reinit(udc);
+ udc->irq = platform_get_irq(pdev, 0);
+ if (udc->irq == 0) {
+ dev_err(dev, "failed to get interrupt\n");
+ retval = -EINVAL;
+ goto err_map;
+ }
+
/* irq setup after old hardware state is cleaned up */
- retval = request_irq(IRQ_USBD, s3c2410_udc_irq,
+ retval = request_irq(udc->irq, s3c2410_udc_irq,
0, gadget_name, udc);
if (retval != 0) {
- dev_err(dev, "cannot get irq %i, err %d\n", IRQ_USBD, retval);
+ dev_err(dev, "cannot get irq %i, err %d\n", udc->irq, retval);
retval = -EBUSY;
goto err_map;
}
- dev_dbg(dev, "got irq %i\n", IRQ_USBD);
+ dev_dbg(dev, "got irq %i\n", udc->irq);
if (udc_info && udc_info->vbus_pin > 0) {
retval = gpio_request(udc_info->vbus_pin, "udc vbus");
@@ -1899,11 +1975,11 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
if (udc_info && udc_info->vbus_pin > 0)
gpio_free(udc_info->vbus_pin);
err_int:
- free_irq(IRQ_USBD, udc);
+ free_irq(udc->irq, udc);
err_map:
iounmap(base_addr);
err_mem:
- release_mem_region(rsrc_start, rsrc_len);
+ release_mem_region(udc->mem->start, resource_size(udc->mem));
return retval;
}
@@ -1933,10 +2009,10 @@ static int s3c2410_udc_remove(struct platform_device *pdev)
free_irq(irq, udc);
}
- free_irq(IRQ_USBD, udc);
+ free_irq(udc->irq, udc);
iounmap(base_addr);
- release_mem_region(rsrc_start, rsrc_len);
+ release_mem_region(udc->mem->start, resource_size(udc->mem));
if (!IS_ERR(udc_clock) && udc_clock != NULL) {
clk_disable_unprepare(udc_clock);
@@ -1974,16 +2050,36 @@ static int s3c2410_udc_resume(struct platform_device *pdev)
#define s3c2410_udc_resume NULL
#endif
+static const struct of_device_id s3c_udc_dt_ids[] = {
+ {
+ .compatible = "samsung,s3c2410-udc",
+ .data = &s3c2410_udc_2410_drv_data,
+ },
+ {
+ .compatible = "samsung,s3c2440-udc",
+ .data = &s3c2410_udc_2440_drv_data,
+ },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, s3c_udc_dt_ids);
+
static const struct platform_device_id s3c_udc_ids[] = {
- { "s3c2410-usbgadget", },
- { "s3c2440-usbgadget", },
- { }
+ {
+ .name = "s3c2410-usbgadget",
+ .driver_data = (kernel_ulong_t) &s3c2410_udc_2410_drv_data,
+ },
+ {
+ .name = "s3c2440-usbgadget",
+ .driver_data = (kernel_ulong_t) &s3c2410_udc_2440_drv_data,
+ },
+ { /* sentinel */},
};
MODULE_DEVICE_TABLE(platform, s3c_udc_ids);
static struct platform_driver udc_driver_24x0 = {
.driver = {
.name = "s3c24x0-usbgadget",
+ .of_match_table = s3c_udc_dt_ids,
},
.probe = s3c2410_udc_probe,
.remove = s3c2410_udc_remove,
diff --git a/drivers/usb/gadget/udc/s3c2410_udc.h b/drivers/usb/gadget/udc/s3c2410_udc.h
index 93bf225f1969..8fdbe77a804d 100644
--- a/drivers/usb/gadget/udc/s3c2410_udc.h
+++ b/drivers/usb/gadget/udc/s3c2410_udc.h
@@ -94,6 +94,10 @@ struct s3c2410_udc {
unsigned req_pending : 1;
u8 vbus;
struct dentry *regs_info;
+ struct platform_device *pdev;
+ struct resource *mem;
+ int irq;
+ unsigned int ep_fifo_size;
};
#define to_s3c2410(g) (container_of((g), struct s3c2410_udc, gadget))
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] dt: bindings: Add support for CSI1 bus
From: Pavel Machek @ 2017-01-11 22:06 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rob Herring, devicetree, ivo.g.dimitrov.75, sre, pali.rohar,
linux-media
In-Reply-To: <20170104085420.GN3958@valkosipuli.retiisi.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2471 bytes --]
Hi!
> Thanks for the review.
>
> On Tue, Jan 03, 2017 at 02:38:54PM -0600, Rob Herring wrote:
> > On Wed, Dec 28, 2016 at 07:30:36PM +0100, Pavel Machek wrote:
> > > From: Sakari Ailus <sakari.ailus@iki.fi>
> > >
> > > In the vast majority of cases the bus type is known to the driver(s)
> > > since a receiver or transmitter can only support a single one. There
> > > are cases however where different options are possible.
> >
> > What cases specifically?
>
> The existing V4L2 OF support tries to figure out the bus type and parse the
> bus parameters based on that. This does not scale too well as there are
> multiple serial busses that share common properties.
>
> Some hardware also supports multiple types of busses on the same interfaces.
Ok, I'll include that in the changelog.
> > As in MIPI CSI2?
>
> Yeah, I guess it'd make sense to make this explicit.
Ok.
> > > should be the combined length of data-lanes and clock-lanes properties.
> > > - If the lane-polarities property is omitted, the value must be interpreted
> > > - as 0 (normal). This property is valid for serial busses only.
> >
> > Why is this removed?
>
> Must have been by mistake. :-)
Fixed.
> > > -
> > > +- clock-inv: Clock or strobe signal inversion.
> > > + Possible values: 0 -- not inverted; 1 -- inverted
> >
> > "invert" assumes I know what is normal and I do not. Define what is
> > "normal" and name the property the opposite of that. If normal is data
> > shifted on clock rising edge, then call the the property
> > "clock-shift-falling-edge" for example..
>
> The hardware documentation says this is the "strobe/clock inversion control
> signal". I'm not entirely sure whether this is just signal polarity (it's a
> differential signal) or inversion of an internal signal of the CCP2 block.
>
> It might make sense to make this a private property for the OMAP 3 ISP
> instead. If it's seen elsewhere, then think about it again. I doubt it
> would, as CCP2 is an old bus that's used on Nokia N9, N950 and N900.
>
> As strobe is included, I'd add that to the name. Say,
> "ti,clock-strobe-inv".
Hmm. N900 does not use inversion. Would it make sense to simply
hardcode it to "not-inverted" for now?
Device tree changes are PITA :-(.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH] pinctrl: core: Fix regression caused by delayed work for hogs
From: Tony Lindgren @ 2017-01-11 22:13 UTC (permalink / raw)
To: Linus Walleij
Cc: Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap,
Gary Bisson
Commit df61b366af26 ("pinctrl: core: Use delayed work for hogs") caused a
regression at least with sh-pfc that is also a GPIO controller as
noted by Geert Uytterhoeven <geert@linux-m68k.org>.
As the original pinctrl_register() has issues calling pin controller
driver functions early before the controller has finished registering,
we can't just revert commit df61b366af26. That would break the drivers
using GENERIC_PINCTRL_GROUPS or GENERIC_PINMUX_FUNCTIONS.
So let's fix the issue with the following steps as a single patch:
1. Revert the late_init parts of commit df61b366af26.
The late_init clearly won't work and we have to just give up
on fixing pinctrl_register() for GENERIC_PINCTRL_GROUPS and
GENERIC_PINMUX_FUNCTIONS.
2. Split pinctrl_register() into two parts
By splitting pinctrl_register() into pinctrl_init_controller()
and pinctrl_create_and_start() we have better control over when
it's safe to call pinctrl_create().
3. Introduce a new pinctrl_register_and_init() function
As suggested by Linus Walleij <linus.walleij@linaro.org>, we
can just introduce a new function for the controllers that need
pinctrl_create() called later.
4. Convert the four known problem cases to use new function
Let's convert pinctrl-imx, pinctrl-single, sh-pfc and ti-iodelay
to use the new function to fix the issues. The rest of the drivers
can be converted later. Let's also update Documentation/pinctrl.txt
accordingly because of the known issues with pinctrl_register().
Fixes: df61b366af26 ("pinctrl: core: Use delayed work for hogs")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Gary Bisson <gary.bisson@boundarydevices.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Documentation/pinctrl.txt | 4 +-
drivers/pinctrl/core.c | 201 ++++++++++++++++++++++----------
drivers/pinctrl/core.h | 2 -
drivers/pinctrl/freescale/pinctrl-imx.c | 8 +-
drivers/pinctrl/pinctrl-single.c | 5 +-
drivers/pinctrl/sh-pfc/pinctrl.c | 4 +-
drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 5 +-
include/linux/pinctrl/pinctrl.h | 15 +++
8 files changed, 165 insertions(+), 79 deletions(-)
diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
--- a/Documentation/pinctrl.txt
+++ b/Documentation/pinctrl.txt
@@ -79,9 +79,7 @@ int __init foo_probe(void)
{
struct pinctrl_dev *pctl;
- pctl = pinctrl_register(&foo_desc, <PARENT>, NULL);
- if (!pctl)
- pr_err("could not register foo pin driver\n");
+ return pinctrl_register_and_init(&foo_desc, <PARENT>, NULL, &pctl);
}
To enable the pinctrl subsystem and the subgroups for PINMUX and PINCONF and
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1905,59 +1905,14 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
}
/**
- * pinctrl_late_init() - finish pin controller device registration
- * @work: work struct
- */
-static void pinctrl_late_init(struct work_struct *work)
-{
- struct pinctrl_dev *pctldev;
-
- pctldev = container_of(work, struct pinctrl_dev, late_init.work);
-
- /*
- * If the pin controller does NOT have hogs, this will report an
- * error and we skip over this entire branch. This is why we can
- * call this function directly when we do not have hogs on the
- * device.
- */
- pctldev->p = create_pinctrl(pctldev->dev, pctldev);
- if (!IS_ERR(pctldev->p)) {
- kref_get(&pctldev->p->users);
- pctldev->hog_default =
- pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
- if (IS_ERR(pctldev->hog_default)) {
- dev_dbg(pctldev->dev,
- "failed to lookup the default state\n");
- } else {
- if (pinctrl_select_state(pctldev->p,
- pctldev->hog_default))
- dev_err(pctldev->dev,
- "failed to select default state\n");
- }
-
- pctldev->hog_sleep =
- pinctrl_lookup_state(pctldev->p,
- PINCTRL_STATE_SLEEP);
- if (IS_ERR(pctldev->hog_sleep))
- dev_dbg(pctldev->dev,
- "failed to lookup the sleep state\n");
- }
-
- mutex_lock(&pinctrldev_list_mutex);
- list_add_tail(&pctldev->node, &pinctrldev_list);
- mutex_unlock(&pinctrldev_list_mutex);
-
- pinctrl_init_device_debugfs(pctldev);
-}
-
-/**
- * pinctrl_register() - register a pin controller device
+ * pinctrl_init_controller() - init a pin controller device
* @pctldesc: descriptor for this pin controller
* @dev: parent device for this pin controller
* @driver_data: private pin controller data for this pin controller
*/
-struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
- struct device *dev, void *driver_data)
+struct pinctrl_dev *pinctrl_init_controller(struct pinctrl_desc *pctldesc,
+ struct device *dev,
+ void *driver_data)
{
struct pinctrl_dev *pctldev;
int ret;
@@ -1983,7 +1938,6 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
INIT_RADIX_TREE(&pctldev->pin_function_tree, GFP_KERNEL);
#endif
INIT_LIST_HEAD(&pctldev->gpio_ranges);
- INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
pctldev->dev = dev;
mutex_init(&pctldev->mutex);
@@ -2018,17 +1972,6 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
goto out_err;
}
- /*
- * If the device has hogs we want the probe() function of the driver
- * to complete before we go in and hog them and add the pin controller
- * to the list of controllers. If it has no hogs, we can just complete
- * the registration immediately.
- */
- if (pinctrl_dt_has_hogs(pctldev))
- schedule_delayed_work(&pctldev->late_init, 0);
- else
- pinctrl_late_init(&pctldev->late_init.work);
-
return pctldev;
out_err:
@@ -2036,8 +1979,107 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
kfree(pctldev);
return ERR_PTR(ret);
}
+
+static int pinctrl_create_and_start(struct pinctrl_dev *pctldev)
+{
+ pctldev->p = create_pinctrl(pctldev->dev, pctldev);
+ if (!IS_ERR(pctldev->p)) {
+ kref_get(&pctldev->p->users);
+ pctldev->hog_default =
+ pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(pctldev->hog_default)) {
+ dev_dbg(pctldev->dev,
+ "failed to lookup the default state\n");
+ } else {
+ if (pinctrl_select_state(pctldev->p,
+ pctldev->hog_default))
+ dev_err(pctldev->dev,
+ "failed to select default state\n");
+ }
+
+ pctldev->hog_sleep =
+ pinctrl_lookup_state(pctldev->p,
+ PINCTRL_STATE_SLEEP);
+ if (IS_ERR(pctldev->hog_sleep))
+ dev_dbg(pctldev->dev,
+ "failed to lookup the sleep state\n");
+ }
+
+ mutex_lock(&pinctrldev_list_mutex);
+ list_add_tail(&pctldev->node, &pinctrldev_list);
+ mutex_unlock(&pinctrldev_list_mutex);
+
+ pinctrl_init_device_debugfs(pctldev);
+
+ return 0;
+}
+
+/**
+ * pinctrl_register() - register a pin controller device
+ * @pctldesc: descriptor for this pin controller
+ * @dev: parent device for this pin controller
+ * @driver_data: private pin controller data for this pin controller
+ *
+ * Note that pinctrl_register() is known to have problems as the pin
+ * controller driver functions are called before the driver has a
+ * struct pinctrl_dev handle. To avoid issues later on, please use the
+ * new pinctrl_register_and_init() below instead.
+ */
+struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
+ struct device *dev, void *driver_data)
+{
+ struct pinctrl_dev *pctldev;
+ int error;
+
+ pctldev = pinctrl_init_controller(pctldesc, dev, driver_data);
+ if (IS_ERR(pctldev))
+ return pctldev;
+
+ error = pinctrl_create_and_start(pctldev);
+ if (error) {
+ mutex_destroy(&pctldev->mutex);
+ kfree(pctldev);
+
+ return ERR_PTR(error);
+ }
+
+ return pctldev;
+
+}
EXPORT_SYMBOL_GPL(pinctrl_register);
+int pinctrl_register_and_init(struct pinctrl_desc *pctldesc,
+ struct device *dev, void *driver_data,
+ struct pinctrl_dev **pctldev)
+{
+ struct pinctrl_dev *p;
+ int error;
+
+ p = pinctrl_init_controller(pctldesc, dev, driver_data);
+ if (IS_ERR(p))
+ return PTR_ERR(p);
+
+ /*
+ * We have pinctrl_start() call functions in the pin controller
+ * driver with create_pinctrl() for at least dt_node_to_map(). So
+ * let's make sure pctldev is properly initialized for the
+ * pin controller driver before we do anything.
+ */
+ *pctldev = p;
+
+ error = pinctrl_create_and_start(p);
+ if (error) {
+ mutex_destroy(&p->mutex);
+ kfree(p);
+ *pctldev = NULL;
+
+ return error;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_register_and_init);
+
/**
* pinctrl_unregister() - unregister pinmux
* @pctldev: pin controller to unregister
@@ -2052,7 +2094,6 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
if (pctldev == NULL)
return;
- cancel_delayed_work_sync(&pctldev->late_init);
mutex_lock(&pctldev->mutex);
pinctrl_remove_device_debugfs(pctldev);
mutex_unlock(&pctldev->mutex);
@@ -2134,6 +2175,42 @@ struct pinctrl_dev *devm_pinctrl_register(struct device *dev,
EXPORT_SYMBOL_GPL(devm_pinctrl_register);
/**
+ * devm_pinctrl_register_and_init() - Resource managed pinctrl register and init
+ * @dev: parent device for this pin controller
+ * @pctldesc: descriptor for this pin controller
+ * @driver_data: private pin controller data for this pin controller
+ *
+ * Returns an error pointer if pincontrol register failed. Otherwise
+ * it returns valid pinctrl handle.
+ *
+ * The pinctrl device will be automatically released when the device is unbound.
+ */
+int devm_pinctrl_register_and_init(struct device *dev,
+ struct pinctrl_desc *pctldesc,
+ void *driver_data,
+ struct pinctrl_dev **pctldev)
+{
+ struct pinctrl_dev **ptr;
+ int error;
+
+ ptr = devres_alloc(devm_pinctrl_dev_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ error = pinctrl_register_and_init(pctldesc, dev, driver_data, pctldev);
+ if (error) {
+ devres_free(ptr);
+ return error;
+ }
+
+ *ptr = *pctldev;
+ devres_add(dev, ptr);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_pinctrl_register_and_init);
+
+/**
* devm_pinctrl_unregister() - Resource managed version of pinctrl_unregister().
* @dev: device for which which resource was allocated
* @pctldev: the pinctrl device to unregister.
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -37,7 +37,6 @@ struct pinctrl_gpio_range;
* @p: result of pinctrl_get() for this device
* @hog_default: default state for pins hogged by this device
* @hog_sleep: sleep state for pins hogged by this device
- * @late_init: delayed work for pin controller to finish registration
* @mutex: mutex taken on each pin controller specific action
* @device_root: debugfs root for this device
*/
@@ -60,7 +59,6 @@ struct pinctrl_dev {
struct pinctrl *p;
struct pinctrl_state *hog_default;
struct pinctrl_state *hog_sleep;
- struct delayed_work late_init;
struct mutex mutex;
#ifdef CONFIG_DEBUG_FS
struct dentry *device_root;
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -774,11 +774,11 @@ int imx_pinctrl_probe(struct platform_device *pdev,
ipctl->info = info;
ipctl->dev = info->dev;
platform_set_drvdata(pdev, ipctl);
- ipctl->pctl = devm_pinctrl_register(&pdev->dev,
- imx_pinctrl_desc, ipctl);
- if (IS_ERR(ipctl->pctl)) {
+ ret = devm_pinctrl_register_and_init(&pdev->dev,
+ imx_pinctrl_desc, ipctl,
+ &ipctl->pctl);
+ if (ret) {
dev_err(&pdev->dev, "could not register IMX pinctrl driver\n");
- ret = PTR_ERR(ipctl->pctl);
goto free;
}
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1747,10 +1747,9 @@ static int pcs_probe(struct platform_device *pdev)
if (ret < 0)
goto free;
- pcs->pctl = pinctrl_register(&pcs->desc, pcs->dev, pcs);
- if (IS_ERR(pcs->pctl)) {
+ ret = pinctrl_register_and_init(&pcs->desc, pcs->dev, pcs, &pcs->pctl);
+ if (ret) {
dev_err(pcs->dev, "could not register single pinctrl driver\n");
- ret = PTR_ERR(pcs->pctl);
goto free;
}
diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -816,6 +816,6 @@ int sh_pfc_register_pinctrl(struct sh_pfc *pfc)
pmx->pctl_desc.pins = pmx->pins;
pmx->pctl_desc.npins = pfc->info->nr_pins;
- pmx->pctl = devm_pinctrl_register(pfc->dev, &pmx->pctl_desc, pmx);
- return PTR_ERR_OR_ZERO(pmx->pctl);
+ return devm_pinctrl_register_and_init(pfc->dev, &pmx->pctl_desc, pmx,
+ &pmx->pctl);
}
diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
--- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
+++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
@@ -891,10 +891,9 @@ static int ti_iodelay_probe(struct platform_device *pdev)
iod->desc.name = dev_name(dev);
iod->desc.owner = THIS_MODULE;
- iod->pctl = pinctrl_register(&iod->desc, dev, iod);
- if (!iod->pctl) {
+ ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl);
+ if (ret) {
dev_err(dev, "Failed to register pinctrl\n");
- ret = -ENODEV;
goto exit_out;
}
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -141,12 +141,27 @@ struct pinctrl_desc {
};
/* External interface to pin controller */
+
+extern int pinctrl_register_and_init(struct pinctrl_desc *pctldesc,
+ struct device *dev, void *driver_data,
+ struct pinctrl_dev **pctldev);
+
+/* Please use pinctrl_register_and_init() instead */
extern struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
struct device *dev, void *driver_data);
+
extern void pinctrl_unregister(struct pinctrl_dev *pctldev);
+
+extern int devm_pinctrl_register_and_init(struct device *dev,
+ struct pinctrl_desc *pctldesc,
+ void *driver_data,
+ struct pinctrl_dev **pctldev);
+
+/* Please use devm_pinctrl_register_and_init() instead */
extern struct pinctrl_dev *devm_pinctrl_register(struct device *dev,
struct pinctrl_desc *pctldesc,
void *driver_data);
+
extern void devm_pinctrl_unregister(struct device *dev,
struct pinctrl_dev *pctldev);
--
2.11.0
^ permalink raw reply
* Re: [PATCH] dt: bindings: Add support for CSI1 bus
From: Sakari Ailus @ 2017-01-11 22:19 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, devicetree, ivo.g.dimitrov.75, sre, pali.rohar,
linux-media
In-Reply-To: <20170111220648.GE29366@amd>
Hi Pavel,
On Wed, Jan 11, 2017 at 11:06:48PM +0100, Pavel Machek wrote:
> > > > +- clock-inv: Clock or strobe signal inversion.
> > > > + Possible values: 0 -- not inverted; 1 -- inverted
> > >
> > > "invert" assumes I know what is normal and I do not. Define what is
> > > "normal" and name the property the opposite of that. If normal is data
> > > shifted on clock rising edge, then call the the property
> > > "clock-shift-falling-edge" for example..
> >
> > The hardware documentation says this is the "strobe/clock inversion control
> > signal". I'm not entirely sure whether this is just signal polarity (it's a
> > differential signal) or inversion of an internal signal of the CCP2 block.
> >
> > It might make sense to make this a private property for the OMAP 3 ISP
> > instead. If it's seen elsewhere, then think about it again. I doubt it
> > would, as CCP2 is an old bus that's used on Nokia N9, N950 and N900.
> >
> > As strobe is included, I'd add that to the name. Say,
> > "ti,clock-strobe-inv".
>
> Hmm. N900 does not use inversion. Would it make sense to simply
> hardcode it to "not-inverted" for now?
>
> Device tree changes are PITA :-(.
I can't remember what's used for the N9 secondary camera. As you said, we
can always add it if needed though. So works for me.
--
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply
* [PATCHv2] dt: bindings: Add support for CSI1 bus
From: Pavel Machek @ 2017-01-11 22:53 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-X3B1VOXEql0, sre-DgEjT+Ai2ygdnm+yROfE0A,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161228183036.GA13139@amd>
[-- Attachment #1: Type: text/plain, Size: 2368 bytes --]
From: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
In the vast majority of cases the bus type is known to the driver(s)
since a receiver or transmitter can only support a single one. There
are cases however where different options are possible.
The existing V4L2 OF support tries to figure out the bus type and
parse the bus parameters based on that. This does not scale too well
as there are multiple serial busses that share common properties.
Some hardware also supports multiple types of busses on the same
interfaces.
Document the CSI1/CCP2 property strobe. It signifies the clock or
strobe mode.
Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 9cd2a36..08c4498 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -76,6 +76,11 @@ Optional endpoint properties
mode horizontal and vertical synchronization signals are provided to the
slave device (data source) by the master device (data sink). In the master
mode the data source device is also the source of the synchronization signals.
+- bus-type: data bus type. Possible values are:
+ 0 - MIPI CSI2
+ 1 - parallel / Bt656
+ 2 - MIPI CSI1
+ 3 - CCP2
- bus-width: number of data lines actively used, valid for the parallel busses.
- data-shift: on the parallel data busses, if bus-width is used to specify the
number of data lines, data-shift can be used to specify which data lines are
@@ -112,7 +117,8 @@ Optional endpoint properties
should be the combined length of data-lanes and clock-lanes properties.
If the lane-polarities property is omitted, the value must be interpreted
as 0 (normal). This property is valid for serial busses only.
-
+- strobe: Whether the clock signal is used as clock or strobe. Used
+ with CCP2, for instance.
Example
-------
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related
* Re: [PATCH 4/4] mmc: pwrseq-simple: add disable-post-power-on option
From: Ulf Hansson @ 2017-01-11 22:55 UTC (permalink / raw)
To: Matt Ranostay
Cc: Linus Walleij, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tony Lindgren,
Liam Breck
In-Reply-To: <CAJ_EiSQYz2aEGiCv2npxmby0nHwmJJyR_jzY5wJOK98zp6Bvzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 9 January 2017 at 05:53, Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org> wrote:
> On Fri, Dec 30, 2016 at 12:05 AM, Linus Walleij
> <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Mon, Dec 19, 2016 at 1:01 AM, Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org> wrote:
>>
>>> * SD8787 has a "powerdown" line, and CW1200 has a "powerup" line.. I
>>> know this is a simple logic inversion.
>>
>> If this is a GPIO line, the GPIO subsystem can flag a line for
>> inverted logic. GPIO_ACTIVE_LOW from device tree for example.
>
> Slight ping on Ulf on this thread :).
Thanks, sorry for the delay!
>
> I do understand the inverted logic flag but that doesn't help if there
> are different logic states between various chipsets.
For cw1200 (I looked at code from an old ST-Ericsson vendor tree), the
sequence is as follows:
1) Enable clock/power to the card/chip.
2) Assert GPIO pin. I assume this also can be done before the
clock/power is enabled.
3) Wait some time (~50ms).
4) De-assert GPIO pin.
5) Wait some time (~20ms)
6) Assert GPIO pin.
7) Wait some time (~32ms).
At power off, the GPIO pin is de-asserted and of course also the
clock/power is disabled. Just to make sure we have all the relevant
logic.
Looking at mmc pwrseq simple, perhaps we can extend this to deal with
GPIOs that needs to be *toggled*, as this is not just reset GPIOs.
Then we also need to deal with the delays in-between the toggling.
Thoughts?
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 00/24] i.MX Media Driver
From: Tim Harvey @ 2017-01-11 23:14 UTC (permalink / raw)
To: Steve Longerbeam
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King - ARM Linux, mchehab, Hans Verkuil, nick,
markus.heiser, Philipp Zabel, laurent.pinchart+renesas, bparrot,
geert, Arnd Bergmann, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, Simon Horman,
Niklas Söderlund, robert.jarzmik, songjun.wu
In-Reply-To: <1483755102-24785-1-git-send-email-steve_longerbeam@mentor.com>
On Fri, Jan 6, 2017 at 6:11 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> In version 3:
>
> Changes suggested by Rob Herring <robh@kernel.org>:
>
> - prepended FIM node properties with vendor prefix "fsl,".
>
> - make mipi csi-2 receiver compatible string SoC specific:
> "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
>
> - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
>
> - removed board-specific info from the media driver binding doc. These
> were all related to sensor bindings, which already are (adv7180)
> or will be (ov564x) covered in separate binding docs. All reference
> board info not related to DT bindings has been moved to
> Documentation/media/v4l-drivers/imx.rst.
>
> - removed "_mipi" from the OV5640 compatible string.
>
> Changes suggested by Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>:
>
> Mostly cosmetic/non-functional changes which I won't list here, except
> for the following:
>
> - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
>
> - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
>
> - check/handle return code from required reg property of CSI port nodes.
>
> - check/handle return code from clk_prepare_enable().
>
> Changes suggested by Fabio Estevam <festevam@gmail.com>:
>
> - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
>
> - finally got around to passing valid IOMUX pin config values to the
> pin groups.
>
> Other changes:
>
> - removed the FIM properties that overrided the v4l2 FIM control defaults
> values. This was left-over from a requirement of a customer and is not
> necessary here.
>
> - The FIM must be explicitly enabled in the fim child node under the CSI
> port nodes, using the status property. If not enabled, FIM v4l2 controls
> will not appear in the video capture driver.
>
> - brought in additional media types patch from Philipp Zabel. Use new
> MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
>
> - brought in latest platform generic video multiplexer subdevice driver
> from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
>
> - removed imx-media-of.h, moved those prototypes into imx-media.h.
>
>
<snip>
Hi Steve,
I took a stab at testing this today on a gw51xx which has an adv7180
hooked up as follows:
- i2c3@0x20
- 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
(CSI0_IPU1)
- PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
- IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
- all three analog inputs available to off-board connector
My patch to the imx6qdl-gw51xx dtsi is:
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index afec2c7..2583d72 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -165,6 +174,52 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c3>;
status = "okay";
+
+ camera: adv7180@20 {
+ compatible = "adi,adv7180";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_adv7180>;
+ reg = <0x20>;
+ powerdown-gpio = <&gpio5 20 GPIO_ACTIVE_LOW>;
+ interrupt-parent = <&gpio5>;
+ interrupts = <23 GPIO_ACTIVE_LOW>;
+ inputs = <0x00 0x01 0x02>;
+ input-names = "ADV7180 Composite on Ain1",
+ "ADV7180 Composite on Ain2",
+ "ADV7180 Composite on Ain3";
+
+ port {
+ adv7180_to_ipu1_csi0_mux: endpoint {
+ remote-endpoint =
<&ipu1_csi0_mux_from_parallel_sensor>;
+ bus-width = <8>;
+ };
+ };
+ };
+};
+
+&ipu1_csi0_from_ipu1_csi0_mux {
+ bus-width = <8>;
+};
+
+&ipu1_csi0_mux_from_parallel_sensor {
+ remote-endpoint = <&adv7180_to_ipu1_csi0_mux>;
+ bus-width = <8>;
+};
+
+&ipu1_csi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ipu1_csi0>;
+
+ /* enable frame interval monitor on this port */
+ fim {
+ status = "okay";
+ };
};
&pcie {
@@ -236,6 +291,13 @@
&iomuxc {
imx6qdl-gw51xx {
+ pinctrl_adv7180: adv7180grp {
+ fsl,pins = <
+ MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
0x0001b0b0 /* VIDDEC_IRQ# */
+ MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
0x4001b0b0 /* VIDDEC_EN */
+ >;
+ };
+
pinctrl_enet: enetgrp {
fsl,pins = <
MX6QDL_PAD_RGMII_RXC__RGMII_RXC 0x1b030
@@ -306,6 +368,22 @@
>;
};
+ pinctrl_ipu1_csi0: ipu1csi0grp { /* IPU1_CSI0: 8-bit input */
+ fsl,pins = <
+
MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18 0x1b0b0
+
MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19 0x1b0b0
+ MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC
0x1b0b0
+ MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC
0x1b0b0
+
MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK 0x1b0b0
+ >;
+ };
+
pinctrl_pcie: pciegrp {
fsl,pins = <
MX6QDL_PAD_GPIO_0__GPIO1_IO00 0x1b0b0
On an IMX6Q I'm getting the following when the adv7180 module loads:
[ 12.862477] adv7180 2-0020: chip found @ 0x20 (21a8000.i2c)
[ 12.907767] imx-media: Registered subdev adv7180 2-0020
[ 12.907793] imx-media soc:media@0: Entity type for entity adv7180
2-0020 was not initialized!
[ 12.907867] imx-media: imx_media_create_link: adv7180 2-0020:0 ->
ipu1_csi0_mux:1
Is the warning that adv7180 was not initialized expected and or an issue?
Now that your driver is hooking into the current media framework, I'm
not at all clear on how to link and configure the media entities.
Using 'media-ctl -p' to list the entities I see:
Media controller API version 0.1.0
Media device information
------------------------
driver imx-media
model imx-media
serial
bus info
hw revision 0x0
driver version 0.0.0
Device topology
- entity 1: ipu2_csi1_mux (3 pads, 1 link)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev0
pad0: Sink
[fmt:unknown/0x0]
pad1: Sink
[fmt:unknown/0x0]
pad2: Source
[fmt:unknown/0x0]
-> "ipu2_csi1":0 []
- entity 5: ipu1_csi0_mux (3 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev1
pad0: Sink
[fmt:unknown/0x0]
pad1: Sink
[fmt:unknown/0x0]
<- "adv7180 2-0020":0 []
pad2: Source
[fmt:unknown/0x0]
-> "ipu1_csi0":0 []
- entity 9: ipu1_ic_prpenc (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev2
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_csi0":1 []
<- "ipu1_csi1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif0":0 []
-> "camif1":0 []
- entity 12: ipu1_ic_prpvf (2 pads, 8 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev3
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_csi0":1 []
<- "ipu1_csi1":1 []
<- "ipu1_smfc0":1 []
<- "ipu1_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif0":0 []
-> "camif1":0 []
-> "ipu1_ic_pp0":0 []
-> "ipu1_ic_pp1":0 []
- entity 15: ipu1_ic_pp0 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev4
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_ic_prpvf":1 []
<- "ipu1_smfc0":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif0":0 []
-> "camif1":0 []
- entity 18: ipu1_ic_pp1 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev5
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_ic_prpvf":1 []
<- "ipu1_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif0":0 []
-> "camif1":0 []
- entity 21: ipu2_ic_prpenc (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev6
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_csi0":1 []
<- "ipu2_csi1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif2":0 []
-> "camif3":0 []
- entity 24: ipu2_ic_prpvf (2 pads, 8 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev7
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_csi0":1 []
<- "ipu2_csi1":1 []
<- "ipu2_smfc0":1 []
<- "ipu2_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif2":0 []
-> "camif3":0 []
-> "ipu2_ic_pp0":0 []
-> "ipu2_ic_pp1":0 []
- entity 27: ipu2_ic_pp0 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev8
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_ic_prpvf":1 []
<- "ipu2_smfc0":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif2":0 []
-> "camif3":0 []
- entity 30: ipu2_ic_pp1 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev9
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_ic_prpvf":1 []
<- "ipu2_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif2":0 []
-> "camif3":0 []
- entity 33: ipu1_csi0 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev10
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_csi0_mux":2 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none
crop.bounds:(0,0)/640x480
crop:(0,0)/0x0]
-> "ipu1_ic_prpenc":0 []
-> "ipu1_ic_prpvf":0 []
-> "ipu1_smfc0":0 []
- entity 36: ipu1_csi1 (2 pads, 3 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev11
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
pad1: Source
[fmt:UYVY2X8/640x480 field:none
crop.bounds:(0,0)/640x480
crop:(0,0)/0x0]
-> "ipu1_ic_prpenc":0 []
-> "ipu1_ic_prpvf":0 []
-> "ipu1_smfc1":0 []
- entity 39: ipu2_csi0 (2 pads, 3 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev12
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
pad1: Source
[fmt:UYVY2X8/640x480 field:none
crop.bounds:(0,0)/640x480
crop:(0,0)/0x0]
-> "ipu2_ic_prpenc":0 []
-> "ipu2_ic_prpvf":0 []
-> "ipu2_smfc0":0 []
- entity 42: ipu2_csi1 (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev13
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_csi1_mux":2 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none
crop.bounds:(0,0)/640x480
crop:(0,0)/0x0]
-> "ipu2_ic_prpenc":0 []
-> "ipu2_ic_prpvf":0 []
-> "ipu2_smfc1":0 []
- entity 45: ipu1_smfc0 (2 pads, 5 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev14
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_csi0":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "ipu1_ic_prpvf":0 []
-> "ipu1_ic_pp0":0 []
-> "camif0":0 []
-> "camif1":0 []
- entity 48: ipu1_smfc1 (2 pads, 5 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev15
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_csi1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "ipu1_ic_prpvf":0 []
-> "ipu1_ic_pp1":0 []
-> "camif0":0 []
-> "camif1":0 []
- entity 51: ipu2_smfc0 (2 pads, 5 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev16
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_csi0":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "ipu2_ic_prpvf":0 []
-> "ipu2_ic_pp0":0 []
-> "camif2":0 []
-> "camif3":0 []
- entity 54: ipu2_smfc1 (2 pads, 5 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev17
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_csi1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "ipu2_ic_prpvf":0 []
-> "ipu2_ic_pp1":0 []
-> "camif2":0 []
-> "camif3":0 []
- entity 57: camif0 (2 pads, 7 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev18
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_ic_prpenc":1 []
<- "ipu1_ic_prpvf":1 []
<- "ipu1_ic_pp0":1 []
<- "ipu1_ic_pp1":1 []
<- "ipu1_smfc0":1 []
<- "ipu1_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif0 devnode":0 [ENABLED,IMMUTABLE]
- entity 58: camif0 devnode (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Sink
<- "camif0":1 [ENABLED,IMMUTABLE]
- entity 66: camif1 (2 pads, 7 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev19
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu1_ic_prpenc":1 []
<- "ipu1_ic_prpvf":1 []
<- "ipu1_ic_pp0":1 []
<- "ipu1_ic_pp1":1 []
<- "ipu1_smfc0":1 []
<- "ipu1_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif1 devnode":0 [ENABLED,IMMUTABLE]
- entity 67: camif1 devnode (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video1
pad0: Sink
<- "camif1":1 [ENABLED,IMMUTABLE]
- entity 75: camif2 (2 pads, 7 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev20
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_ic_prpenc":1 []
<- "ipu2_ic_prpvf":1 []
<- "ipu2_ic_pp0":1 []
<- "ipu2_ic_pp1":1 []
<- "ipu2_smfc0":1 []
<- "ipu2_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif2 devnode":0 [ENABLED,IMMUTABLE]
- entity 76: camif2 devnode (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video2
pad0: Sink
<- "camif2":1 [ENABLED,IMMUTABLE]
- entity 84: camif3 (2 pads, 7 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev21
pad0: Sink
[fmt:UYVY2X8/640x480 field:none]
<- "ipu2_ic_prpenc":1 []
<- "ipu2_ic_prpvf":1 []
<- "ipu2_ic_pp0":1 []
<- "ipu2_ic_pp1":1 []
<- "ipu2_smfc0":1 []
<- "ipu2_smfc1":1 []
pad1: Source
[fmt:UYVY2X8/640x480 field:none]
-> "camif3 devnode":0 [ENABLED,IMMUTABLE]
- entity 85: camif3 devnode (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video3
pad0: Sink
<- "camif3":1 [ENABLED,IMMUTABLE]
- entity 93: adv7180 2-0020 (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 20004
device node name /dev/v4l-subdev22
pad0: Source
[fmt:UYVY2X8/720x480 field:interlaced]
-> "ipu1_csi0_mux":1 []
How do I link the entities here to be able to capture frames with
v4l2-ctl or gstreamer?
Additionally I've found that on an IMX6S/IMX6DL we crash while
registering the media-ic subdev's:
[ 3.975473] imx-media: Registered subdev ipu1_csi1_mux
[ 3.980921] imx-media: Registered subdev ipu1_csi0_mux
[ 4.003205] imx-media: Registered subdev ipu1_ic_prpenc
[ 4.025373] imx-media: Registered subdev ipu1_ic_prpvf
[ 4.037944] ------------[ cut here ]------------
[ 4.042571] Kernel BUG at c06717dc [verbose debug info unavailable]
[ 4.048845] Internal error: Oops - BUG: 0 [#1] SMP ARM
[ 4.053990] Modules linked in:
[ 4.057076] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
4.9.0-rc6-00524-g84dad6e-dirty #446
[ 4.065260] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
...
[ 4.296250] [<c0671780>] (v4l2_subdev_init) from [<c06fb02c>]
(imx_ic_probe+0x94/0x1ac)
[ 4.304271] [<c06faf98>] (imx_ic_probe) from [<c05173d8>]
(platform_drv_probe+0x54/0xb8)
[ 4.312373] r9:c0d5e858 r8:00000000 r7:fffffdfb r6:c0e5dbf8
r5:da603810 r4:c16738d8
[ 4.320129] [<c0517384>] (platform_drv_probe) from [<c0515978>]
(driver_probe_device+0x20c/0x2c0)
[ 4.329010] r7:c0e5dbf8 r6:00000000 r5:da603810 r4:c16738d8
[ 4.334681] [<c051576c>] (driver_probe_device) from [<c0515af4>]
(__driver_attach+0xc8/0xcc)
[ 4.343129] r9:c0d5e858 r8:00000000 r7:00000000 r6:da603844
r5:c0e5dbf8 r4:da603810
[ 4.350889] [<c0515a2c>] (__driver_attach) from [<c0513adc>]
(bus_for_each_dev+0x74/0xa8)
[ 4.359078] r7:00000000 r6:c0515a2c r5:c0e5dbf8 r4:00000000
[ 4.364753] [<c0513a68>] (bus_for_each_dev) from [<c05151d4>]
(driver_attach+0x20/0x28)
I assume there is an iteration that needs a test on a missing pointer
only available on chips with both IPU's or PRP
Regards,
Tim
^ permalink raw reply related
* Re: [PATCHv2] dt: bindings: Add support for CSI1 bus
From: Sebastian Reichel @ 2017-01-11 23:55 UTC (permalink / raw)
To: Pavel Machek
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-X3B1VOXEql0, pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170111225335.GA21553@amd>
[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]
Hi,
On Wed, Jan 11, 2017 at 11:53:35PM +0100, Pavel Machek wrote:
> From: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
>
> In the vast majority of cases the bus type is known to the driver(s)
> since a receiver or transmitter can only support a single one. There
> are cases however where different options are possible.
>
> The existing V4L2 OF support tries to figure out the bus type and
> parse the bus parameters based on that. This does not scale too well
> as there are multiple serial busses that share common properties.
>
> Some hardware also supports multiple types of busses on the same
> interfaces.
>
> Document the CSI1/CCP2 property strobe. It signifies the clock or
> strobe mode.
>
> Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
>
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 9cd2a36..08c4498 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -76,6 +76,11 @@ Optional endpoint properties
> mode horizontal and vertical synchronization signals are provided to the
> slave device (data source) by the master device (data sink). In the master
> mode the data source device is also the source of the synchronization signals.
> +- bus-type: data bus type. Possible values are:
> + 0 - MIPI CSI2
> + 1 - parallel / Bt656
> + 2 - MIPI CSI1
> + 3 - CCP2
> - bus-width: number of data lines actively used, valid for the parallel busses.
> - data-shift: on the parallel data busses, if bus-width is used to specify the
> number of data lines, data-shift can be used to specify which data lines are
> @@ -112,7 +117,8 @@ Optional endpoint properties
> should be the combined length of data-lanes and clock-lanes properties.
> If the lane-polarities property is omitted, the value must be interpreted
> as 0 (normal). This property is valid for serial busses only.
> -
> +- strobe: Whether the clock signal is used as clock or strobe. Used
> + with CCP2, for instance.
>
> Example
> -------
>
>
Reviewed-By: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 0/4] ASpeed mailbox and LPC control drivers
From: Cyril Bur @ 2017-01-12 0:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
andrew-zrmu5oMJ5Fs, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
xow-hpIqsD4AKlfQT0dZR+AlfA, jk-mnsaURCQ41sdnm+yROfE0A
Hello,
I have written some drivers for the ASpeed AST2400/2500 chips. These
chips are designed to be used as BMCs and the core reason for these
drivers has been to introduce a communication channel between two
processors.
The mailbox registers are the channel through which the two processors
can communicate, it is worth noting here that the mailbox driver is
designed to be flexible enough do allow userspace to arbitrarily write
to one or all or a subset of the mailbox registers. This is important
as the ASpeed chip allows for enabling hardware interrupts based on
writes at a per data register level. It is possible that the other
processor will enable interrupts on one of the data registers, as
such, the ASpeed end of the protocol will need to be able to treat
that register specially.
The first intended use of a communication protocol between the ASpeed
and the other processor is so they can arbitrate on board flash chip
access. The goal is to have the ASpeed chip perform the reads and
writes to the flash and present to the other processor in an area of
its RAM across a shared bus. Currently the other processor has the
flash mapped directly on the shared bus. The LPC bus controlling
driver provides a way for the ASpeed userspace to control the mapping
across the LPC bus between the ASpeed and the other processor. The RAM
region that the LPC control driver will use should be specified in the
device tree.
Cyril Bur (4):
Documentation: dt: mailbox: Add Aspeed ast2400/2500 bindings
Documentation: dt: misc: Add Aspeed ast2400/2500 LPC Control bindings
drivers/misc: Add ASpeed LPC control driver
drivers/mailbox: Add ASpeed mailbox driver
.../devicetree/bindings/mailbox/aspeed-mbox.txt | 44 +++
.../devicetree/bindings/misc/aspeed-lpc-ctrl.txt | 78 +++++
drivers/mailbox/Kconfig | 9 +
drivers/mailbox/Makefile | 2 +
drivers/mailbox/aspeed-mbox.c | 334 +++++++++++++++++++++
drivers/misc/Kconfig | 9 +
drivers/misc/Makefile | 1 +
drivers/misc/aspeed-lpc-ctrl.c | 269 +++++++++++++++++
include/uapi/linux/aspeed-lpc-ctrl.h | 25 ++
9 files changed, 771 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
create mode 100644 Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
create mode 100644 drivers/mailbox/aspeed-mbox.c
create mode 100644 drivers/misc/aspeed-lpc-ctrl.c
create mode 100644 include/uapi/linux/aspeed-lpc-ctrl.h
--
2.11.0
--
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 1/4] Documentation: dt: mailbox: Add Aspeed ast2400/2500 bindings
From: Cyril Bur @ 2017-01-12 0:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
andrew-zrmu5oMJ5Fs, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
xow-hpIqsD4AKlfQT0dZR+AlfA, jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170112002910.3650-1-cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../devicetree/bindings/mailbox/aspeed-mbox.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
diff --git a/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt b/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
new file mode 100644
index 000000000000..633cd534d91c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/aspeed-mbox.txt
@@ -0,0 +1,44 @@
+ASpeed Mailbox Driver
+=====================
+
+The ASpeed mailbox allows for communication between different
+processors. The mailbox on the ASpeed ast2400 and ast2500 is a set of
+16 single byte data registers along with interrupt and configuration
+registers directly on the SoC. These are memory mapped on the aspeed
+and can be accessed via the SuperIO registers on the other processor.
+
+Device Node:
+============
+This represents the mailbox on the Soc.
+
+As the mailbox registers sit on the LPC bus, it makes most sense for
+the device to be within the LPC host node. See
+Documentation/devicetree/bindings/mfd/aspeed-lpc.txt for more
+information. This does not have to be the case, provided the reg
+property can give the full address of the mbox registers.
+
+Required Properties:
+--------------------
+- compatible: Should be one of the following,
+ "aspeed,ast2400-mbox" for ast2400 SoCs
+ "aspeed,ast2500-mbox" for ast2500 SoCs
+
+- reg: Contains the mailbox address register range (base
+ address and length). Keeping in mind that if the node
+ exists within the LPC host node and that base is
+ relative to that.
+
+- interrupts: Contains interrupt information for the mailbox device.
+
+- #mbox-cells: Common property, should be 1.
+
+Example:
+--------
+
+mbox: mbox@180 {
+ compatible = "aspeed,ast2400-mbox";
+ reg = <0x180 0x5c>;
+ interrupts = <46>;
+ #mbox-cells = <1>;
+};
+
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/4] Documentation: dt: misc: Add Aspeed ast2400/2500 LPC Control bindings
From: Cyril Bur @ 2017-01-12 0:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
andrew-zrmu5oMJ5Fs, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
xow-hpIqsD4AKlfQT0dZR+AlfA, jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170112002910.3650-1-cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
.../devicetree/bindings/misc/aspeed-lpc-ctrl.txt | 78 ++++++++++++++++++++++
1 file changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
diff --git a/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt b/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
new file mode 100644
index 000000000000..f84ac83211ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/aspeed-lpc-ctrl.txt
@@ -0,0 +1,78 @@
+ASpeed LPC Control
+==================
+This binding defines the LPC control for ASpeed SoCs. Partitions of
+the LPC bus can be access by other processors on the system, address
+ranges on the bus can map accesses from another processor to regions
+of the ASpeed SoC memory space.
+
+Reserved Memory:
+================
+The driver provides functionality to map the LPC bus to a region of
+ASpeed ram. A phandle to a reserved memory node must be provided so
+that the driver can safely use this region.
+
+Flash:
+======
+The driver provides functionality to unmap the LPC bus from ASpeed
+RAM, historically the default mapping has been to the SPI flash
+controller on the ASpeed SoC, a phandle to this node should be
+supplied.
+
+Device Node:
+============
+
+As LPC bus configuration registers are at the start of the LPC bus
+memory space, it makes most sense for the device to be within the LPC
+host node. See Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
+for more information. This does not have to be the case, provided the
+reg property can give the full address of the LPC bus.
+
+Required properties:
+--------------------
+
+- compatible: "aspeed,ast2400-lpc-ctrl" for ASpeed ast2400 SoCs
+ "aspeed,ast2500-lpc-ctrl" for ASpeed ast2500 SoCs
+
+- reg: Location and size of the configuration registers
+ for the LPC bus. Note that if the device node is
+ within the LPC host node then base is relative to
+ that.
+
+- memory-region: phandle of the reserved memory region
+- flash: phandle of the SPI flash controller
+
+Example:
+--------
+
+reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ ...
+
+ flash_memory: region@54000000 {
+ compatible = "aspeed,ast2400-lpc-ctrl";
+ no-map;
+ reg = <0x54000000 0x04000000>; /* 64M */
+ };
+};
+
+host_pnor: spi@1e630000 {
+ reg = < 0x1e630000 0x18
+ 0x30000000 0x02000000 >;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "aspeed,ast2400-smc";
+
+ ...
+
+};
+
+lpc-ctrl@0 {
+ compatible = "aspeed,ast2400-lpc-ctrl";
+ memory-region = <&flash_memory>;
+ flash = <&host_pnor>;
+ reg = <0x0 0x80>;
+};
+
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/4] drivers/misc: Add ASpeed LPC control driver
From: Cyril Bur @ 2017-01-12 0:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
andrew-zrmu5oMJ5Fs, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
xow-hpIqsD4AKlfQT0dZR+AlfA, jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170112002910.3650-1-cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This driver exposes a reserved chunk of BMC ram to userspace as well as
an ioctl interface to control the BMC<->HOST mapping of the LPC bus.
This allows for a communication channel between the BMC and the host
Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/misc/Kconfig | 9 ++
drivers/misc/Makefile | 1 +
drivers/misc/aspeed-lpc-ctrl.c | 269 +++++++++++++++++++++++++++++++++++
include/uapi/linux/aspeed-lpc-ctrl.h | 25 ++++
4 files changed, 304 insertions(+)
create mode 100644 drivers/misc/aspeed-lpc-ctrl.c
create mode 100644 include/uapi/linux/aspeed-lpc-ctrl.h
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971baf11fa..8696627ce9d2 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -766,6 +766,15 @@ config PANEL_BOOT_MESSAGE
An empty message will only clear the display at driver init time. Any other
printf()-formatted message is valid with newline and escape codes.
+config ASPEED_LPC_CTRL
+ depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
+ bool "Build a driver to control the BMC to HOST LPC bus"
+ default "y"
+ ---help---
+ Provides a driver to control BMC to HOST LPC mappings through
+ ioctl()s, the driver aso provides a read/write interface to a BMC ram
+ region where host LPC can be buffered.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 31983366090a..de1925a9c80b 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_ECHO) += echo/
obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PANEL) += panel.o
+obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
new file mode 100644
index 000000000000..36ab718681a5
--- /dev/null
+++ b/drivers/misc/aspeed-lpc-ctrl.c
@@ -0,0 +1,269 @@
+/*
+ * Copyright 2016 IBM Corporation
+ *
+ * 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/mfd/syscon.h>
+#include <linux/miscdevice.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/regmap.h>
+
+#include <linux/aspeed-lpc-ctrl.h>
+
+#define DEVICE_NAME "aspeed-lpc-ctrl"
+
+#define HICR7 0x8
+#define HICR8 0xc
+
+struct lpc_ctrl {
+ struct miscdevice miscdev;
+ struct regmap *regmap;
+ phys_addr_t base;
+ resource_size_t size;
+ uint32_t pnor_size;
+ uint32_t pnor_base;
+};
+
+static atomic_t lpc_ctrl_open_count = ATOMIC_INIT(0);
+
+static struct lpc_ctrl *file_lpc_ctrl(struct file *file)
+{
+ return container_of(file->private_data, struct lpc_ctrl, miscdev);
+}
+
+static int lpc_ctrl_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct lpc_ctrl *lpc_ctrl = file_lpc_ctrl(file);
+ unsigned long vsize = vma->vm_end - vma->vm_start;
+
+ if (vma->vm_pgoff + vsize > lpc_ctrl->base + lpc_ctrl->size)
+ return -EINVAL;
+
+ /* Other checks? */
+
+ if (remap_pfn_range(vma, vma->vm_start,
+ (lpc_ctrl->base >> PAGE_SHIFT) + vma->vm_pgoff,
+ vsize, vma->vm_page_prot))
+ return -EAGAIN;
+
+ return 0;
+}
+
+static int lpc_ctrl_open(struct inode *inode, struct file *file)
+{
+ if (atomic_inc_return(&lpc_ctrl_open_count) == 1)
+ return 0;
+
+ atomic_dec(&lpc_ctrl_open_count);
+ return -EBUSY;
+}
+
+static ssize_t lpc_ctrl_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
+
+ return -EPERM;
+}
+
+static ssize_t lpc_ctrl_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ if (!access_ok(VERIFY_READ, buf, count))
+ return -EFAULT;
+
+ return -EPERM;
+}
+
+static long lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
+ unsigned long param)
+{
+ long rc;
+ struct lpc_mapping map;
+ struct lpc_ctrl *lpc_ctrl = file_lpc_ctrl(file);
+ void __user *p = (void __user *)param;
+
+ switch (cmd) {
+ case LPC_CTRL_IOCTL_SIZE:
+ return copy_to_user(p, &lpc_ctrl->size,
+ sizeof(lpc_ctrl->size)) ? -EFAULT : 0;
+ case LPC_CTRL_IOCTL_MAP:
+ if (copy_from_user(&map, p, sizeof(map)))
+ return -EFAULT;
+
+
+ /*
+ * The top half of HICR7 is the MSB of the BMC address of the
+ * mapping.
+ * The bottom half of HICR7 is the MSB of the HOST LPC
+ * firmware space address of the mapping.
+ *
+ * The 1 bits in the top of half of HICR8 represent the bits
+ * (in the requested address) that should be ignored and
+ * replaced with those from the top half of HICR7.
+ * The 1 bits in the bottom half of HICR8 represent the bits
+ * (in the requested address) that should be kept and pass
+ * into the BMC address space.
+ */
+
+ rc = regmap_write(lpc_ctrl->regmap, HICR7,
+ (lpc_ctrl->base | (map.hostaddr >> 16)));
+ if (rc)
+ return rc;
+
+ rc = regmap_write(lpc_ctrl->regmap, HICR8,
+ (~(map.size - 1)) | ((map.size >> 16) - 1));
+ return rc;
+ case LPC_CTRL_IOCTL_UNMAP:
+ /*
+ * The top nibble in host lpc addresses references which
+ * firmware space, use space zero hence the & 0x0fff
+ */
+
+ rc = regmap_write(lpc_ctrl->regmap, HICR7,
+ lpc_ctrl->pnor_base | (((-lpc_ctrl->pnor_size) >> 16) & 0x0fff));
+ if (rc)
+ return rc;
+
+ rc = regmap_write(lpc_ctrl->regmap, HICR8,
+ (~(lpc_ctrl->pnor_size - 1)) | ((lpc_ctrl->pnor_size >> 16) - 1));
+ return rc;
+ }
+
+ return -EINVAL;
+}
+
+static int lpc_ctrl_release(struct inode *inode, struct file *file)
+{
+ atomic_dec(&lpc_ctrl_open_count);
+ return 0;
+}
+
+static const struct file_operations lpc_ctrl_fops = {
+ .owner = THIS_MODULE,
+ .mmap = lpc_ctrl_mmap,
+ .open = lpc_ctrl_open,
+ .read = lpc_ctrl_read,
+ .write = lpc_ctrl_write,
+ .release = lpc_ctrl_release,
+ .unlocked_ioctl = lpc_ctrl_ioctl,
+};
+
+static int lpc_ctrl_probe(struct platform_device *pdev)
+{
+ struct lpc_ctrl *lpc_ctrl;
+ struct device *dev;
+ struct device_node *node;
+ struct resource resm;
+ int rc;
+
+ if (!pdev || !pdev->dev.of_node)
+ return -ENODEV;
+
+ dev = &pdev->dev;
+
+ lpc_ctrl = devm_kzalloc(dev, sizeof(*lpc_ctrl), GFP_KERNEL);
+ if (!lpc_ctrl)
+ return -ENOMEM;
+
+ node = of_parse_phandle(dev->of_node, "flash", 0);
+ if (!node) {
+ dev_err(dev, "Didn't find host pnor flash node\n");
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_u32_index(node, "reg", 3,
+ &lpc_ctrl->pnor_size);
+ if (rc)
+ return rc;
+ rc = of_property_read_u32_index(node, "reg", 2,
+ &lpc_ctrl->pnor_base);
+ if (rc)
+ return rc;
+
+ dev_info(dev, "Host PNOR base: 0x%08x size: 0x%08x\n",
+ lpc_ctrl->pnor_base, lpc_ctrl->pnor_size);
+
+ dev_set_drvdata(&pdev->dev, lpc_ctrl);
+
+ node = of_parse_phandle(dev->of_node, "memory-region", 0);
+ if (!node) {
+ dev_err(dev, "Didn't find reserved memory\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ rc = of_address_to_resource(node, 0, &resm);
+ of_node_put(node);
+ if (rc) {
+ dev_err(dev, "Could address to resource\n");
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ lpc_ctrl->size = resource_size(&resm);
+ lpc_ctrl->base = resm.start;
+
+ lpc_ctrl->regmap = syscon_node_to_regmap(
+ pdev->dev.parent->of_node);
+ if (IS_ERR(lpc_ctrl->regmap)) {
+ dev_err(dev, "Couldn't get regmap\n");
+ rc = -ENODEV;
+ goto out;
+ }
+
+ lpc_ctrl->miscdev.minor = MISC_DYNAMIC_MINOR;
+ lpc_ctrl->miscdev.name = DEVICE_NAME;
+ lpc_ctrl->miscdev.fops = &lpc_ctrl_fops;
+ lpc_ctrl->miscdev.parent = dev;
+ rc = misc_register(&lpc_ctrl->miscdev);
+ if (rc)
+ dev_err(dev, "Unable to register device\n");
+ else
+ dev_info(dev, "Loaded at 0x%08x (0x%08x)\n",
+ lpc_ctrl->base, lpc_ctrl->size);
+
+out:
+ return rc;
+}
+
+static int lpc_ctrl_remove(struct platform_device *pdev)
+{
+ struct lpc_ctrl *lpc_ctrl = dev_get_drvdata(&pdev->dev);
+
+ misc_deregister(&lpc_ctrl->miscdev);
+ lpc_ctrl = NULL;
+
+ return 0;
+}
+
+static const struct of_device_id lpc_ctrl_match[] = {
+ { .compatible = "aspeed,ast2400-lpc-ctrl" },
+ { },
+};
+
+static struct platform_driver lpc_ctrl_driver = {
+ .driver = {
+ .name = DEVICE_NAME,
+ .of_match_table = lpc_ctrl_match,
+ },
+ .probe = lpc_ctrl_probe,
+ .remove = lpc_ctrl_remove,
+};
+
+module_platform_driver(lpc_ctrl_driver);
+
+MODULE_DEVICE_TABLE(of, lpc_ctrl_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("Linux device interface to control LPC bus");
diff --git a/include/uapi/linux/aspeed-lpc-ctrl.h b/include/uapi/linux/aspeed-lpc-ctrl.h
new file mode 100644
index 000000000000..c5f1caf827ac
--- /dev/null
+++ b/include/uapi/linux/aspeed-lpc-ctrl.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 IBM Corp.
+ *
+ * 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.
+ */
+
+#ifndef _UAPI_LINUX_LPC_CTRL_H
+#define _UAPI_LINUX_LPC_CTRL_H
+
+#include <linux/ioctl.h>
+
+struct lpc_mapping {
+ uint32_t hostaddr;
+ uint32_t size;
+};
+
+#define __LPC_CTRL_IOCTL_MAGIC 0xb2
+#define LPC_CTRL_IOCTL_SIZE _IOR(__LPC_CTRL_IOCTL_MAGIC, 0x00, uint32_t)
+#define LPC_CTRL_IOCTL_MAP _IOW(__LPC_CTRL_IOCTL_MAGIC, 0x01, struct lpc_mapping)
+#define LPC_CTRL_IOCTL_UNMAP _IO(__LPC_CTRL_IOCTL_MAGIC, 0x02)
+
+#endif /* _UAPI_LINUX_LPC_CTRL_H */
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 4/4] drivers/mailbox: Add ASpeed mailbox driver
From: Cyril Bur @ 2017-01-12 0:29 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: joel-U3u1mxZcP9KHXe+LvDLADg, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, openbmc-uLR06cmDAlY/bJ5BZ2RsiQ,
andrew-zrmu5oMJ5Fs, benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
xow-hpIqsD4AKlfQT0dZR+AlfA, jk-mnsaURCQ41sdnm+yROfE0A
In-Reply-To: <20170112002910.3650-1-cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This provides access to the mbox registers on the ast2400 and ast2500
boards.
This driver allows arbitrary reads and writes to the 16 data registers as
the other end may have configured the mbox hardware to provide an
interrupt when a specific register gets written to.
Signed-off-by: Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/mailbox/Kconfig | 9 ++
drivers/mailbox/Makefile | 2 +
drivers/mailbox/aspeed-mbox.c | 334 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 345 insertions(+)
create mode 100644 drivers/mailbox/aspeed-mbox.c
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ceff415f201c..10a7f3f2765c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -152,4 +152,13 @@ config BCM_PDC_MBOX
Mailbox implementation for the Broadcom PDC ring manager,
which provides access to various offload engines on Broadcom
SoCs. Say Y here if you want to use the Broadcom PDC.
+
+config ASPEED_MBOX
+ depends on (ARCH_ASPEED || COMPILE_TEST) && REGMAP && MFD_SYSCON
+ bool "Aspeed ast2400/2500 Mailbox Controller"
+ default "y"
+ ---help---
+ Provides a driver for the MBOX registers found on Aspeed SOCs
+ (AST2400 and AST2500). This driver provides a device for aspeed
+ mbox registers
endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 7dde4f609ae8..db5b4f3f29e0 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -31,3 +31,5 @@ obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o
obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o
obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o
+
+obj-$(CONFIG_ASPEED_MBOX) += aspeed-mbox.o
diff --git a/drivers/mailbox/aspeed-mbox.c b/drivers/mailbox/aspeed-mbox.c
new file mode 100644
index 000000000000..c4ee6ba228ea
--- /dev/null
+++ b/drivers/mailbox/aspeed-mbox.c
@@ -0,0 +1,334 @@
+/*
+ * Copyright 2016 IBM Corporation
+ *
+ * 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/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/poll.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define DEVICE_NAME "aspeed-mbox"
+
+#define ASPEED_MBOX_NUM_REGS 16
+
+#define ASPEED_MBOX_DATA_0 0x00
+#define ASPEED_MBOX_STATUS_0 0x40
+#define ASPEED_MBOX_STATUS_1 0x44
+#define ASPEED_MBOX_BMC_CTRL 0x48
+#define ASPEED_MBOX_CTRL_RECV BIT(7)
+#define ASPEED_MBOX_CTRL_MASK BIT(1)
+#define ASPEED_MBOX_CTRL_SEND BIT(0)
+#define ASPEED_MBOX_HOST_CTRL 0x4c
+#define ASPEED_MBOX_INTERRUPT_0 0x50
+#define ASPEED_MBOX_INTERRUPT_1 0x54
+
+struct aspeed_mbox {
+ struct miscdevice miscdev;
+ struct regmap *regmap;
+ unsigned int base;
+ wait_queue_head_t queue;
+ struct mutex mutex;
+};
+
+static atomic_t aspeed_mbox_open_count = ATOMIC_INIT(0);
+
+static u8 aspeed_mbox_inb(struct aspeed_mbox *mbox, int reg)
+{
+ /*
+ * The mbox registers are actually only one byte but are addressed
+ * four bytes apart. The other three bytes are marked 'reserved',
+ * they *should* be zero but lets not rely on it.
+ * I am going to rely on the fact we can casually read/write to them...
+ */
+ unsigned int val = 0xff; /* If regmap throws an error return 0xff */
+ int rc = regmap_read(mbox->regmap, mbox->base + reg, &val);
+
+ if (rc)
+ dev_err(mbox->miscdev.parent, "regmap_read() failed with "
+ "%d (reg: 0x%08x)\n", rc, reg);
+
+ return val & 0xff;
+}
+
+static void aspeed_mbox_outb(struct aspeed_mbox *mbox, u8 data, int reg)
+{
+ int rc = regmap_write(mbox->regmap, mbox->base + reg, data);
+
+ if (rc)
+ dev_err(mbox->miscdev.parent, "regmap_write() failed with "
+ "%d (data: %u reg: 0x%08x)\n", rc, data, reg);
+}
+
+static struct aspeed_mbox *file_mbox(struct file *file)
+{
+ return container_of(file->private_data, struct aspeed_mbox, miscdev);
+}
+
+static int aspeed_mbox_open(struct inode *inode, struct file *file)
+{
+ struct aspeed_mbox *mbox = file_mbox(file);
+
+ if (atomic_inc_return(&aspeed_mbox_open_count) == 1) {
+ /*
+ * Clear the interrupt status bit if it was left on and unmask
+ * interrupts.
+ * ASPEED_MBOX_CTRL_RECV bit is W1C, this also unmasks in 1 step
+ */
+ aspeed_mbox_outb(mbox, ASPEED_MBOX_CTRL_RECV, ASPEED_MBOX_BMC_CTRL);
+ return 0;
+ }
+
+ atomic_dec(&aspeed_mbox_open_count);
+ return -EBUSY;
+}
+
+static ssize_t aspeed_mbox_read(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct aspeed_mbox *mbox = file_mbox(file);
+ char __user *p = buf;
+ ssize_t ret;
+ int i;
+
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
+
+ if (count + *ppos > ASPEED_MBOX_NUM_REGS)
+ return -EINVAL;
+
+ if (file->f_flags & O_NONBLOCK) {
+ if (!(aspeed_mbox_inb(mbox, ASPEED_MBOX_BMC_CTRL) &
+ ASPEED_MBOX_CTRL_RECV))
+ return -EAGAIN;
+ } else if (wait_event_interruptible(mbox->queue,
+ aspeed_mbox_inb(mbox, ASPEED_MBOX_BMC_CTRL) &
+ ASPEED_MBOX_CTRL_RECV)) {
+ return -ERESTARTSYS;
+ }
+
+ mutex_lock(&mbox->mutex);
+
+ for (i = *ppos; count > 0 && i < ASPEED_MBOX_NUM_REGS; i++) {
+ uint8_t reg = aspeed_mbox_inb(mbox, ASPEED_MBOX_DATA_0 + (i * 4));
+
+ ret = __put_user(reg, p);
+ if (ret)
+ goto out_unlock;
+
+ p++;
+ count--;
+ }
+
+ /* ASPEED_MBOX_CTRL_RECV bit is W1C, this also unmasks in 1 step */
+ aspeed_mbox_outb(mbox, ASPEED_MBOX_CTRL_RECV, ASPEED_MBOX_BMC_CTRL);
+ ret = p - buf;
+
+out_unlock:
+ mutex_unlock(&mbox->mutex);
+ return ret;
+}
+
+static ssize_t aspeed_mbox_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct aspeed_mbox *mbox = file_mbox(file);
+ const char __user *p = buf;
+ ssize_t ret;
+ char c;
+ int i;
+
+ if (!access_ok(VERIFY_READ, buf, count))
+ return -EFAULT;
+
+ if (count + *ppos > ASPEED_MBOX_NUM_REGS)
+ return -EINVAL;
+
+ mutex_lock(&mbox->mutex);
+
+ for (i = *ppos; count > 0 && i < ASPEED_MBOX_NUM_REGS; i++) {
+ ret = __get_user(c, p);
+ if (ret)
+ goto out_unlock;
+
+ aspeed_mbox_outb(mbox, c, ASPEED_MBOX_DATA_0 + (i * 4));
+ p++;
+ count--;
+ }
+
+ aspeed_mbox_outb(mbox, ASPEED_MBOX_CTRL_SEND, ASPEED_MBOX_BMC_CTRL);
+ ret = p - buf;
+
+out_unlock:
+ mutex_unlock(&mbox->mutex);
+ return ret;
+}
+
+static unsigned int aspeed_mbox_poll(struct file *file, poll_table *wait)
+{
+ struct aspeed_mbox *mbox = file_mbox(file);
+ unsigned int mask = 0;
+
+ poll_wait(file, &mbox->queue, wait);
+
+ if (aspeed_mbox_inb(mbox, ASPEED_MBOX_BMC_CTRL) & ASPEED_MBOX_CTRL_RECV)
+ mask |= POLLIN;
+
+ return mask;
+}
+
+static int aspeed_mbox_release(struct inode *inode, struct file *file)
+{
+ atomic_dec(&aspeed_mbox_open_count);
+ return 0;
+}
+
+static const struct file_operations aspeed_mbox_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_seek_end_llseek,
+ .read = aspeed_mbox_read,
+ .write = aspeed_mbox_write,
+ .open = aspeed_mbox_open,
+ .release = aspeed_mbox_release,
+ .poll = aspeed_mbox_poll,
+};
+
+static irqreturn_t aspeed_mbox_irq(int irq, void *arg)
+{
+ struct aspeed_mbox *mbox = arg;
+
+ if (!(aspeed_mbox_inb(mbox, ASPEED_MBOX_BMC_CTRL) & ASPEED_MBOX_CTRL_RECV))
+ return IRQ_NONE;
+
+ /*
+ * Leave the status bit set so that we know the data is for us,
+ * clear it once it has been read.
+ */
+
+ /* Mask it off, we'll clear it when we the data gets read */
+ aspeed_mbox_outb(mbox, ASPEED_MBOX_CTRL_MASK, ASPEED_MBOX_BMC_CTRL);
+
+ wake_up(&mbox->queue);
+ return IRQ_HANDLED;
+}
+
+static int aspeed_mbox_config_irq(struct aspeed_mbox *mbox,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int rc, irq;
+
+ irq = irq_of_parse_and_map(dev->of_node, 0);
+ if (!irq)
+ return -ENODEV;
+
+ rc = devm_request_irq(dev, irq, aspeed_mbox_irq,
+ IRQF_SHARED, DEVICE_NAME, mbox);
+ if (rc < 0) {
+ dev_err(dev, "Unable to request IRQ %d\n", irq);
+ return rc;
+ }
+
+ /*
+ * Disable all register based interrupts.
+ */
+ aspeed_mbox_outb(mbox, 0x00, ASPEED_MBOX_INTERRUPT_0); /* regs 0 - 7 */
+ aspeed_mbox_outb(mbox, 0x00, ASPEED_MBOX_INTERRUPT_1); /* regs 8 - 15 */
+
+ /* These registers are write one to clear. Clear them. */
+ aspeed_mbox_outb(mbox, 0xff, ASPEED_MBOX_STATUS_0);
+ aspeed_mbox_outb(mbox, 0xff, ASPEED_MBOX_STATUS_1);
+
+ aspeed_mbox_outb(mbox, ASPEED_MBOX_CTRL_RECV, ASPEED_MBOX_BMC_CTRL);
+ return 0;
+}
+
+static int aspeed_mbox_probe(struct platform_device *pdev)
+{
+ struct aspeed_mbox *mbox;
+ struct device *dev;
+ int rc;
+
+ dev = &pdev->dev;
+
+ mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+ if (!mbox)
+ return -ENOMEM;
+
+ dev_set_drvdata(&pdev->dev, mbox);
+
+ rc = of_property_read_u32(dev->of_node, "reg", &mbox->base);
+ if (rc) {
+ dev_err(dev, "Couldn't read reg device-tree property\n");
+ return rc;
+ }
+
+ mbox->regmap = syscon_node_to_regmap(
+ pdev->dev.parent->of_node);
+ if (IS_ERR(mbox->regmap)) {
+ dev_err(dev, "Couldn't get regmap\n");
+ return -ENODEV;
+ }
+
+ mutex_init(&mbox->mutex);
+ init_waitqueue_head(&mbox->queue);
+
+ mbox->miscdev.minor = MISC_DYNAMIC_MINOR;
+ mbox->miscdev.name = DEVICE_NAME;
+ mbox->miscdev.fops = &aspeed_mbox_fops;
+ mbox->miscdev.parent = dev;
+ rc = misc_register(&mbox->miscdev);
+ if (rc) {
+ dev_err(dev, "Unable to register device\n");
+ return rc;
+ }
+
+ rc = aspeed_mbox_config_irq(mbox, pdev);
+ if (rc) {
+ dev_err(dev, "Failed to configure IRQ\n");
+ misc_deregister(&mbox->miscdev);
+ return rc;
+ }
+
+ return 0;
+}
+
+static int aspeed_mbox_remove(struct platform_device *pdev)
+{
+ struct aspeed_mbox *mbox = dev_get_drvdata(&pdev->dev);
+
+ misc_deregister(&mbox->miscdev);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_mbox_match[] = {
+ { .compatible = "aspeed,ast2400-mbox" },
+ { .compatible = "aspeed,ast2500-mbox" },
+ { },
+};
+
+static struct platform_driver aspeed_mbox_driver = {
+ .driver = {
+ .name = DEVICE_NAME,
+ .of_match_table = aspeed_mbox_match,
+ },
+ .probe = aspeed_mbox_probe,
+ .remove = aspeed_mbox_remove,
+};
+
+module_platform_driver(aspeed_mbox_driver);
+
+MODULE_DEVICE_TABLE(of, aspeed_mbox_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Cyril Bur <cyrilbur-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("ASpeed mailbox device driver");
--
2.11.0
--
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 1/3] dt: bindings: add documentation for zx2967 family thermal sensor
From: Shawn Guo @ 2017-01-12 0:40 UTC (permalink / raw)
To: Baoyou Xie
Cc: rui.zhang, edubezval, robh+dt, mark.rutland, jun.nie, linux-pm,
devicetree, linux-kernel, linux-arm-kernel, xie.baoyou,
chen.chaokai, wang.qiang01
In-Reply-To: <1484129651-17531-1-git-send-email-baoyou.xie@linaro.org>
On Wed, Jan 11, 2017 at 06:14:09PM +0800, Baoyou Xie wrote:
> This patch adds dt-binding documentation for zx2967 family thermal sensor.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Reviewed-by: Shawn Guo <shawnguo@kernel.org>
Rob already gave his ACK on v1. Since v2 is just an improved version of
v1 (without significant rewriting), you should add his ACK tag, so that
thermal maintainers know that the bindings has been ACK-ed by DT
maintainer.
Shawn
^ permalink raw reply
* Re: [PATCH v2 0/3] Adding DT support for TI HECC module
From: Tony Lindgren @ 2017-01-12 0:47 UTC (permalink / raw)
To: Yegor Yefremov
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Andrey Skvortsov, hs-ynQEQJNshbs
In-Reply-To: <CAGm1_kvWXfZ_f3PPL1VJj8AhBf59Pax_GFHDVdejuMBRDu9y6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170111 13:52]:
> On Wed, Jan 11, 2017 at 3:05 PM, <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >
> > This is an attempt to revive DT support for TI HECC that was started in 2015.
> >
> > I haven't changed much because not all questions could be fully answered:
> >
> > * Should HECC use "am3505" as compatible?
>
> I mean "ti,am3505-hecc"
Yeah it should use the device name for the driver.
> > * What should be done to the offsets (ti,scc-ram-offset, ti,hecc-ram-offset, ti,mbx-offset)?
The devicetree maintainers need to ack the binding doc. Maybe
send that as a first patch?
Regards,
Tony
> First submission and related comments:
> http://comments.gmane.org/gmane.linux.can/8616
--
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 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
From: Doug Anderson @ 2017-01-12 0:50 UTC (permalink / raw)
To: Heiko Stübner
Cc: Xing Zheng, open list:ARM/Rockchip SoC..., Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon, Caesar Wang,
Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang, David Wu,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1523574.hS8CXi1tTP@diego>
Hi,
On Tue, Jan 10, 2017 at 11:58 AM, Heiko Stübner <heiko@sntech.de> wrote:
> Hi Doug,
>
> Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko Stübner:
>> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
>> > Hi,
>> >
>> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing@rock-chips.com>
>>
>> wrote:
>> > > The structure rockchip_clk_provider needs to refer the GRF regmap
>> > > in somewhere, if the CRU node has not "rockchip,grf" property,
>> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
>> > > regmap, and the MUXGRF type clock will be not supported.
>> > >
>> > > Therefore, we need to add them.
>> > >
>> > > Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
>> > > ---
>> > >
>> > > Changes in v4:
>> > > - separte the binding patch
>> > >
>> > > Changes in v3:
>> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
>> > >
>> > > Changes in v2:
>> > > - referring pmugrf for PMUGRU
>> > > - fix the typo "invaild" in COMMIT message
>> > >
>> > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
>> > > 1 file changed, 2 insertions(+)
>> >
>> > This seems fine to me, so:
>> >
>> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>> >
>> > ...but I will say that before you actually add any real "MUXGRF"
>> > clocks on rk3399 you _might_ need to rework the code to make things
>> > truly "optional". If it turns out that any existing clocks that
>> > already exist today already go through one of these muxes in the GRF
>> > and we've always been assuming one setting of the mux, we'll need to
>> > make sure we keep assuming that setting of the mux even if the "grf"
>> > isn't specified.
>>
>> I guess I see that a bit more relaxed :-) .
>>
>> I.e. the GRF being optional is a remnant of syscons not being available when
>> the clocks get set up- so were coming in later or not at all. For the
>> rk3288 I converted, there we never really had the case of the GRF missing.
>>
>> And the GRF mux for the vcodec now present is not being used by anything yet
>> (neither driver nor binding), so no old devicetree can break.
>>
>> > As I understand it, your motivation for this patch is to eventually be
>> > able to model the EDP reference clock which can either be xin24 or
>> > "edp osc". Presumably the eDP "reference clock" isn't related to any
>> > of the pre-existing eDP clocks so that one should be safe.
>>
>> Same here, so far we don't even have edp or even any other graphical output
>> on the rk3399, so again there is no old devicetree that could break when
>> the grf is not specified.
>
> reading all of the above again, it feels like you essentially also said
> similar things already in your original reply and I misread some of it.
>
> But again, I don't see the need for any more code right now, as hopefully the
> simple stuff we currently only support does not have any grf-based muxes in
> it. Xing + Rockchip people, please correct me if I'm wrong here :-)
Right. I have no objection to Xing's patch. I just want to make sure
that if it's listed as "Optional" that it's really optional.
I was worried that we would introduce some GRF-based mux in the
_middle_ of some existing clock tree because we simply didn't model
the mux before and assumed one particular setting. If nothing like
that ever happens then we're fine.
Sorry to be so confusing.
-Doug
^ permalink raw reply
* [PATCH V2 1/2] clk: vc5: Add bindings for IDT VersaClock 5P49V5923 and 5P49V5933
From: Marek Vasut @ 2017-01-12 1:03 UTC (permalink / raw)
To: linux-clk-u79uwXL29TY76Z2rM5mHXA
Cc: Marek Vasut, Michael Turquette, Stephen Boyd, Laurent Pinchart,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
Add bindings for IDT VersaClock 5 5P49V5923 and 5P49V5933 chips.
These are I2C clock generators with optional clock source from
either XTal or dedicated clock generator and, depending on the
model, two or more clock outputs.
Signed-off-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
V2: Add mapping between the clock specifier and physical pins of the chip
---
.../devicetree/bindings/clock/idt,versaclock5.txt | 65 ++++++++++++++++++++++
1 file changed, 65 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/idt,versaclock5.txt
diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
new file mode 100644
index 000000000000..87e9c47a89a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
@@ -0,0 +1,65 @@
+Binding for IDT VersaClock5 programmable i2c clock generator.
+
+The IDT VersaClock5 are programmable i2c clock generators providing
+from 3 to 12 output clocks.
+
+==I2C device node==
+
+Required properties:
+- compatible: shall be one of "idt,5p49v5923" , "idt,5p49v5933".
+- reg: i2c device address, shall be 0x68 or 0x6a.
+- #clock-cells: from common clock binding; shall be set to 1.
+- clocks: from common clock binding; list of parent clock handles,
+ - 5p49v5923: (required) either or both of XTAL or CLKIN
+ reference clock.
+ - 5p49v5933: (optional) property not present (internal
+ Xtal used) or CLKIN reference
+ clock.
+- clock-names: from common clock binding; clock input names, can be
+ - 5p49v5923: (required) either or both of "xin", "clkin".
+ - 5p49v5933: (optional) property not present or "clkin".
+
+==Mapping between clock specifier and physical pins==
+
+When referencing the provided clock in the DT using phandle and
+clock specifier, the following mapping applies:
+
+5P49V5923:
+ 0 -- OUT0_SEL_I2CB
+ 1 -- OUT1
+ 2 -- OUT2
+
+5P49V5933:
+ 0 -- OUT0_SEL_I2CB
+ 1 -- OUT1
+ 2 -- OUT4
+
+==Example==
+
+/* 25MHz reference crystal */
+ref25: ref25m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000000>;
+};
+
+i2c-master-node {
+
+ /* IDT 5P49V5923 i2c clock generator */
+ vc5: clock-generator@6a {
+ compatible = "idt,5p49v5923";
+ reg = <0x6a>;
+ #clock-cells = <1>;
+
+ /* Connect XIN input to 25MHz reference */
+ clocks = <&ref25m>;
+ clock-names = "xin";
+ };
+};
+
+/* Consumer referencing the 5P49V5923 pin OUT1 */
+consumer {
+ ...
+ clocks = <&vc5 1>;
+ ...
+}
--
2.11.0
--
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 3/3] thermal: zx2967: add thermal driver for ZTE's zx2967 family
From: Shawn Guo @ 2017-01-12 1:09 UTC (permalink / raw)
To: Baoyou Xie
Cc: rui.zhang, edubezval, robh+dt, mark.rutland, jun.nie, linux-pm,
devicetree, linux-kernel, linux-arm-kernel, xie.baoyou,
chen.chaokai, wang.qiang01
In-Reply-To: <1484129651-17531-3-git-send-email-baoyou.xie@linaro.org>
On Wed, Jan 11, 2017 at 06:14:11PM +0800, Baoyou Xie wrote:
> This patch adds thermal driver for ZTE's zx2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Looks fine to me, except a few minor comments below.
> ---
> drivers/thermal/Kconfig | 6 +
> drivers/thermal/Makefile | 1 +
> drivers/thermal/zx2967_thermal.c | 247 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 254 insertions(+)
> create mode 100644 drivers/thermal/zx2967_thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 18f2de6..0dd597e 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -445,3 +445,9 @@ config BCM2835_THERMAL
> Support for thermal sensors on Broadcom bcm2835 SoCs.
>
> endif
> +
> +config ZX2967_THERMAL
> + tristate "Thermal sensors on zx2967 SoC"
> + depends on ARCH_ZX
> + help
> + Support for thermal sensors on ZTE zx2967 SoCs.
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 677c6d9..c00c05e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
> obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o
> obj-$(CONFIG_GENERIC_ADC_THERMAL) += thermal-generic-adc.o
> obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o
> +obj-$(CONFIG_ZX2967_THERMAL) += zx2967_thermal.o
> diff --git a/drivers/thermal/zx2967_thermal.c b/drivers/thermal/zx2967_thermal.c
> new file mode 100644
> index 0000000..bdd2d5e
> --- /dev/null
> +++ b/drivers/thermal/zx2967_thermal.c
> @@ -0,0 +1,247 @@
> +/*
> + * ZTE's zx2967 family thermal sensor driver
> + *
> + * Copyright (C) 2017 ZTE Ltd.
> + *
> + * Author: Baoyou Xie <baoyou.xie@linaro.org>
> + *
> + * License terms: GNU General Public License (GPL) version 2
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +/* Power Mode: 0->low 1->high */
> +#define ZX2967_THERMAL_POWER_MODE (0)
Unnecessary parenthesis.
> +
> +/* DCF Control Register */
> +#define ZX2967_THERMAL_DCF 0x4
> +
> +/* Selection Register */
> +#define ZX2967_THERMAL_SEL 0x8
> +
> +/* Control Register */
> +#define ZX2967_THERMAL_CTRL 0x10
> +
> +#define ZX2967_THERMAL_READY (0x1000)
> +#define ZX2967_THERMAL_TEMP_MASK (0xfff)
> +#define ZX2967_THERMAL_ID_MASK (0x18)
> +#define ZX2967_THERMAL_ID0 (0x8)
> +#define ZX2967_THERMAL_ID1 (0x10)
Ditto
> +
> +struct zx2967_thermal_sensor {
> + struct zx2967_thermal_priv *priv;
> + struct thermal_zone_device *tzd;
> + int id;
> +};
> +
> +#define NUM_SENSORS 1
> +
> +struct zx2967_thermal_priv {
> + struct zx2967_thermal_sensor sensors[NUM_SENSORS];
> + struct mutex lock;
> + struct clk *clk_gate;
> + struct clk *pclk;
> + void __iomem *regs;
> +};
> +
> +static int zx2967_thermal_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
> +
> + if (priv && priv->pclk)
> + clk_disable_unprepare(priv->pclk);
> +
> + if (priv && priv->clk_gate)
> + clk_disable_unprepare(priv->clk_gate);
> +
> + return 0;
> +}
> +
> +static int zx2967_thermal_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
> + int error;
> +
> + error = clk_prepare_enable(priv->clk_gate);
> + if (error)
> + return error;
> +
> + error = clk_prepare_enable(priv->pclk);
> + if (error) {
> + clk_disable_unprepare(priv->clk_gate);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int zx2967_thermal_get_temp(void *data, int *temp)
> +{
> + void __iomem *regs;
> + struct zx2967_thermal_sensor *sensor = data;
> + struct zx2967_thermal_priv *priv = sensor->priv;
> + unsigned long timeout = jiffies + msecs_to_jiffies(100);
> + u32 val, sel_id;
> +
> + regs = priv->regs;
> + mutex_lock(&priv->lock);
> +
> + writel_relaxed(0, regs + ZX2967_THERMAL_POWER_MODE);
> + writel_relaxed(2, regs + ZX2967_THERMAL_DCF);
> +
> + val = readl_relaxed(regs + ZX2967_THERMAL_SEL);
> + val &= ~ZX2967_THERMAL_ID_MASK;
> + sel_id = sensor->id ? ZX2967_THERMAL_ID0 : ZX2967_THERMAL_ID1;
> + val |= sel_id;
> + writel_relaxed(val, regs + ZX2967_THERMAL_SEL);
> +
> + usleep_range(100, 300);
> + val = readl_relaxed(regs + ZX2967_THERMAL_CTRL);
> + while (!(val & ZX2967_THERMAL_READY)) {
> + if (time_after(jiffies, timeout)) {
> + pr_err("Thermal sensor %d data timeout\n",
> + sensor->id);
Again, dev_err() should be used in the driver consistently to make sure
the error messages coming from the driver are in the same format. You
can embed a device pointer in zx2967_thermal_priv for use here, I guess.
> + mutex_unlock(&priv->lock);
> + return -ETIMEDOUT;
> + }
> + val = readl_relaxed(regs + ZX2967_THERMAL_CTRL);
> + }
> +
> + writel_relaxed(3, regs + ZX2967_THERMAL_DCF);
> + val = readl_relaxed(regs + ZX2967_THERMAL_CTRL)
> + & ZX2967_THERMAL_TEMP_MASK;
> + writel_relaxed(1, regs + ZX2967_THERMAL_POWER_MODE);
> +
> + /** Calculate temperature */
/* ... */
> + *temp = DIV_ROUND_CLOSEST((val - 922) * 1000, 1951);
We should probably either have defines for these magic numbers or add
some comments for the formula?
Shawn
> +
> + mutex_unlock(&priv->lock);
> +
> + return 0;
> +}
> +
> +static struct thermal_zone_of_device_ops zx2967_of_thermal_ops = {
> + .get_temp = zx2967_thermal_get_temp,
> +};
> +
> +static int zx2967_thermal_probe(struct platform_device *pdev)
> +{
> + struct zx2967_thermal_priv *priv;
> + struct resource *res;
> + int ret, i;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + priv->regs = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->regs))
> + return PTR_ERR(priv->regs);
> +
> + priv->clk_gate = devm_clk_get(&pdev->dev, "gate");
> + if (IS_ERR(priv->clk_gate)) {
> + ret = PTR_ERR(priv->clk_gate);
> + dev_err(&pdev->dev, "failed to get clock gate: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(priv->clk_gate);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
> + ret);
> + return ret;
> + }
> +
> + priv->pclk = devm_clk_get(&pdev->dev, "pclk");
> + if (IS_ERR(priv->pclk)) {
> + ret = PTR_ERR(priv->pclk);
> + dev_err(&pdev->dev, "failed to get apb clock: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(priv->pclk);
> + if (ret) {
> + clk_disable_unprepare(priv->clk_gate);
> + dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
> + ret);
> + return ret;
> + }
> +
> + mutex_init(&priv->lock);
> + for (i = 0; i < NUM_SENSORS; i++) {
> + struct zx2967_thermal_sensor *sensor = &priv->sensors[i];
> +
> + sensor->priv = priv;
> + sensor->id = i;
> + sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev,
> + i, sensor, &zx2967_of_thermal_ops);
> + if (IS_ERR(sensor->tzd)) {
> + ret = PTR_ERR(sensor->tzd);
> + dev_err(&pdev->dev, "failed to register sensor %d: %d\n",
> + i, ret);
> + goto remove_ts;
> + }
> + }
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +
> +remove_ts:
> + clk_disable_unprepare(priv->clk_gate);
> + clk_disable_unprepare(priv->pclk);
> + for (i--; i >= 0; i--)
> + thermal_zone_of_sensor_unregister(&pdev->dev,
> + priv->sensors[i].tzd);
> +
> + return ret;
> +}
> +
> +static int zx2967_thermal_exit(struct platform_device *pdev)
> +{
> + struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
> + int i;
> +
> + for (i = 0; i < NUM_SENSORS; i++) {
> + struct zx2967_thermal_sensor *sensor = &priv->sensors[i];
> +
> + thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
> + }
> + clk_disable_unprepare(priv->pclk);
> + clk_disable_unprepare(priv->clk_gate);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id zx2967_thermal_id_table[] = {
> + { .compatible = "zte,zx296718-thermal" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, zx2967_thermal_id_table);
> +
> +static SIMPLE_DEV_PM_OPS(zx2967_thermal_pm_ops,
> + zx2967_thermal_suspend, zx2967_thermal_resume);
> +
> +static struct platform_driver zx2967_thermal_driver = {
> + .probe = zx2967_thermal_probe,
> + .remove = zx2967_thermal_exit,
> + .driver = {
> + .name = "zx2967_thermal",
> + .of_match_table = zx2967_thermal_id_table,
> + .pm = &zx2967_thermal_pm_ops,
> + },
> +};
> +module_platform_driver(zx2967_thermal_driver);
> +
> +MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
> +MODULE_DESCRIPTION("ZTE zx2967 thermal driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v2] Documentation: dt: reset: Revise typos in TI syscon reset example
From: Suman Anna @ 2017-01-12 1:22 UTC (permalink / raw)
To: Philipp Zabel
Cc: devicetree, linux-kernel, Andrew Davis, Rob Herring,
Santosh Shilimkar, linux-arm-kernel
In-Reply-To: <20170109192814.26811-1-s-anna@ti.com>
Fix couple of typos in the example given in the TI syscon reset
binding. The ti,reset-bits used for DSP0 are corrected to match
the values that will be used in the actual DT node.
Signed-off-by: Suman Anna <s-anna@ti.com>
---
v2: Address Rob Herring's comment to change the reset node name
from "psc-reset" to "reset-controller"
Documentation/devicetree/bindings/reset/ti-syscon-reset.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
index 164c7f34c451..c516d24959f2 100644
--- a/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
+++ b/Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
@@ -63,7 +63,7 @@ Example:
--------
The following example demonstrates a syscon node, the reset controller node
using the syscon node, and a consumer (a DSP device) on the TI Keystone 2
-Edison SoC.
+66AK2E SoC.
/ {
soc {
@@ -71,13 +71,13 @@ Edison SoC.
compatible = "syscon", "simple-mfd";
reg = <0x02350000 0x1000>;
- pscrst: psc-reset {
+ pscrst: reset-controller {
compatible = "ti,k2e-pscrst", "ti,syscon-reset";
#reset-cells = <1>;
ti,reset-bits = <
- 0xa3c 8 0xa3c 8 0x83c 8 (ASSERT_SET|DEASSERT_CLEAR|STATUS_SET) /* 0: pcrst-dsp0 */
- 0xa40 5 0xa44 3 0 0 (ASSERT_SET|DEASSERT_CLEAR|STATUS_NONE) /* 1: pcrst-example */
+ 0xa3c 8 0xa3c 8 0x83c 8 (ASSERT_CLEAR | DEASSERT_SET | STATUS_CLEAR) /* 0: dsp0 */
+ 0xa40 5 0xa44 3 0 0 (ASSERT_SET | DEASSERT_CLEAR | STATUS_NONE) /* 1: example */
>;
};
};
--
2.10.2
^ permalink raw reply related
* RE: [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
From: Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA @ 2017-01-12 1:25 UTC (permalink / raw)
To: linux-I+IVW8TIWO2tmTQ+vhA3Yw, jjhiblot-Re5JQEeQqe8AvxtiuMwx3w
Cc: alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170111111814.GJ14217-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4856 bytes --]
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
> Sent: 2017年1月11日 19:18
> To: Jean-Jacques Hiblot <jjhiblot@gmail.com>
> Cc: Wenyou Yang - A41535 <Wenyou.Yang@microchip.com>; Alexandre Belloni
> <alexandre.belloni@free-electrons.com>; Mark Rutland <mark.rutland@arm.com>;
> devicetree <devicetree@vger.kernel.org>; Nicolas Ferre
> <nicolas.ferre@atmel.com>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; robh+dt <robh+dt@kernel.org>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH 1/3] ARM: at91: flush the L2 cache before entering cpu idle
>
> On Wed, Jan 11, 2017 at 12:05:05PM +0100, Jean-Jacques Hiblot wrote:
> > 2017-01-11 9:15 GMT+01:00 <Wenyou.Yang@microchip.com>:
> > > Hi Jean-Jacques,
> > >
> > >> -----Original Message-----
> > >> From: Jean-Jacques Hiblot [mailto:jjhiblot@gmail.com]
> > >> Sent: 2017年1月11日 0:51
> > >> To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > >> Cc: Wenyou Yang - A41535 <Wenyou.Yang@microchip.com>; Mark Rutland
> > >> <mark.rutland@arm.com>; devicetree <devicetree@vger.kernel.org>;
> > >> Russell King <linux@arm.linux.org.uk>; Wenyou Yang - A41535
> > >> <Wenyou.Yang@microchip.com>; Nicolas Ferre
> > >> <nicolas.ferre@atmel.com>; Linux Kernel Mailing List
> > >> <linux-kernel@vger.kernel.org>; Rob Herring <robh+dt@kernel.org>;
> > >> linux-arm-kernel@lists.infradead.org
> > >> Subject: Re: [PATCH 1/3] ARM: at91: flush the L2 cache before
> > >> entering cpu idle
> > >>
> > >> 2017-01-10 17:18 GMT+01:00 Alexandre Belloni
> > >> <alexandre.belloni@free-electrons.com>:
> > >> > I though a bit more about it, and I don't really like the new
> > >> > compatible string. I don't feel this should be necessary.
> > >> >
> > >> > What about the following:
> > >> >
> > >> > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> > >> > index
> > >> > b4332b727e9c..0333aca63e44 100644
> > >> > --- a/arch/arm/mach-at91/pm.c
> > >> > +++ b/arch/arm/mach-at91/pm.c
> > >> > @@ -53,6 +53,7 @@ extern void at91_pinctrl_gpio_resume(void);
> > >> > static struct {
> > >> > unsigned long uhp_udp_mask;
> > >> > int memctrl;
> > >> > + bool has_l2_cache;
> > >> > } at91_pm_data;
> > >> >
> > >> > void __iomem *at91_ramc_base[2]; @@ -267,6 +268,11 @@ static
> > >> > void at91_ddr_standby(void)
> > >> > u32 lpr0, lpr1 = 0;
> > >> > u32 saved_lpr0, saved_lpr1 = 0;
> > >> >
> > >>
> > >> > + if (at91_pm_data.has_l2_cache) {
> > >> > + flush_cache_all();
> > >> what is the point of calling flush_cache_all() here ? Do we really
> > >> care that dirty data in L1 is written to DDR ? I may be missing
> > >> something but to me it's just extra latency.
> > >
> > > Are you mean use outer_flush_all() to flush all cache lines in the outer cache
> only?
> >
> > Yes that's what I meant. You see, you don't flush the cache for
> > sama5d3 so it shouldn't be required either for sam5d4. You should be
> > able to test it quickly and see if L1 flush is indeed required by
> > replacing flush_cache_all() with outer_flush_all(). BTW is highly
> > probable that L2 cache flush is done in outer_disable() so calling
> > outer_flush_all() is probably no required.
>
> Please don't. Read the comments in the code, and understand the APIs that
> you're suggesting people use _before_ making the suggestion:
>
> /**
> * outer_flush_all - clean and invalidate all cache lines in the outer cache
> *
> * Note: depending on implementation, this may not be atomic - it must
> * only be called with interrupts disabled and no other active outer
> * cache masters.
> *
> * It is intended that this function is only used by implementations
> * needing to override the outer_cache.disable() method due to security.
> * (Some implementations perform this as a clean followed by an invalidate.) */
>
> So, outer_flush_all() should not be called except from L2 cache code
> implementing the outer_disable() function - it's not intended for platforms to use.
>
> There are, however, sadly three users of outer_flush_all() which have crept in
> through arm-soc, that should be outer_disable() instead.
Here, outer_flush_all() should not be called, calling outer_disable() is enough. Is it right?
In the implementation of l2c_disable(void) of in mm/cache-l2x0.c, the outer_cache.flush_all() is called.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
Best Regards,
Wenyou Yang
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i
^ permalink raw reply
* Re: [PATCH v4 1/2] arm64: dts: rockchip: add "rockchip, grf" property for RK3399 PMUCRU/CRU
From: Heiko Stuebner @ 2017-01-12 1:28 UTC (permalink / raw)
To: Doug Anderson
Cc: Xing Zheng, open list:ARM/Rockchip SoC..., Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon, Caesar Wang,
Shawn Lin, Brian Norris, Jianqun Xu, Elaine Zhang, David Wu,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAD=FV=WMQ22G67YoQx2t7J6_ZuB3sUjVjXh-0KhPEO_6zbiyUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Am Mittwoch, 11. Januar 2017, 16:50:10 CET schrieb Doug Anderson:
> Hi,
>
> On Tue, Jan 10, 2017 at 11:58 AM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> > Hi Doug,
> >
> > Am Dienstag, 10. Januar 2017, 20:46:12 schrieb Heiko Stübner:
> >> Am Dienstag, 10. Januar 2017, 10:45:48 schrieb Doug Anderson:
> >> > Hi,
> >> >
> >> > On Mon, Jan 9, 2017 at 10:15 PM, Xing Zheng <zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >>
> >> wrote:
> >> > > The structure rockchip_clk_provider needs to refer the GRF regmap
> >> > > in somewhere, if the CRU node has not "rockchip,grf" property,
> >> > > calling syscon_regmap_lookup_by_phandle will return an invalid GRF
> >> > > regmap, and the MUXGRF type clock will be not supported.
> >> > >
> >> > > Therefore, we need to add them.
> >> > >
> >> > > Signed-off-by: Xing Zheng <zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >> > > ---
> >> > >
> >> > > Changes in v4:
> >> > > - separte the binding patch
> >> > >
> >> > > Changes in v3:
> >> > > - add optional roperty rockchip,grf in rockchip,rk3399-cru.txt
> >> > >
> >> > > Changes in v2:
> >> > > - referring pmugrf for PMUGRU
> >> > > - fix the typo "invaild" in COMMIT message
> >> > >
> >> > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 ++
> >> > > 1 file changed, 2 insertions(+)
> >> >
> >> > This seems fine to me, so:
> >> >
> >> > Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> >
> >> > ...but I will say that before you actually add any real "MUXGRF"
> >> > clocks on rk3399 you _might_ need to rework the code to make things
> >> > truly "optional". If it turns out that any existing clocks that
> >> > already exist today already go through one of these muxes in the GRF
> >> > and we've always been assuming one setting of the mux, we'll need to
> >> > make sure we keep assuming that setting of the mux even if the "grf"
> >> > isn't specified.
> >>
> >> I guess I see that a bit more relaxed :-) .
> >>
> >> I.e. the GRF being optional is a remnant of syscons not being available
> >> when the clocks get set up- so were coming in later or not at all. For
> >> the rk3288 I converted, there we never really had the case of the GRF
> >> missing.
> >>
> >> And the GRF mux for the vcodec now present is not being used by anything
> >> yet (neither driver nor binding), so no old devicetree can break.
> >>
> >> > As I understand it, your motivation for this patch is to eventually be
> >> > able to model the EDP reference clock which can either be xin24 or
> >> > "edp osc". Presumably the eDP "reference clock" isn't related to any
> >> > of the pre-existing eDP clocks so that one should be safe.
> >>
> >> Same here, so far we don't even have edp or even any other graphical
> >> output
> >> on the rk3399, so again there is no old devicetree that could break when
> >> the grf is not specified.
> >
> > reading all of the above again, it feels like you essentially also said
> > similar things already in your original reply and I misread some of it.
> >
> > But again, I don't see the need for any more code right now, as hopefully
> > the simple stuff we currently only support does not have any grf-based
> > muxes in it. Xing + Rockchip people, please correct me if I'm wrong here
> > :-)
> Right. I have no objection to Xing's patch. I just want to make sure
> that if it's listed as "Optional" that it's really optional.
>
> I was worried that we would introduce some GRF-based mux in the
> _middle_ of some existing clock tree because we simply didn't model
> the mux before and assumed one particular setting. If nothing like
> that ever happens then we're fine.
Thankfully the clock diagrams on the old socs were pretty verbose, listing all
grf-based clocks as well. For example the rk3288 seems to have two and it
seems I've been carrying dummy definitions for those from the time I did the
initial clock tree [0].
And thankfully grf-based clocks somehow always only get used for more
"esotheric" components like vcodec and iep on the rk3288 :-) .
Heiko
[0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/drivers/clk/rockchip/clk-rk3288.c
lines 371 and 799 .. looks like I'll need to add the iep clock as well
at some point.
--
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