* Re: [PATCH V1] mfd: pv88080: Expand driver for GPIO function support.
From: Lee Jones @ 2016-10-25 6:40 UTC (permalink / raw)
To: Eric Jeong
Cc: Alexandre Courbot, DEVICETREE, LINUX-GPIO, LINUX-KERNEL,
Liam Girdwood, Linus Walleij, Mark Brown, Mark Rutland,
Rob Herring, Support Opensource
In-Reply-To: <20161025014148.7DF655FA17@krsrvapps-01.diasemi.com>
On Tue, 25 Oct 2016, Eric Jeong wrote:
>
> From: Eric Jeong <eric.jeong.opensource@diasemi.com>
>
> This patch adds support for the PV88080 PMIC.
>
> This pathch is done as part of the existing PV88080 regulator driver
> by expanding the driver for GPIO function support.
>
> The MFD core driver provides communication through the I2C interface.
> and contains the following components:
>
> - Regulators
> - Configurable GPIOs
>
> Kconfig and Makefile are updated to reflect support for PV88080 PMIC.
>
> Signed-off-by: Eric Jeong <eric.jeong.opensource@diasemi.com>
>
> ---
> This patch applies against linux-next and next-20161024
>
> Hi,
>
> This change is made as a single patch. Because, to ensure that
> kernel builds and runs properly after this patch.
>
> The regulator device driver for PV88080 IC is submitted to Linux kernel.
> And now, GPIO function is required. In order to add GPIO driver,
> MFD driver is also required.
>
> Changes
> - Add MFD driver.
> - Add GPIO driver.
> - Update regulator driver to reflect the support.
> - Delete pv88080-regulator.h file.
> - Move binding document to mfd directory.
>
> Regards,
> Eric Jeong, Dialog Semiconductor Ltd.
>
>
> Documentation/devicetree/bindings/mfd/pv88080.txt | 63 +++++
> .../devicetree/bindings/regulator/pv88080.txt | 62 -----
> drivers/gpio/Kconfig | 11 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-pv88080.c | 195 ++++++++++++++
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/pv88080-core.c | 270 ++++++++++++++++++++
> drivers/mfd/pv88080-i2c.c | 99 +++++++
> drivers/regulator/Kconfig | 5 +-
> drivers/regulator/pv88080-regulator.c | 202 ++++++---------
> drivers/regulator/pv88080-regulator.h | 118 ---------
> include/linux/mfd/pv88080.h | 236 +++++++++++++++++
> 13 files changed, 970 insertions(+), 306 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/pv88080.txt
> delete mode 100644 Documentation/devicetree/bindings/regulator/pv88080.txt
> create mode 100644 drivers/gpio/gpio-pv88080.c
> create mode 100644 drivers/mfd/pv88080-core.c
> create mode 100644 drivers/mfd/pv88080-i2c.c
> delete mode 100644 drivers/regulator/pv88080-regulator.h
> create mode 100644 include/linux/mfd/pv88080.h
You're going to need to split this patch up as much as possible.
No one is going to want to review a 1200 line patch.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Daniel Vetter @ 2016-10-25 6:44 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Maxime Ripard,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <8afc5e020c5767face34fe3a9ab300ce9e67ba00.1477142934.git.moinejf-GANU6spQydw@public.gmane.org>
On Fri, Oct 21, 2016 at 09:26:18AM +0200, Jean-Francois Moine wrote:
> +static int de2_drm_bind(struct device *dev)
> +{
> + struct drm_device *drm;
> + struct priv *priv;
> + int ret;
> +
> + drm = drm_dev_alloc(&de2_drm_driver, dev);
> + if (!drm)
> + return -ENOMEM;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + dev_err(dev, "failed to allocate private area\n");
> + ret = -ENOMEM;
> + goto out1;
> + }
> +
> + dev_set_drvdata(dev, drm);
> + drm->dev_private = priv;
> +
> + drm_mode_config_init(drm);
> + drm->mode_config.min_width = 32; /* needed for cursor */
> + drm->mode_config.min_height = 32;
> + drm->mode_config.max_width = 1920;
> + drm->mode_config.max_height = 1080;
> + drm->mode_config.funcs = &de2_mode_config_funcs;
> +
> + drm->irq_enabled = true;
> +
> + /* initialize the display engine */
> + priv->soc_type = (int) of_match_device(de2_drm_of_match, dev)->data;
> + ret = de2_de_init(priv, dev);
> + if (ret)
> + goto out2;
> +
> + /* start the subdevices */
> + ret = component_bind_all(dev, drm);
> + if (ret < 0)
> + goto out2;
> +
> + ret = drm_dev_register(drm, 0);
This needs to be the very last step in your driver load sequence.
Kerneldoc explains why. Similar, but inverted for unloading:
drm_dev_unregister is the very first thing you must call.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Daniel Vetter @ 2016-10-25 6:46 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Maxime Ripard,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <20161025064422.4jua6qmpr7zu3ijt-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>
On Tue, Oct 25, 2016 at 08:44:22AM +0200, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 09:26:18AM +0200, Jean-Francois Moine wrote:
> > +static int de2_drm_bind(struct device *dev)
> > +{
> > + struct drm_device *drm;
> > + struct priv *priv;
> > + int ret;
> > +
> > + drm = drm_dev_alloc(&de2_drm_driver, dev);
Oh and you might want to look into drm_dev_init, allows you to use
subclassing instead of pointer chasing for your driver-private data.
-Daniel
> > + if (!drm)
> > + return -ENOMEM;
> > +
> > + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > + if (!priv) {
> > + dev_err(dev, "failed to allocate private area\n");
> > + ret = -ENOMEM;
> > + goto out1;
> > + }
> > +
> > + dev_set_drvdata(dev, drm);
> > + drm->dev_private = priv;
> > +
> > + drm_mode_config_init(drm);
> > + drm->mode_config.min_width = 32; /* needed for cursor */
> > + drm->mode_config.min_height = 32;
> > + drm->mode_config.max_width = 1920;
> > + drm->mode_config.max_height = 1080;
> > + drm->mode_config.funcs = &de2_mode_config_funcs;
> > +
> > + drm->irq_enabled = true;
> > +
> > + /* initialize the display engine */
> > + priv->soc_type = (int) of_match_device(de2_drm_of_match, dev)->data;
> > + ret = de2_de_init(priv, dev);
> > + if (ret)
> > + goto out2;
> > +
> > + /* start the subdevices */
> > + ret = component_bind_all(dev, drm);
> > + if (ret < 0)
> > + goto out2;
> > +
> > + ret = drm_dev_register(drm, 0);
>
> This needs to be the very last step in your driver load sequence.
> Kerneldoc explains why. Similar, but inverted for unloading:
> drm_dev_unregister is the very first thing you must call.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
--
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] pinctrl: Add SX150X GPIO Extender Pinctrl Driver
From: Neil Armstrong @ 2016-10-25 6:57 UTC (permalink / raw)
To: Linus Walleij, Andrey Smirnov
Cc: Peter Rosin, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
Roland Stigge, Vladimir Zapolskiy
In-Reply-To: <CACRpkdY7vLU4yVP7M-O4rGt_L5cqL57ApmMQJ4aRGgzh3Nat6Q@mail.gmail.com>
Hi Linus,
Le 24/10/2016 16:39, Linus Walleij a écrit :
> On Mon, Oct 24, 2016 at 6:51 AM, Andrey Smirnov
> <andrew.smirnov@gmail.com> wrote:
>
>> It seem strange to me that the driver uses "handle_edge_irq", given
>> how none of the individual interrupts seem to require any ACKing,
>> since it is all handled in sx150x_irq_thread_fn(), line 533. More so,
>> I had trouble finding who/where sets .irq_ack() callback, which AFAIU
>> is mandatory for handle_edge_irq().
>
> Yes that looks strange.
>
> Neil have you tested IRQs with this code?
To be frank, I took the IRQ code from the GPIO driver verbatim, and only tested
a simple use case on beagle bone black.
The interrupt code is very complex and sincerely I was not able to understand
clearly how it worked.
The IRQ_TYPE_EDGE_BOTH did work for me since the BBB irq controller dealt with it.
I will dig in the datasheet and run more uses cases next week, but since the
code hasn't changed since the GPIO driver, so this is a nice to have !
>
> If there is trouble, please follow up with a fix for the edge handler.
> Maybe it should just be handle_simple_irq().
>
> Yours,
> Linus Walleij
>
Andrey, can we talk over irc somehow ? I'm idling as narmstrong on freenode.
Neil
^ permalink raw reply
* RE: [PATCH V1] mfd: pv88080: Expand driver for GPIO function support.
From: Eric Hyeung Dong Jeong @ 2016-10-25 7:11 UTC (permalink / raw)
To: Lee Jones, Eric Hyeung Dong Jeong
Cc: Alexandre Courbot, DEVICETREE, LINUX-GPIO, LINUX-KERNEL,
Liam Girdwood, Linus Walleij, Mark Brown, Mark Rutland,
Rob Herring, Support Opensource
In-Reply-To: <20161025064040.GE8574@dell>
On Tuesday, October 25, 2016 3:41 PM, Lee Jones wrote:
> On Tue, 25 Oct 2016, Eric Jeong wrote:
>
> >
> > From: Eric Jeong <eric.jeong.opensource@diasemi.com>
> >
> > This patch adds support for the PV88080 PMIC.
> >
> > This pathch is done as part of the existing PV88080 regulator driver
> > by expanding the driver for GPIO function support.
> >
> > The MFD core driver provides communication through the I2C interface.
> > and contains the following components:
> >
> > - Regulators
> > - Configurable GPIOs
> >
> > Kconfig and Makefile are updated to reflect support for PV88080 PMIC.
> >
> > Signed-off-by: Eric Jeong <eric.jeong.opensource@diasemi.com>
> >
> > ---
> > This patch applies against linux-next and next-20161024
> >
> > Hi,
> >
> > This change is made as a single patch. Because, to ensure that kernel
> > builds and runs properly after this patch.
> >
> > The regulator device driver for PV88080 IC is submitted to Linux kernel.
> > And now, GPIO function is required. In order to add GPIO driver, MFD
> > driver is also required.
> >
> > Changes
> > - Add MFD driver.
> > - Add GPIO driver.
> > - Update regulator driver to reflect the support.
> > - Delete pv88080-regulator.h file.
> > - Move binding document to mfd directory.
> >
> > Regards,
> > Eric Jeong, Dialog Semiconductor Ltd.
> >
> >
> > Documentation/devicetree/bindings/mfd/pv88080.txt | 63 +++++
> > .../devicetree/bindings/regulator/pv88080.txt | 62 -----
> > drivers/gpio/Kconfig | 11 +
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/gpio-pv88080.c | 195 ++++++++++++++
> > drivers/mfd/Kconfig | 12 +
> > drivers/mfd/Makefile | 2 +
> > drivers/mfd/pv88080-core.c | 270 ++++++++++++++++++++
> > drivers/mfd/pv88080-i2c.c | 99 +++++++
> > drivers/regulator/Kconfig | 5 +-
> > drivers/regulator/pv88080-regulator.c | 202 ++++++---------
> > drivers/regulator/pv88080-regulator.h | 118 ---------
> > include/linux/mfd/pv88080.h | 236 +++++++++++++++++
> > 13 files changed, 970 insertions(+), 306 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/mfd/pv88080.txt
> > delete mode 100644
> > Documentation/devicetree/bindings/regulator/pv88080.txt
> > create mode 100644 drivers/gpio/gpio-pv88080.c create mode 100644
> > drivers/mfd/pv88080-core.c create mode 100644
> > drivers/mfd/pv88080-i2c.c delete mode 100644
> > drivers/regulator/pv88080-regulator.h
> > create mode 100644 include/linux/mfd/pv88080.h
>
> You're going to need to split this patch up as much as possible.
>
> No one is going to want to review a 1200 line patch.
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source
> software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Sorry for inconvenience.
I will separate the patch and send again.
Regards
Eric
^ permalink raw reply
* Re: [PATCH] leds: leds-pca963x: workaround group blink scaling issue
From: Jacek Anaszewski @ 2016-10-25 7:15 UTC (permalink / raw)
To: Matt Ranostay, linux-kernel, devicetree
Cc: Matt Ranostay, Tony Lindgren, Rob Herring
In-Reply-To: <1476364572-26849-1-git-send-email-matt@ranostay.consulting>
Hi Matt,
Patch applied.
Thanks,
Jacek Anaszewski
On 10/13/2016 03:16 PM, Matt Ranostay wrote:
> PCA9632TK part seems to incorrectly blink at ~1.3x of the programmed
> rate. This patchset add a nxp,period-scale devicetree property to
> adjust for this misconfiguration.
>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
> Documentation/devicetree/bindings/leds/pca963x.txt | 3 +++
> drivers/leds/leds-pca963x.c | 18 +++++++++++++++---
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/leds/pca963x.txt b/Documentation/devicetree/bindings/leds/pca963x.txt
> index dafbe9931c2b..dfbdb123a9bf 100644
> --- a/Documentation/devicetree/bindings/leds/pca963x.txt
> +++ b/Documentation/devicetree/bindings/leds/pca963x.txt
> @@ -7,6 +7,9 @@ Optional properties:
> - nxp,totem-pole : use totem pole (push-pull) instead of open-drain (pca9632 defaults
> to open-drain, newer chips to totem pole)
> - nxp,hw-blink : use hardware blinking instead of software blinking
> +- nxp,period-scale : In some configurations, the chip blinks faster than expected.
> + This parameter provides a scaling ratio (fixed point, decimal divided
> + by 1000) to compensate, e.g. 1300=1.3x and 750=0.75x.
>
> Each led is represented as a sub-node of the nxp,pca963x device.
>
> diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
> index 407eba11e187..b6ce1f2ec33e 100644
> --- a/drivers/leds/leds-pca963x.c
> +++ b/drivers/leds/leds-pca963x.c
> @@ -59,6 +59,7 @@ struct pca963x_chipdef {
> u8 grpfreq;
> u8 ledout_base;
> int n_leds;
> + unsigned int scaling;
> };
>
> static struct pca963x_chipdef pca963x_chipdefs[] = {
> @@ -189,6 +190,14 @@ static int pca963x_led_set(struct led_classdev *led_cdev,
> return pca963x_brightness(pca963x, value);
> }
>
> +static unsigned int pca963x_period_scale(struct pca963x_led *pca963x,
> + unsigned int val)
> +{
> + unsigned int scaling = pca963x->chip->chipdef->scaling;
> +
> + return scaling ? DIV_ROUND_CLOSEST(val * scaling, 1000) : val;
> +}
> +
> static int pca963x_blink_set(struct led_classdev *led_cdev,
> unsigned long *delay_on, unsigned long *delay_off)
> {
> @@ -207,14 +216,14 @@ static int pca963x_blink_set(struct led_classdev *led_cdev,
> time_off = 500;
> }
>
> - period = time_on + time_off;
> + period = pca963x_period_scale(pca963x, time_on + time_off);
>
> /* If period not supported by hardware, default to someting sane. */
> if ((period < PCA963X_BLINK_PERIOD_MIN) ||
> (period > PCA963X_BLINK_PERIOD_MAX)) {
> time_on = 500;
> time_off = 500;
> - period = time_on + time_off;
> + period = pca963x_period_scale(pca963x, 1000);
> }
>
> /*
> @@ -222,7 +231,7 @@ static int pca963x_blink_set(struct led_classdev *led_cdev,
> * (time_on / period) = (GDC / 256) ->
> * GDC = ((time_on * 256) / period)
> */
> - gdc = (time_on * 256) / period;
> + gdc = (pca963x_period_scale(pca963x, time_on) * 256) / period;
>
> /*
> * From manual: period = ((GFRQ + 1) / 24) in seconds.
> @@ -294,6 +303,9 @@ pca963x_dt_init(struct i2c_client *client, struct pca963x_chipdef *chip)
> else
> pdata->blink_type = PCA963X_SW_BLINK;
>
> + if (of_property_read_u32(np, "nxp,period-scale", &chip->scaling))
> + chip->scaling = 1000;
> +
> return pdata;
> }
>
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH RESEND 1/2] dt: bindings: add allwinner,otg-routed property for phy-sun4i-usb
From: Maxime Ripard @ 2016-10-25 7:18 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Rob Herring, Chen-Yu Tsai, Kishon Vijay Abraham I, Hans de Goede,
Mark Rutland, Reinder de Haan, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161025041139.46454-1-icenowy-ymACFijhrKM@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
Hi,
On Tue, Oct 25, 2016 at 12:11:38PM +0800, Icenowy Zheng wrote:
> On some newer Allwinner SoCs (H3 or A64), the PHY0 can be either routed to
> the MUSB controller (which is an OTG controller) or the OHCI/EHCI pair
> (which is a Host-only controller, but more stable and easy to implement).
>
> This property marks whether on a certain board which controller should be
> attached to the PHY.
>
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Didn't Hans reject this already? Why are you resending it?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver
From: Axel Haslam @ 2016-10-25 7:39 UTC (permalink / raw)
To: David Lechner
Cc: Greg KH, Johan Hovold, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Sekhar Nori, Alan Stern, Kevin Hilman, Sergei Shtylyov,
manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A, Mark Brown,
Alexandre Bailon, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <403743d8-dff2-3389-105b-1082b674b0b8-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
On Tue, Oct 25, 2016 at 2:38 AM, David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> wrote:
> On 10/24/2016 11:46 AM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote:
>>
>> From: Manjunath Goudar <manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> Separate the Davinci OHCI host controller driver from ohci-hcd
>> host code so that it can be built as a separate driver module.
>> This work is part of enabling multi-platform kernels on ARM;
>> it would be nice to have in 3.11.
>
>
> No need for comment about kernel 3.11.
yes, ok.
>
>>
>> Signed-off-by: Manjunath Goudar <manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> drivers/usb/host/Kconfig | 2 +-
>> drivers/usb/host/Makefile | 1 +
>> drivers/usb/host/ohci-da8xx.c | 185
>> +++++++++++++++++-------------------------
>> drivers/usb/host/ohci-hcd.c | 18 ----
>> 4 files changed, 76 insertions(+), 130 deletions(-)
>>
>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>> index 83b6cec..642c6fe8 100644
>> --- a/drivers/usb/host/Kconfig
>> +++ b/drivers/usb/host/Kconfig
>> @@ -479,7 +479,7 @@ config USB_OHCI_HCD_OMAP3
>> OMAP3 and later chips.
>>
>> config USB_OHCI_HCD_DAVINCI
>> - bool "OHCI support for TI DaVinci DA8xx"
>> + tristate "OHCI support for TI DaVinci DA8xx"
>> depends on ARCH_DAVINCI_DA8XX
>> depends on USB_OHCI_HCD=y
>
>
> Need to drop the "=y" here, otherwise you still can't compile this as a
> module.
Im able to complile it as a module, but ok ill remove it.
>
>> select PHY_DA8XX_USB
>> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
>> index 6ef785b..2644537 100644
>> --- a/drivers/usb/host/Makefile
>> +++ b/drivers/usb/host/Makefile
>> @@ -61,6 +61,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o
>> obj-$(CONFIG_USB_OHCI_HCD_S3C2410) += ohci-s3c2410.o
>> obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
>> obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o
>> +obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
>>
>> obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
>> obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
>> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
>> index e98066d..5585d9e 100644
>> --- a/drivers/usb/host/ohci-da8xx.c
>> +++ b/drivers/usb/host/ohci-da8xx.c
>> @@ -11,16 +11,31 @@
>> * kind, whether express or implied.
>> */
>>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> #include <linux/interrupt.h>
>> #include <linux/jiffies.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> #include <linux/platform_device.h>
>> -#include <linux/clk.h>
>> #include <linux/phy/phy.h>
>> #include <linux/platform_data/usb-davinci.h>
>> +#include <linux/platform_device.h>
>
>
> linux/platform_device.h is listed twice
>
>> +#include <linux/usb.h>
>> +#include <linux/usb/hcd.h>
>> +#include <asm/unaligned.h>
>>
>> -#ifndef CONFIG_ARCH_DAVINCI_DA8XX
>> -#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
>> -#endif
>> +#include "ohci.h"
>> +
>> +#define DRIVER_DESC "OHCI DA8XX driver"
>> +
>> +static const char hcd_name[] = "ohci-da8xx";
>
>
> why static const char instead of #define? This is only used one time in a
> pr_info, so it seems kind of pointless anyway.
Other drivers are using static const for the same variable.
i think static const is preferred over #define because #define doet give a type.
If you dont mind ill keep it static const.
>
>> +
>> +static struct hc_driver __read_mostly ohci_da8xx_hc_driver;
>> +
>> +static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
>> + u16 wValue, u16 wIndex, char *buf, u16 wLength);
>> +static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
>>
>> static struct clk *usb11_clk;
>> static struct phy *usb11_phy;
>> @@ -73,7 +88,7 @@ static void ohci_da8xx_ocic_handler(struct
>> da8xx_ohci_root_hub *hub)
>> hub->set_power(0);
>> }
>>
>> -static int ohci_da8xx_init(struct usb_hcd *hcd)
>> +static int ohci_da8xx_reset(struct usb_hcd *hcd)
>> {
>> struct device *dev = hcd->self.controller;
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>> @@ -93,7 +108,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
>> */
>> ohci->num_ports = 1;
>>
>> - result = ohci_init(ohci);
>> + result = ohci_setup(hcd);
>> if (result < 0) {
>> ohci_da8xx_disable();
>> return result;
>> @@ -121,30 +136,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
>> return result;
>> }
>>
>> -static void ohci_da8xx_stop(struct usb_hcd *hcd)
>> -{
>> - ohci_stop(hcd);
>> - ohci_da8xx_disable();
>> -}
>> -
>> -static int ohci_da8xx_start(struct usb_hcd *hcd)
>> -{
>> - struct ohci_hcd *ohci = hcd_to_ohci(hcd);
>> - int result;
>> -
>> - result = ohci_run(ohci);
>> - if (result < 0)
>> - ohci_da8xx_stop(hcd);
>> -
>> - return result;
>> -}
>> -
>> /*
>> * Update the status data from the hub with the over-current indicator
>> change.
>> */
>> static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
>> {
>> - int length = ohci_hub_status_data(hcd, buf);
>> + int length = orig_ohci_hub_status_data(hcd, buf);
>>
>> /* See if we have OCIC flag set */
>> if (ocic_flag) {
>> @@ -226,66 +223,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd
>> *hcd, u16 typeReq, u16 wValue,
>> }
>> }
>>
>> - return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf,
>> wLength);
>> + return orig_ohci_hub_control(hcd, typeReq, wValue,
>> + wIndex, buf, wLength);
>> }
>>
>> -static const struct hc_driver ohci_da8xx_hc_driver = {
>> - .description = hcd_name,
>> - .product_desc = "DA8xx OHCI",
>> - .hcd_priv_size = sizeof(struct ohci_hcd),
>> -
>> - /*
>> - * generic hardware linkage
>> - */
>> - .irq = ohci_irq,
>> - .flags = HCD_USB11 | HCD_MEMORY,
>> -
>> - /*
>> - * basic lifecycle operations
>> - */
>> - .reset = ohci_da8xx_init,
>> - .start = ohci_da8xx_start,
>> - .stop = ohci_da8xx_stop,
>> - .shutdown = ohci_shutdown,
>> -
>> - /*
>> - * managing i/o requests and associated device resources
>> - */
>> - .urb_enqueue = ohci_urb_enqueue,
>> - .urb_dequeue = ohci_urb_dequeue,
>> - .endpoint_disable = ohci_endpoint_disable,
>> -
>> - /*
>> - * scheduling support
>> - */
>> - .get_frame_number = ohci_get_frame,
>> -
>> - /*
>> - * root hub support
>> - */
>> - .hub_status_data = ohci_da8xx_hub_status_data,
>> - .hub_control = ohci_da8xx_hub_control,
>> -
>> -#ifdef CONFIG_PM
>> - .bus_suspend = ohci_bus_suspend,
>> - .bus_resume = ohci_bus_resume,
>> -#endif
>> - .start_port_reset = ohci_start_port_reset,
>> -};
>> -
>>
>> /*-------------------------------------------------------------------------*/
>>
>> -
>> -/**
>> - * usb_hcd_da8xx_probe - initialize DA8xx-based HCDs
>> - * Context: !in_interrupt()
>> - *
>> - * Allocates basic resources for this USB host controller, and
>> - * then invokes the start() method for the HCD associated with it
>> - * through the hotplug entry's driver_data.
>> - */
>> -static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
>> - struct platform_device *pdev)
>> +static int ohci_da8xx_probe(struct platform_device *pdev)
>> {
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
>> struct usb_hcd *hcd;
>> @@ -295,6 +239,11 @@ static int usb_hcd_da8xx_probe(const struct hc_driver
>> *driver,
>> if (hub == NULL)
>> return -ENODEV;
>>
>> + hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
>> + dev_name(&pdev->dev));
>> + if (!hcd)
>> + return -ENOMEM;
>> +
>
>
> Won't this leak hdc if there is an error later?
>
>> usb11_clk = devm_clk_get(&pdev->dev, "usb11");
>> if (IS_ERR(usb11_clk)) {
>> if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
>> @@ -309,9 +258,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver
>> *driver,
>> return PTR_ERR(usb11_phy);
>> }
>>
>> - hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
>> - if (!hcd)
>> - return -ENOMEM;
>
>
> Why does this need to be moved?
>
it should not have moved this, will fix.
>>
>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
>> @@ -323,13 +269,12 @@ static int usb_hcd_da8xx_probe(const struct
>> hc_driver *driver,
>> hcd->rsrc_start = mem->start;
>> hcd->rsrc_len = resource_size(mem);
>>
>> - ohci_hcd_init(hcd_to_ohci(hcd));
>> -
>> irq = platform_get_irq(pdev, 0);
>> if (irq < 0) {
>> error = -ENODEV;
>> goto err;
>> }
>> +
>> error = usb_add_hcd(hcd, irq, 0);
>> if (error)
>> goto err;
>> @@ -348,35 +293,14 @@ static int usb_hcd_da8xx_probe(const struct
>> hc_driver *driver,
>> return error;
>> }
>>
>> -/**
>> - * usb_hcd_da8xx_remove - shutdown processing for DA8xx-based HCDs
>> - * @dev: USB Host Controller being removed
>> - * Context: !in_interrupt()
>> - *
>> - * Reverses the effect of usb_hcd_da8xx_probe(), first invoking
>> - * the HCD's stop() method. It is always called from a thread
>> - * context, normally "rmmod", "apmd", or something similar.
>> - */
>> -static inline void
>> -usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev)
>> +static int ohci_da8xx_remove(struct platform_device *pdev)
>> {
>> + struct usb_hcd *hcd = platform_get_drvdata(pdev);
>> struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
>>
>> hub->ocic_notify(NULL);
>> usb_remove_hcd(hcd);
>> usb_put_hcd(hcd);
>> -}
>> -
>> -static int ohci_hcd_da8xx_drv_probe(struct platform_device *dev)
>> -{
>> - return usb_hcd_da8xx_probe(&ohci_da8xx_hc_driver, dev);
>> -}
>> -
>> -static int ohci_hcd_da8xx_drv_remove(struct platform_device *dev)
>> -{
>> - struct usb_hcd *hcd = platform_get_drvdata(dev);
>> -
>> - usb_hcd_da8xx_remove(hcd, dev);
>>
>> return 0;
>> }
>> @@ -426,12 +350,16 @@ static int ohci_da8xx_resume(struct platform_device
>> *dev)
>> }
>> #endif
>>
>> +static const struct ohci_driver_overrides da8xx_overrides __initconst = {
>> + .reset = ohci_da8xx_reset
>> +};
>> +
>> /*
>> * Driver definition to register with platform structure.
>> */
>> static struct platform_driver ohci_hcd_da8xx_driver = {
>> - .probe = ohci_hcd_da8xx_drv_probe,
>> - .remove = ohci_hcd_da8xx_drv_remove,
>> + .probe = ohci_da8xx_probe,
>> + .remove = ohci_da8xx_remove,
>> .shutdown = usb_hcd_platform_shutdown,
>> #ifdef CONFIG_PM
>> .suspend = ohci_da8xx_suspend,
>
>
> It would probably be a good idea to change the driver name here. Currently
> it is "ohci". Although this would be better in a separate patch if the name
> has to be changed to match in other files as well.
>
ok, ill do a separate patch for that.
>> @@ -442,4 +370,39 @@ static int ohci_da8xx_resume(struct platform_device
>> *dev)
>> },
>> };
>>
>> +static int __init ohci_da8xx_init(void)
>> +{
>> +
>> + if (usb_disabled())
>> + return -ENODEV;
>> +
>> + pr_info("%s: " DRIVER_DESC "\n", hcd_name);
>> + ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides);
>> +
>> + /*
>> + * The Davinci da8xx HW has some unusual quirks, which require
>> + * da8xx-specific workarounds. We override certain hc_driver
>> + * functions here to achieve that. We explicitly do not enhance
>> + * ohci_driver_overrides to allow this more easily, since this
>> + * is an unusual case, and we don't want to encourage others to
>> + * override these functions by making it too easy.
>> + */
>> +
>> + orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control;
>> + orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data;
>> +
>> + ohci_da8xx_hc_driver.hub_status_data =
>> ohci_da8xx_hub_status_data;
>> + ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control;
>> +
>> + return platform_driver_register(&ohci_hcd_da8xx_driver);
>> +}
>> +module_init(ohci_da8xx_init);
>> +
>> +static void __exit ohci_da8xx_cleanup(void)
>
>
> ohci_da8xx_exit would be a better name
>
ok.
>> +{
>> + platform_driver_unregister(&ohci_hcd_da8xx_driver);
>> +}
>> +module_exit(ohci_da8xx_cleanup);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> +MODULE_LICENSE("GPL");
>> MODULE_ALIAS("platform:ohci");
>
>
> this will need to be changed too if you change the driver name
>
>> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
>> index 1700908..8de174a 100644
>> --- a/drivers/usb/host/ohci-hcd.c
>> +++ b/drivers/usb/host/ohci-hcd.c
>> @@ -1219,11 +1219,6 @@ void ohci_init_driver(struct hc_driver *drv,
>> #define SA1111_DRIVER ohci_hcd_sa1111_driver
>> #endif
>>
>> -#ifdef CONFIG_USB_OHCI_HCD_DAVINCI
>> -#include "ohci-da8xx.c"
>> -#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver
>> -#endif
>> -
>> #ifdef CONFIG_USB_OHCI_HCD_PPC_OF
>> #include "ohci-ppc-of.c"
>> #define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver
>> @@ -1303,19 +1298,9 @@ static int __init ohci_hcd_mod_init(void)
>> goto error_tmio;
>> #endif
>>
>> -#ifdef DAVINCI_PLATFORM_DRIVER
>> - retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER);
>> - if (retval < 0)
>> - goto error_davinci;
>> -#endif
>> -
>> return retval;
>>
>> /* Error path */
>> -#ifdef DAVINCI_PLATFORM_DRIVER
>> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
>> - error_davinci:
>> -#endif
>> #ifdef TMIO_OHCI_DRIVER
>> platform_driver_unregister(&TMIO_OHCI_DRIVER);
>> error_tmio:
>> @@ -1351,9 +1336,6 @@ static int __init ohci_hcd_mod_init(void)
>>
>> static void __exit ohci_hcd_mod_exit(void)
>> {
>> -#ifdef DAVINCI_PLATFORM_DRIVER
>> - platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
>> -#endif
>> #ifdef TMIO_OHCI_DRIVER
>> platform_driver_unregister(&TMIO_OHCI_DRIVER);
>> #endif
>>
>
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM: sti: stih410-clocks: Add PROC_STFE as a critical clock
From: Lee Jones @ 2016-10-25 7:42 UTC (permalink / raw)
To: Peter Griffin
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-F5mvAk5X5gdBDgjK7y7TUQ, patrice.chotard-qxv4g6HH51o,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161024134416.GB10440@griffinp-ThinkPad-X1-Carbon-2nd>
On Mon, 24 Oct 2016, Peter Griffin wrote:
> Hi Lee,
>
> On Mon, 24 Oct 2016, Lee Jones wrote:
> > On Tue, 18 Oct 2016, Peter Griffin wrote:
> >
> > > Once the ST frontend demux HW IP has been enabled, the clock can't
> > > be disabled otherwise the system will hang and the board will
> > > be unserviceable.
> > >
> > > To allow balanced clock enable/disable calls in the driver we use
> > > the critical clock infrastructure to take an extra reference on the
> > > clock so the clock will never actually be disabled.
> >
> > This is an abuse of the critical-clocks framework, and is exactly the
> > type of hack I promised the clk guys I'd try to prevent.
>
> I expect the best way to do this would be to write some documentation on the
> clock-critical DT binding and/or CRITICAL_CLK flag. The only documentation I can
> find currently is with the initial patch series [1] and the comment in
> clk-provider.h of
>
> #define CLK_IS_CRITICAL BIT(11) /* do not gate, ever */
>
> Or the patch decription
>
> "Critical clocks are those which must not be gated, else undefined
> or catastrophic failure would occur. Here we have chosen to
> ensure the prepare/enable counts are correctly incremented, so as
> not to confuse users with enabled clocks with no visible users."
>
> Which is the functionality I want for this clock.
No, that's not the functionality you want. You want for the clock not
to be RE-gated (big difference). Currently, the STFE clock will never
be gated, even when a) it's not used and b) can actually be disabled.
You're needlessly wasting power here.
Also, in your use-case there is a visible user, and the prepare/enable
counts will be correct.
> > If this, or
> > any other IP has some quirks (i.e. once enabled, if this clock is
> > subsequently disabled it will have a catastrophic effect on the
> > platform), then they should be worked around in the driver.
> >
> > The correct thing to do here is craft a clk-keep-on flag and ensure it
> > is set to true for the effected platform(s)' platform data.
>
> I'm always wary of creating a driver specific flag, especially when its
> purpose is to do the same thing as an existing mechanism provided by the
> subsystem of not gating the clock.
Using existing sub-system supplied mechanisms in the way they were not
intended is sub-optimal (read "hacky").
> I can see a couple of problems with what you propose:
>
> 1) You have to put the clk-keep-on flag in every driver which consumes the
> clock. IMO it is much better to have this knowledge in the SoC's
> clock driver so every consumer of the clock automatically benefits.
That would also be fine(ish). The issue is that this problem is
board specific, so the platform clock driver would have to contain
board level knowledge. Also, if you were to implement this, it would
too mess up reference counting in the core.
> 2) You don't benefit from the CLK_IS_CRITICAL reference counting logic in
> clk.c. So then each driver has to also work around that to get sensible reference
> counts. e.g.
>
> if (!__clk_is_enabled(clk) && pdata->clk-keep-on)
> clk_enable(clk)
>
> Which seems to me to be fighting against the subsystem. Given that the only use of
> _clk_is_enabled() outside drivers/clk is in an old arch/arm/mach-omap2/pm24xx.c
> driver I suspect its use is frowned upon, and it shouldn't really be an EXPORTED_SYMBOL.
In this instance, since the STFE clock is only used by this IP, I
would choose to handle it in the driver. This can be done using a
single flag stored in pdata which should be fetched using
of_match_device(). This way there is no need for any more API abuse;
either by incorrectly identifying the STFE clock as critical OR
invoking any internal __clk_*() calls.
Enable the clock once in .probe(), which you already do.
Then, whenever you do any power saving do:
suspend()
{
if (!ddata->enable_clk_once)
clk_disable(clk);
}
resume()
{
if (!ddata->enable_clk_once)
clk_enable(clk);
}
However, looking at your driver, I think this point might even be
moot, since you don't have any power saving. The only time you
disable the clock is in the error path. Just replace that with a
comment about the platform's unfortunate errata.
> [1] https://lkml.org/lkml/2016/1/18/272
I'm glad you mentioned this. Let's take a look:
> Some platforms contain clocks which if gated, will cause undefined or
> catastrophic behaviours. As such they are not to be turned off, ever.
Not the case here.
This clock *can* be gated and can be turned off *sometimes*.
> Many of these such clocks do not have devices, thus device drivers
> where clocks may be enabled and references taken to ensure they stay
> enabled do not exist. Therefore, we must handle these such cases in
> the core.
This clock *does* have a driver and correct references *can* be
taken.
[...]
All I'm saying is, and it's the same thing I've said many times; these
types of issues do not exhibit the same set of symptoms as a critical
clock by definition. Critical clocks are those which references can
not be taken by any other means. Think of the critical clock
framework as a mechanism to circumvent the requirement of writing a
special driver which would *only* handle clocks i.e. an interconnect
driver in the ST case.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [RFC V4 PATCH 5/6] arm64: dts: msm8994 SoC and Huawei Angler (Nexus 6P) support
From: Jeremy McNicoll @ 2016-10-25 8:00 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Jeremy McNicoll, linux-arm-msm, linux-soc, devicetree, robh,
andy.gross, sboyd, arnd, mark.rutland, michael.scott
In-Reply-To: <20161021182533.GX7509@tuxbot>
On Fri, Oct 21, 2016 at 11:25:33AM -0700, Bjorn Andersson wrote:
> On Fri 21 Oct 04:11 PDT 2016, Jeremy McNicoll wrote:
>
> > From: Bastian Köcher <mail@kchr.de>
> >
> > Initial device tree support for Qualcomm MSM8994 SoC and
> > Huawei Angler / Google Nexus 6P support.
> >
> > The device tree is based on the Google 3.10 kernel tree.
> >
> > The device can be booted into the initrd with only one CPU running.
> >
>
> Looks reasonable as well, same two comments as the 8992 below.
>
> > Signed-off-by: Bastian Köcher <mail@kchr.de>
> > [jeremymc@redhat.com: removed Kconfig, defconfig, move from Huawei to qcom dir]
> > Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> > ---
> > arch/arm64/boot/dts/qcom/Makefile | 2 +-
> > .../arm64/boot/dts/qcom/msm8994-angler-rev-101.dts | 42 ++++
> > arch/arm64/boot/dts/qcom/msm8994-pins.dtsi | 38 ++++
> > arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi | 32 +++
> > arch/arm64/boot/dts/qcom/msm8994.dtsi | 236 +++++++++++++++++++++
> > 5 files changed, 349 insertions(+), 1 deletion(-)
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8994-pins.dtsi
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8994-v2.0.dtsi
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8994.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index abb366e..4040fc8 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -1,6 +1,6 @@
> > dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb msm8916-mtp.dtb
> > dtb-$(CONFIG_ARCH_QCOM) += msm8996-mtp.dtb apq8096-db820c.dtb
> > -dtb-$(CONFIG_ARCH_QCOM) += msm8992-bullhead-rev-101.dtb
> > +dtb-$(CONFIG_ARCH_QCOM) += msm8992-bullhead-rev-101.dtb msm8994-angler-rev-101.dtb
>
> This won't scale, use multiple lines instead.
>
Done, one entry per line.
> >
> > always := $(dtb-y)
> > subdir-y := $(dts-dirs)
> > diff --git a/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > new file mode 100644
> > index 0000000..9684e6c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/msm8994-angler-rev-101.dts
> > @@ -0,0 +1,42 @@
> > +/* Copyright (c) 2015, Huawei Inc. All rights reserved.
> > + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +#include "msm8994-v2.0.dtsi"
> > +
> > +/ {
> > + model = "HUAWEI MSM8994 ANGLER rev-1.01";
> > + compatible = "qcom,msm8994";
> > + /* required for bootloader to select correct board */
> > + qcom,board-id = <8026 0>;
> > +};
> > +
> > +/ {
>
> No need to close and reopen / {} here.
>
updated.
> > + aliases {
> > + serial0 = &blsp1_uart2;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + soc {
> > + serial@f991e000 {
> > + status = "okay";
> > + pinctrl-names = "default", "sleep";
> > + pinctrl-0 = <&blsp1_uart2_default>;
> > + pinctrl-1 = <&blsp1_uart2_sleep>;
> > + };
> > + };
> > +};
> [..]
> > diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
> [..]
> > + msmgpio: pinctrl@fd510000 {
> > + compatible = "qcom,msm8994-pinctrl", "qcom,msm8974-pinctrl";
>
> msm8974 TLMM won't work here either, so please drop that.
>
Dropped 8974 pinctrl now that Michael Scott has created a proper pinctrl
driver for msm8994. There was sufficient overlap between 8974 and 8994
for initial dev work. [https://patches.linaro.org/patch/78750/]
-jeremy
> > + reg = <0xfd510000 0x4000>;
> > + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + };
>
> Regards,
> Bjorn
^ permalink raw reply
* Re: [PATCH/RFT v2 17/17] ARM: dts: da850: add usb device node
From: Axel Haslam @ 2016-10-25 8:03 UTC (permalink / raw)
To: David Lechner
Cc: Greg KH, Johan Hovold, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Sekhar Nori, Alan Stern, Kevin Hilman, Sergei Shtylyov,
Mark Brown, Alexandre Bailon, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <59ca6a1f-6db9-eed2-8e8a-77657c7febac-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
On Tue, Oct 25, 2016 at 2:48 AM, David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> wrote:
> On 10/24/2016 11:46 AM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote:
>>
>> From: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>
>> This adds the usb (ohci) device node for the da850 soc.
>> Also it enables it for the lcdk board
>>
>> Signed-off-by: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>> b/arch/arm/boot/dts/da850-lcdk.dts
>> index 7b8ab21..fa91339 100644
>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>> @@ -86,6 +86,14 @@
>> };
>> };
>>
>> +&usb_phy {
>> + status = "okay";
>> +};
>> +
>> +&usb {
>> + status = "okay";
>
>
> Don't you need to specify a regulator here using the vbus-supply property?
its is not mandatory, the regulator framework is giving a dummy supply
on regulator_get if it does not find a regulator on device tree. That is
what i could understand from: _regulator_get in regulator/core.c
Also, in the case of platform based boot, if the board init calls
regulator_has_full_constraints which is anyways needed for the phy to probe
correctly it will also return a dummy regulator.
>
>> +};
>> +
>> &serial2 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&serial2_rxtx_pins>;
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 33fcdce..ec2cec3 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -381,6 +381,14 @@
>> #phy-cells = <1>;
>> status = "disabled";
>> };
>> + usb: usb@0225000 {
>
>
> Don't need the leading 0 on usb@225000
>
> The alias (usb:) might need to be more specific since there is a second usb
> device that will be added later for musb. (The comments in the previous
> review only referred to the "usb" in "usb@", not the alias.)
ok.
>
>> + compatible = "ti,da830-ohci";
>> + reg = <0x225000 0x1000>;
>> + interrupts = <59>;
>> + phys = <&usb_phy 1>;
>> + phy-names = "usb-phy";
>> + status = "disabled";
>> + };
>> gpio: gpio@226000 {
>> compatible = "ti,dm6441-gpio";
>> gpio-controller;
>>
>
--
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: [RFC V4 PATCH 6/6] arm64: configs: enable configs for msm899(2/4) basic support
From: Jeremy McNicoll @ 2016-10-25 8:07 UTC (permalink / raw)
To: Bjorn Andersson, Jeremy McNicoll
Cc: linux-arm-msm, linux-soc, devicetree, robh, andy.gross, sboyd,
arnd, mark.rutland, michael.scott
In-Reply-To: <ba4e8128-40c1-e2ff-8eda-130cbb62e71e@redhat.com>
On Fri, Oct 21, 2016 at 03:32:08PM -0700, Jeremy McNicoll wrote:
> On 2016-10-21 11:27 AM, Bjorn Andersson wrote:
> >On Fri 21 Oct 04:11 PDT 2016, Jeremy McNicoll wrote:
> >
> >>From: Jeremy McNicoll <jmcnicol@redhat.com>
> >>
> >>Given the mimimal hardware support for msm899(2/4) currently.
> >>A few config options are needed to allow for continued
> >>development and booting.
> >>
> >>The following are needed for continued development and
> >>booting:
> >> -8x74 pinctrl for serial support
> >
> >I don't have the datasheet for 899[24], but I don't think this is a good
> >idea. We need a 899[24] pinctrl driver for this.
> >
>
> Based on the downstream code, for serial it was sufficient.
>
> There is a 8994 pinctrl driver on its away.
>
Here is a link to the 8994 pinctrl needed for 899(2/4).
https://patches.linaro.org/patch/78750/
-jeremy
^ permalink raw reply
* Re: [PATCH v2 1/8] drm/bridge: rgb-to-vga: Support an enable GPIO
From: Archit Taneja @ 2016-10-25 8:09 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard, David Airlie, Rob Herring,
Mark Rutland
Cc: linux-arm-kernel, devicetree, linux-sunxi, linux-kernel,
dri-devel
In-Reply-To: <20161020034344.14154-2-wens@csie.org>
Hi,
On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
> an enable pin on the bridge IC, or indirectly controlling a power
> switch.
>
> Add support for it.
Does the bridge on your platform have an active/passive DAC, or is it a
smarter encoder chip that is capable of doing more? If so, it might be
good to have a separate DT compatible string to it, like what's done
in the patch titled:
drm: bridge: vga-dac: Add adi,adv7123 compatible string
so that we can switch to a different driver later if needed.
Thanks,
Archit
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28 ++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..d3484822bf77 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> - Video port 0 for RGB input
> - Video port 1 for VGA output
>
> +Optional properties:
> +- enable-gpios: GPIO pin to enable or disable the bridge
>
> Example
> -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..b487e5e9b56d 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -10,6 +10,7 @@
> * the License, or (at your option) any later version.
> */
>
> +#include <linux/gpio/consumer.h>
> #include <linux/module.h>
> #include <linux/of_graph.h>
>
> @@ -23,6 +24,7 @@ struct dumb_vga {
> struct drm_connector connector;
>
> struct i2c_adapter *ddc;
> + struct gpio_desc *enable_gpio;
> };
>
> static inline struct dumb_vga *
> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
> return 0;
> }
>
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> + if (vga->enable_gpio)
> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
> +}
> +
> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> .attach = dumb_vga_attach,
> + .enable = dumb_vga_enable,
> + .disable = dumb_vga_disable,
> };
>
> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
> return -ENOMEM;
> platform_set_drvdata(pdev, vga);
>
> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(vga->enable_gpio)) {
> + ret = PTR_ERR(vga->enable_gpio);
> + dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
> + return ret;
> + }
> +
> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> if (IS_ERR(vga->ddc)) {
> if (PTR_ERR(vga->ddc) == -ENODEV) {
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH/RFT v2 16/17] USB: ohci-da8xx: Allow probing from DT
From: Axel Haslam @ 2016-10-25 8:10 UTC (permalink / raw)
To: David Lechner
Cc: Greg KH, Johan Hovold, robh+dt, Sekhar Nori, Alan Stern,
Kevin Hilman, Sergei Shtylyov, Mark Brown, Alexandre Bailon,
linux-usb, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <a32c58db-d3f8-2a1b-53b3-ae78e342c5e7@lechnology.com>
On Tue, Oct 25, 2016 at 2:53 AM, David Lechner <david@lechnology.com> wrote:
> On 10/24/2016 11:46 AM, ahaslam@baylibre.com wrote:
>>
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> This adds the compatible string to the ohci driver
>> to be able to probe from DT
>>
>> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
>> ---
>> drivers/usb/host/ohci-da8xx.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
>> index bebc3f0..1a8db25 100644
>> --- a/drivers/usb/host/ohci-da8xx.c
>> +++ b/drivers/usb/host/ohci-da8xx.c
>> @@ -273,6 +273,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd
>> *hcd, u16 typeReq, u16 wValue,
>> }
>>
>>
>> /*-------------------------------------------------------------------------*/
>> +#ifdef CONFIG_OF
>
>
> #ifdef CONFIG_OF is probably not needed here...
>
>> +static const struct of_device_id da8xx_ohci_ids[] = {
>> + { .compatible = "ti,da830-ohci" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
>> +#endif
>>
>> static int ohci_da8xx_probe(struct platform_device *pdev)
>> {
>> @@ -421,6 +428,7 @@ static int ohci_da8xx_resume(struct platform_device
>> *dev)
>> #endif
>> .driver = {
>> .name = "ohci",
>> + .of_match_table = da8xx_ohci_ids,
>
>
> ...otherwise, da8xx_ohci_ids will not be defined here
this should be .of_match_table = of_match_ptr(da8xx_ohci_ids),
will fix.
>
>> },
>> };
>>
>>
>
^ permalink raw reply
* Re: [PATCH/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: Sekhar Nori @ 2016-10-25 8:10 UTC (permalink / raw)
To: ahaslam, gregkh, johan, robh+dt, stern, khilman, sshtylyov, david,
manjunath.goudar, broonie, abailon
Cc: linux-usb, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20161024164634.4330-3-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam@baylibre.com wrote:
> From: David Lechner <david@lechnology.com>
>
> The CFGCHIP registers are used by a number of devices, so using a syscon
> device to share them. The first consumer of this will by the phy-da8xx-usb
> driver.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> [Axel: minor fix: change id to -1]
Can you please clarify this change? There could be other syscon devices
on the chip for other common registers. Why use the singular device-id?
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v2 8/8] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge
From: Archit Taneja @ 2016-10-25 8:13 UTC (permalink / raw)
To: Chen-Yu Tsai, Maxime Ripard, David Airlie, Rob Herring,
Mark Rutland
Cc: devicetree, linux-sunxi, linux-arm-kernel, dri-devel,
linux-kernel
In-Reply-To: <20161020034344.14154-9-wens@csie.org>
On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
> The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
> output from the LCD interface to VGA signals.
>
> Enable this part of the display pipeline.
I couldn't find the enable-gpios binding for the bridge that you
introduced in the previous patch. Is that intentional?
Thanks,
Archit
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56 +++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> index 9a74637f677f..05a49b2147f1 100644
> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
> @@ -63,6 +63,49 @@
> stdout-path = "serial0:115200n8";
> };
>
> + bridge {
> + compatible = "dumb-vga-dac";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&tcon0_out_vga>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_con_in>;
> + };
> + };
> + };
> + };
> +
> + vga {
> + compatible = "vga-connector";
> +
> + port {
> + vga_con_in: endpoint {
> + remote-endpoint = <&vga_bridge_out>;
> + };
> + };
> + };
> +
> wifi_pwrseq: wifi_pwrseq {
> compatible = "mmc-pwrseq-simple";
> reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
> @@ -245,6 +288,19 @@
> status = "okay";
> };
>
> +&tcon0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd0_rgb888_pins>;
> + status = "okay";
> +};
> +
> +&tcon0_out {
> + tcon0_out_vga: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_bridge_in>;
> + };
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_pins_a>;
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [RFC V4 PATCH 1/6] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
From: Jeremy McNicoll @ 2016-10-25 8:15 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Jeremy McNicoll, linux-arm-msm, linux-soc, devicetree, robh,
andy.gross, sboyd, arnd, mark.rutland, michael.scott
In-Reply-To: <20161021182004.GW7509@tuxbot>
On Fri, Oct 21, 2016 at 11:20:04AM -0700, Bjorn Andersson wrote:
> On Fri 21 Oct 04:11 PDT 2016, Jeremy McNicoll wrote:
>
> > From: Jeremy McNicoll <jmcnicol@redhat.com>
> >
> > Initial device tree support for Qualcomm MSM8992 SoC and
> > LG Bullhead / Google Nexus 5X support.
> >
>
> Looks reasonable, just two small nits below.
>
Thanks for the feedback.
> > Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> > ---
> > arch/arm64/boot/dts/qcom/Makefile | 4 +-
> > .../boot/dts/qcom/msm8992-bullhead-rev-101.dts | 42 ++++
> > arch/arm64/boot/dts/qcom/msm8992-pins.dtsi | 38 ++++
> > arch/arm64/boot/dts/qcom/msm8992.dtsi | 214 +++++++++++++++++++++
> > 4 files changed, 296 insertions(+), 2 deletions(-)
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8992-pins.dtsi
> > create mode 100644 arch/arm64/boot/dts/qcom/msm8992.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 5dd05de..abb366e 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -1,6 +1,6 @@
> > dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb msm8916-mtp.dtb
> > -dtb-$(CONFIG_ARCH_QCOM) += msm8996-mtp.dtb
> > -dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb
> > +dtb-$(CONFIG_ARCH_QCOM) += msm8996-mtp.dtb apq8096-db820c.dtb
>
> Don't change these, just add yours.
>
Done, one line per entry.
> [..]
> > diff --git a/arch/arm64/boot/dts/qcom/msm8992.dtsi b/arch/arm64/boot/dts/qcom/msm8992.dtsi
> [..]
> > +
> > + msmgpio: pinctrl@fd510000 {
> > + compatible = "qcom,msm8994-pinctrl", "qcom,msm8974-pinctrl";
>
> This device is not compatible with the msm8974 TLMM, so please drop the
> second compatible.
Updated to use, https://patches.linaro.org/patch/78750/
-jeremy
^ permalink raw reply
* Re: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent
From: Axel Haslam @ 2016-10-25 8:24 UTC (permalink / raw)
To: David Lechner
Cc: Greg KH, Johan Hovold, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
Sekhar Nori, Alan Stern, Kevin Hilman, Sergei Shtylyov,
Mark Brown, Alexandre Bailon, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <672c1f1a-58b7-6947-7fb2-acd38fab8597-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
On Tue, Oct 25, 2016 at 3:39 AM, David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> wrote:
> On 10/24/2016 11:46 AM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote:
>>
>> From: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>
>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in
>> board files to handle vbus and overcurrent irqs form the power supply.
>> However, this does not play nice when moving to a DT based boot were
>> we wont have board files.
>>
>> Instead of requesting and handling the gpio, use the regulator framework
>> to take care of enabling and disabling vbus power.
>> This has the benefit
>> that we dont need to pass any more platform data to the driver:
>>
>> These will be handled by the regulator framework:
>> set_power -> regulator_enable/regulator_disable
>> get_power -> regulator_is_enabled
>> get_oci -> regulator_get_mode
>> ocic_notify -> regulator notification
>>
>> We can keep the default potpgt and use the regulator start delay instead:
>> potpgt -> regulator startup delay time
>>
>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus,
>> (they should not have been decleared/reserved) so, just remove those
>> definitions from the hwk board file.
>>
>> Signed-off-by: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>
>
>
> How do you recover after an overcurrent event?
>
> I have configured a fixed-regulator using device-tree, but similar to the
> configuration in the board files here. However, when I shorted out the VBUS
> and caused an overcurrent event, I see nothing in the kernel log saying that
> there was an overcurrent event and after I remove the short, the regulator
> is never turned back on.
>
>
You should have the patch to fix gpiolib, and you should declare the
over current gpio on the regulator as such:
(if the pin is enabled high you should add oc-active-high);
vbus_fixed: fixed-regulator-vbus {
compatible = "regulator-fixed";
gpio = <&gpio 109 0>;
oc-gpio = <&gpio 36 0>;
regulator-boot-on;
enable-active-high;
regulator-name = "vbus";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
};
Question: Do you see that the over current gpio was requested
in debugfs/gpio? and, do you see the interrupt in /proc/interrupts?
If you unplug and plug in back the usb device it should work again.
also you can unbind and bind it should also start to work:
something like:
echo usb1 >/sys/bus/usb/drivers/usb/unbind
echo usb1 >/sys/bus/usb/drivers/usb/bind
>
>> @@ -163,7 +198,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd,
>> u16 typeReq, u16 wValue,
>> u16 wIndex, char *buf, u16 wLength)
>> {
>> struct device *dev = hcd->self.controller;
>> - struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>
>
> nit: unnecessary whitespace change
>
>> int temp;
>>
>> switch (typeReq) {
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 8/8] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge
From: Chen-Yu Tsai @ 2016-10-25 8:50 UTC (permalink / raw)
To: Archit Taneja
Cc: Chen-Yu Tsai, Maxime Ripard, David Airlie, Rob Herring,
Mark Rutland, dri-devel, linux-arm-kernel, linux-kernel,
devicetree, linux-sunxi
In-Reply-To: <bac9a59f-df97-245b-f349-bed57dec6aad-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Tue, Oct 25, 2016 at 4:13 PM, Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
>
> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>
>> The Hummingbird A31 board has a RGB-to-VGA bridge which converts RGB
>> output from the LCD interface to VGA signals.
>>
>> Enable this part of the display pipeline.
>
>
> I couldn't find the enable-gpios binding for the bridge that you
> introduced in the previous patch. Is that intentional?
Error on my part. Thanks for spotting that.
ChenYu
>
> Thanks,
> Archit
>
>
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 56
>> +++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> index 9a74637f677f..05a49b2147f1 100644
>> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>> @@ -63,6 +63,49 @@
>> stdout-path = "serial0:115200n8";
>> };
>>
>> + bridge {
>> + compatible = "dumb-vga-dac";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + vga_bridge_in: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint =
>> <&tcon0_out_vga>;
>> + };
>> + };
>> +
>> + port@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + vga_bridge_out: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_con_in>;
>> + };
>> + };
>> + };
>> + };
>> +
>> + vga {
>> + compatible = "vga-connector";
>> +
>> + port {
>> + vga_con_in: endpoint {
>> + remote-endpoint = <&vga_bridge_out>;
>> + };
>> + };
>> + };
>> +
>> wifi_pwrseq: wifi_pwrseq {
>> compatible = "mmc-pwrseq-simple";
>> reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
>> @@ -245,6 +288,19 @@
>> status = "okay";
>> };
>>
>> +&tcon0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&lcd0_rgb888_pins>;
>> + status = "okay";
>> +};
>> +
>> +&tcon0_out {
>> + tcon0_out_vga: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_bridge_in>;
>> + };
>> +};
>> +
>> &uart0 {
>> pinctrl-names = "default";
>> pinctrl-0 = <&uart0_pins_a>;
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/8] drm/bridge: rgb-to-vga: Support an enable GPIO
From: Chen-Yu Tsai @ 2016-10-25 8:59 UTC (permalink / raw)
To: Archit Taneja
Cc: Chen-Yu Tsai, Maxime Ripard, David Airlie, Rob Herring,
Mark Rutland, devicetree, linux-sunxi, dri-devel, linux-kernel,
linux-arm-kernel
In-Reply-To: <0b5fbe8e-e51b-c874-e1a3-0b88dc65e361-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Tue, Oct 25, 2016 at 4:09 PM, Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Hi,
>
> On 10/20/2016 09:13 AM, Chen-Yu Tsai wrote:
>>
>> Some rgb-to-vga bridges have an enable GPIO, either directly tied to
>> an enable pin on the bridge IC, or indirectly controlling a power
>> switch.
>>
>> Add support for it.
>
>
> Does the bridge on your platform have an active/passive DAC, or is it a
> smarter encoder chip that is capable of doing more? If so, it might be
> good to have a separate DT compatible string to it, like what's done
> in the patch titled:
>
> drm: bridge: vga-dac: Add adi,adv7123 compatible string
>
> so that we can switch to a different driver later if needed.
The chip is GM7123. It is not configurable. It just takes the LCD RGB/SYNC
signals and converts them to analog. The only things you can change are
putting it into sleep mode and tweaking the output drive strength by
changing the external reference resistor. The latter would be a hardware
design decision. I would say this qualifies as "dumb".
I revisited the board schematics, and the enable GPIO actually toggles
an external LDO regulator. So this might be better modeled as a regulator
supply?
Thanks
ChenYu
>
> Thanks,
> Archit
>
>
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>> .../bindings/display/bridge/dumb-vga-dac.txt | 2 ++
>> drivers/gpu/drm/bridge/dumb-vga-dac.c | 28
>> ++++++++++++++++++++++
>> 2 files changed, 30 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..d3484822bf77 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in
>> Documentation/devicetree/bindings/graph.txt.
>> - Video port 0 for RGB input
>> - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- enable-gpios: GPIO pin to enable or disable the bridge
>>
>> Example
>> -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..b487e5e9b56d 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -10,6 +10,7 @@
>> * the License, or (at your option) any later version.
>> */
>>
>> +#include <linux/gpio/consumer.h>
>> #include <linux/module.h>
>> #include <linux/of_graph.h>
>>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>> struct drm_connector connector;
>>
>> struct i2c_adapter *ddc;
>> + struct gpio_desc *enable_gpio;
>> };
>>
>> static inline struct dumb_vga *
>> @@ -124,8 +126,26 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>> return 0;
>> }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> + if (vga->enable_gpio)
>> + gpiod_set_value_cansleep(vga->enable_gpio, 1);
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> + struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> + if (vga->enable_gpio)
>> + gpiod_set_value_cansleep(vga->enable_gpio, 0);
>> +}
>> +
>> static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>> .attach = dumb_vga_attach,
>> + .enable = dumb_vga_enable,
>> + .disable = dumb_vga_disable,
>> };
>>
>> static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +189,14 @@ static int dumb_vga_probe(struct platform_device
>> *pdev)
>> return -ENOMEM;
>> platform_set_drvdata(pdev, vga);
>>
>> + vga->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(vga->enable_gpio)) {
>> + ret = PTR_ERR(vga->enable_gpio);
>> + dev_err(&pdev->dev, "failed to request GPIO: %d\n", ret);
>> + return ret;
>> + }
>> +
>> vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>> if (IS_ERR(vga->ddc)) {
>> if (PTR_ERR(vga->ddc) == -ENODEV) {
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: rk3x: Give the tuning value 0 during rk3x_i2c_v0_calc_timings
From: Wolfram Sang @ 2016-10-25 9:12 UTC (permalink / raw)
To: David Wu
Cc: heiko, dianders, linux-arm-kernel, linux-rockchip, linux-i2c,
devicetree, linux-kernel
In-Reply-To: <1477125822-30644-1-git-send-email-david.wu@rock-chips.com>
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On Sat, Oct 22, 2016 at 04:43:42PM +0800, David Wu wrote:
> We found a bug that i2c transfer sometimes failed on 3066a board with
> stabel-4.8, the con register would be updated by uninitialized tuning
> value, it made the i2c transfer failed.
>
> So give the tuning value to be zero during rk3x_i2c_v0_calc_timings.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
Added stable and applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/9] pinctrl: meson: allow gpio to request irq
From: Linus Walleij @ 2016-10-25 9:14 UTC (permalink / raw)
To: Jerome Brunet
Cc: Carlo Caione, Kevin Hilman, open list:ARM/Amlogic Meson...,
linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring,
Catalin Marinas, Will Deacon, Russell King
In-Reply-To: <1477040798.15560.96.camel@baylibre.com>
On Fri, Oct 21, 2016 at 11:06 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>> Isn't this usecase (also as described in the cover letter) a textbook
>> example of when you should be using hierarchical irqdomain?
>>
>> Please check with Marc et al on hierarchical irqdomains.
>
> Linus,
> Do you mean I should create a new hierarchical irqdomains in each of
> the two pinctrl instances we have in these SoC, these domains being
> stacked on the one I just added for controller in irqchip ?
>
> I did not understand this is what you meant when I asked you the
> question at ELCE.
Honestly, I do not understand when and where to properly use
hierarchical irqdomain, even after Marc's talk at ELC-E.
Which is problematic since quite a few GPIO drivers now
need to use them.
I will review his slides, in the meantime I would say: whatever
Marc ACKs is fine with me. I trust this guy 100%. So I guess I
could ask that he ACK the entire chain of patches
from GIC->specialchip->GPIO.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH/RFT v2 03/17] ARM: davinci: da8xx: Add USB PHY platform declaration
From: Sekhar Nori @ 2016-10-25 9:18 UTC (permalink / raw)
To: ahaslam, gregkh, johan, robh+dt, stern, khilman, sshtylyov, david,
broonie, abailon
Cc: linux-usb, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20161024164634.4330-4-ahaslam@baylibre.com>
On Monday 24 October 2016 10:16 PM, ahaslam@baylibre.com wrote:
> +static struct platform_device da8xx_usb_phy = {
> + .name = "da8xx-usb-phy",
> + .id = 0,
There is a single phy control in the system for both 1.1 and 2.0 PHYs.
so this can be a singular device (id -1).
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
From: Axel Haslam @ 2016-10-25 9:37 UTC (permalink / raw)
To: Sekhar Nori
Cc: Greg KH, Johan Hovold, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Alan Stern,
Kevin Hilman, Sergei Shtylyov, David Lechner,
manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A, Mark Brown,
Alexandre Bailon, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <7f9efe9d-0912-e10a-3e45-24c5d2b455ab-l0cyMroinI0@public.gmane.org>
Hi Sekar,
On Tue, Oct 25, 2016 at 10:10 AM, Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> On Monday 24 October 2016 10:16 PM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote:
>> From: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>>
>> The CFGCHIP registers are used by a number of devices, so using a syscon
>> device to share them. The first consumer of this will by the phy-da8xx-usb
>> driver.
>>
>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>> [Axel: minor fix: change id to -1]
>
> Can you please clarify this change? There could be other syscon devices
> on the chip for other common registers. Why use the singular device-id?
>
in the case of non DT boot, the phy driver is looking for "syscon" :
d_phy->regmap = syscon_regmap_lookup_by_pdevname("syscon");
if we register the syscon driver with id = 0, the actual name of the syscon
device will be "syscon.0" and the phy driver will fail to probe, because
the strncmp match in the syscon driver (syscon_match_pdevname)
will fail.
should i change the phy driver instead?
Regards,
Axel.
>> Signed-off-by: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>
> Thanks,
> Sekhar
>
--
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/RFT v2 03/17] ARM: davinci: da8xx: Add USB PHY platform declaration
From: Axel Haslam @ 2016-10-25 9:37 UTC (permalink / raw)
To: Sekhar Nori
Cc: Greg KH, Johan Hovold, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Alan Stern,
Kevin Hilman, Sergei Shtylyov, David Lechner, Mark Brown,
Alexandre Bailon, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <4129f1be-42f6-f182-c8a2-84befe01d298-l0cyMroinI0@public.gmane.org>
On Tue, Oct 25, 2016 at 11:18 AM, Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> wrote:
> On Monday 24 October 2016 10:16 PM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote:
>> +static struct platform_device da8xx_usb_phy = {
>> + .name = "da8xx-usb-phy",
>> + .id = 0,
>
> There is a single phy control in the system for both 1.1 and 2.0 PHYs.
> so this can be a singular device (id -1).
>
Ok.
> Thanks,
> Sekhar
>
--
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