* Re: [PATCH v2 03/17] input: convert LoCoMo keyboard driver to use new locomo core
From: Dmitry Torokhov @ 2015-05-12 21:13 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Bryan Wu, Richard Purdie,
Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami, linux-arm-kernel, linux-gpio@vger.kernel.org,
linux-i2c, linux-input, linux-leds, linux-spi
In-Reply-To: <CALT56yOjiBWHH3Uuo82Hqh8Va8V_KCUaaLL7RFtPUjWmfOFbGA@mail.gmail.com>
On Wed, May 13, 2015 at 12:01:23AM +0300, Dmitry Eremin-Solenikov wrote:
> Hello,
>
> 2015-05-12 23:21 GMT+03:00 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > Hi Dmitry,
> >
> > On Tue, Apr 28, 2015 at 02:55:40AM +0300, Dmitry Eremin-Solenikov wrote:
> >> As LoCoMo is switching to new device model, adapt keyboard driver to
> >> support new locomo core driver.
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >> ---
>
> Thanks for the review.
>
> >> /* helper functions for reading the keyboard matrix */
> >> -static inline void locomokbd_charge_all(unsigned long membase)
> >> +static inline void locomokbd_charge_all(struct locomokbd *locomokbd)
> >> {
> >> - locomo_writel(0x00FF, membase + LOCOMO_KSC);
> >> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0x00ff);
> >> }
> >>
> >> -static inline void locomokbd_activate_all(unsigned long membase)
> >> +static inline void locomokbd_activate_all(struct locomokbd *locomokbd)
> >
> > Drop "inline"s from the .c file please.
>
> Why?
Because compiler usually knows better whether a function should be
inlined or not as it keeps track of available registers, so leave the
decision to it.
>
> >
> >> {
> >> - unsigned long r;
> >> -
> >> - locomo_writel(0, membase + LOCOMO_KSC);
> >> - r = locomo_readl(membase + LOCOMO_KIC);
> >> - r &= 0xFEFF;
> >> - locomo_writel(r, membase + LOCOMO_KIC);
> >> + regmap_write(locomokbd->regmap, LOCOMO_KSC, 0);
> >> + regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x100, 0);
> >> }
> >>
>
> [skipped]
>
> >> @@ -291,16 +275,30 @@ static int locomokbd_probe(struct locomo_dev *dev)
> >>
> >> input_set_drvdata(input_dev, locomokbd);
> >>
> >> - memcpy(locomokbd->keycode, locomokbd_keycode, sizeof(locomokbd->keycode));
> >> + memcpy(locomokbd->keycode,
> >> + locomokbd_keycode,
> >> + sizeof(locomokbd->keycode));
> >> +
> >> + if (machine_is_collie())
> >> + locomokbd->keycode[18] = KEY_HOME;
> >> + else
> >> + locomokbd->keycode[3] = KEY_HOME;
> >
> > This seems like a new addition. Ideally keymap twiddling shoudl be done
> > from userspace.
>
> This fixes a hardware issue. Home key is wired differently on two
> devices using this driver.
> I'd prefer to have such setting in board file or in DTS in future,
> however that looks like an
> overkill. What would be your suggestion?
I am OK with doing this in driver, just as a separate patch please.
>
> >> /* attempt to get the interrupt */
> >> - err = request_irq(dev->irq[0], locomokbd_interrupt, 0, "locomokbd", locomokbd);
> >> + err = request_irq(locomokbd->irq, locomokbd_interrupt, 0,
> >> + "locomokbd", locomokbd);
> >
> > devm_request_irq()?
> >
> [skipped]
>
> >> -static int locomokbd_remove(struct locomo_dev *dev)
> >> +static int locomokbd_remove(struct platform_device *dev)
> >> {
> >> - struct locomokbd *locomokbd = locomo_get_drvdata(dev);
> >> + struct locomokbd *locomokbd = platform_get_drvdata(dev);
> >>
> >> - free_irq(dev->irq[0], locomokbd);
> >> + free_irq(locomokbd->irq, locomokbd);
> >
> > Is not needed with devm.
>
> Not quite. There will be a possibility for the IRQ to happen after deleting
> a timer in locomokbd_remove() and before freeing the IRQ through the devres
> core. Oops.
Right, but if you make sure that device does not generate interrupts in
probe() until open() is called and do the same in close(), then it
should be OK.
>
> >
> >>
> >> del_timer_sync(&locomokbd->timer);
> >
> > Should likely to go into close().
>
> Hmm. I will rethink this part, thank you.
>
> >> +
> >> +#ifdef CONFIG_PM_SLEEP
> >> +static int locomokbd_suspend(struct device *dev)
> >
> > Mark as __maybe_unused instead of giarding with CONFIG_PM_SLEEP.
>
> Fine, however I thought that #ifdef's here are a typical pattern.
It is up to subsystems, __maybe_unused provides better compile coverage.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/3] Phy: DT binding documentation for Broadcom Cygnus USB PHY driver
From: Arun Ramamurthy @ 2015-05-12 22:05 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Gregory CLEMENT,
Gabriel FERNANDEZ, Jason Cooper, Thomas Petazzoni,
Greg Kroah-Hartman, Arnd Bergmann
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w
In-Reply-To: <5550B878.7060608-l0cyMroinI0@public.gmane.org>
Hi
On 15-05-11 07:11 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 23 April 2015 04:44 AM, Arun Ramamurthy wrote:
>> Broadcom's Cygnus chip has a USB 2.0 host controller connected to
>> three separate phys. One of the phs (port 2) is also connectd to
>> a usb 2.0 device controller
>>
>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>> Reviewed-by: Ray Jui <rjui@broadcom.com>
>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>> ---
>> .../bindings/phy/brcm,cygnus-usb-phy.txt | 69
>> ++++++++++++++++++++++
>> include/dt-bindings/phy/phy.h | 2 +
>> 2 files changed, 71 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>
>> diff --git
>> a/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>> b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>> new file mode 100644
>> index 0000000..ec62044
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>> @@ -0,0 +1,69 @@
>> +BROADCOM CYGNUS USB PHY
>> +
>> +Required Properties:
>> + - compatible: brcm,cygnus-usb-phy
>> + - reg : usbphy_regs - Base address of phy registers
>> + usb2h_idm_regs - Base address of host idm registers
>> + usb2d_idm_regs - Base address of device idm registers
>> + - phy-cells - must be 1 for each port declared. The node
>> + that uses the phy must provide either PHY_CONFIG_DEVICE
>> for device
>> + or PHY_CONFIG_HOST for host
>> +
>> +NOTE: port 0 and port 1 are host only and port 2 can be configured
>> for host or
>> +device.
>> +
>> +Example of phy :
>> + usbphy0: usbphy@0x0301c000 {
>> + compatible = "brcm,cygnus-usb-phy";
>> + reg = <0x0301c000 0x2000>,
>> + <0x18115000 0x1000>,
>> + <0x18111000 0x1000>;
>> + status = "okay";
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + usbphy0_0: usbphy0@0 {
>> + #phy-cells = <1>;
>> + reg = <0>;
>> + status = "okay";
>> + phy-supply = <&vbus_p0>;
>> + };
>> +
>> + usbphy0_1: usbphy0@1 {
>> + #phy-cells = <1>;
>> + reg = <1>;
>> + status = "okay";
>> + };
>> +
>> + usbphy0_2: usbphy0@2 {
>> + #phy-cells = <1>;
>> + reg = <2>;
>> + status = "okay";
>> + phy-supply = <&vbus_p2>;
>> + };
>> + };
>> +
>> +Example of node using the phy:
>> +
>> + /* This nodes declares all three ports as host */
>> +
>> + ehci0: usb@0x18048000 {
>> + compatible = "generic-ehci";
>> + reg = <0x18048000 0x100>;
>> + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
>> + phys = <&usbphy0_0 PHY_CONFIG_HOST &usbphy0_1 PHY_CONFIG_HOST
>> &usbphy0_2 PHY_CONFIG_HOST>;
>> + status = "okay";
>> + };
>> +
>> + /*
>> + * This node declares port 2 phy
>> + * and configures it for device
>> + */
>> +
>> + usbd_udc_dwc1: usbd_udc_dwc@0x1804c000 {
>> + compatible = "iproc-udc";
>> + reg = <0x1804c000 0x2000>;
>> + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
>> + phys = <&usbphy0_2 PHY_CONFIG_DEVICE>;
>> + phy-names = "usb";
>> + };
>> diff --git a/include/dt-bindings/phy/phy.h
>> b/include/dt-bindings/phy/phy.h
>> index 6c90193..3f6b1ac 100644
>> --- a/include/dt-bindings/phy/phy.h
>> +++ b/include/dt-bindings/phy/phy.h
>> @@ -15,5 +15,7 @@
>> #define PHY_TYPE_PCIE 2
>> #define PHY_TYPE_USB2 3
>> #define PHY_TYPE_USB3 4
>> +#define PHY_CONFIG_HOST 1
>> +#define PHY_CONFIG_DEVICE 0
>
> '0' and '1' are already defined for "PHY_NONE" and "PHY_TYPE_SATA". Is
> this for USB2 or for USB3?
>
This is for USB2, do you want me to prefix the defines with USB2?
I think It would be misleading to use PHY_TYPE_SATA or PHY_NONE in this
scenario
> Thanks
> Kishon
^ permalink raw reply
* Re: [PATCH v3 2/3] Phy: DT binding documentation for Broadcom Cygnus USB PHY driver
From: Kishon Vijay Abraham I @ 2015-05-13 6:14 UTC (permalink / raw)
To: Arun Ramamurthy, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Russell King, Gregory CLEMENT,
Gabriel FERNANDEZ, Jason Cooper, Thomas Petazzoni,
Greg Kroah-Hartman, Arnd Bergmann
Cc: devicetree, linux-kernel, linux-fbdev, Dmitry Torokhov,
Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
bcm-kernel-feedback-list
In-Reply-To: <55527935.5060907@broadcom.com>
Hi,
On Wednesday 13 May 2015 03:35 AM, Arun Ramamurthy wrote:
> Hi
>
> On 15-05-11 07:11 AM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 23 April 2015 04:44 AM, Arun Ramamurthy wrote:
>>> Broadcom's Cygnus chip has a USB 2.0 host controller connected to
>>> three separate phys. One of the phs (port 2) is also connectd to
>>> a usb 2.0 device controller
>>>
>>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>>> Reviewed-by: Ray Jui <rjui@broadcom.com>
>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>> ---
>>> .../bindings/phy/brcm,cygnus-usb-phy.txt | 69
>>> ++++++++++++++++++++++
>>> include/dt-bindings/phy/phy.h | 2 +
>>> 2 files changed, 71 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>> b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>> new file mode 100644
>>> index 0000000..ec62044
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
>>> @@ -0,0 +1,69 @@
>>> +BROADCOM CYGNUS USB PHY
>>> +
>>> +Required Properties:
>>> + - compatible: brcm,cygnus-usb-phy
>>> + - reg : usbphy_regs - Base address of phy registers
>>> + usb2h_idm_regs - Base address of host idm registers
>>> + usb2d_idm_regs - Base address of device idm registers
>>> + - phy-cells - must be 1 for each port declared. The node
>>> + that uses the phy must provide either PHY_CONFIG_DEVICE
>>> for device
>>> + or PHY_CONFIG_HOST for host
>>> +
>>> +NOTE: port 0 and port 1 are host only and port 2 can be configured
>>> for host or
>>> +device.
>>> +
>>> +Example of phy :
>>> + usbphy0: usbphy@0x0301c000 {
>>> + compatible = "brcm,cygnus-usb-phy";
>>> + reg = <0x0301c000 0x2000>,
>>> + <0x18115000 0x1000>,
>>> + <0x18111000 0x1000>;
>>> + status = "okay";
>>> +
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + usbphy0_0: usbphy0@0 {
>>> + #phy-cells = <1>;
>>> + reg = <0>;
>>> + status = "okay";
>>> + phy-supply = <&vbus_p0>;
>>> + };
>>> +
>>> + usbphy0_1: usbphy0@1 {
>>> + #phy-cells = <1>;
>>> + reg = <1>;
>>> + status = "okay";
>>> + };
>>> +
>>> + usbphy0_2: usbphy0@2 {
>>> + #phy-cells = <1>;
>>> + reg = <2>;
>>> + status = "okay";
>>> + phy-supply = <&vbus_p2>;
>>> + };
>>> + };
>>> +
>>> +Example of node using the phy:
>>> +
>>> + /* This nodes declares all three ports as host */
>>> +
>>> + ehci0: usb@0x18048000 {
>>> + compatible = "generic-ehci";
>>> + reg = <0x18048000 0x100>;
>>> + interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
>>> + phys = <&usbphy0_0 PHY_CONFIG_HOST &usbphy0_1 PHY_CONFIG_HOST
>>> &usbphy0_2 PHY_CONFIG_HOST>;
>>> + status = "okay";
>>> + };
>>> +
>>> + /*
>>> + * This node declares port 2 phy
>>> + * and configures it for device
>>> + */
>>> +
>>> + usbd_udc_dwc1: usbd_udc_dwc@0x1804c000 {
>>> + compatible = "iproc-udc";
>>> + reg = <0x1804c000 0x2000>;
>>> + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
>>> + phys = <&usbphy0_2 PHY_CONFIG_DEVICE>;
>>> + phy-names = "usb";
>>> + };
>>> diff --git a/include/dt-bindings/phy/phy.h
>>> b/include/dt-bindings/phy/phy.h
>>> index 6c90193..3f6b1ac 100644
>>> --- a/include/dt-bindings/phy/phy.h
>>> +++ b/include/dt-bindings/phy/phy.h
>>> @@ -15,5 +15,7 @@
>>> #define PHY_TYPE_PCIE 2
>>> #define PHY_TYPE_USB2 3
>>> #define PHY_TYPE_USB3 4
>>> +#define PHY_CONFIG_HOST 1
>>> +#define PHY_CONFIG_DEVICE 0
>>
>> '0' and '1' are already defined for "PHY_NONE" and "PHY_TYPE_SATA". Is
>> this for USB2 or for USB3?
>>
> This is for USB2, do you want me to prefix the defines with USB2?
yes. That will avoid confusion when someone tries to add USB3 HOST PHY or USB3
DEVICE PHY.
> I think It would be misleading to use PHY_TYPE_SATA or PHY_NONE in this
> scenario
I meant to use the next available values like 5 and 6.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH v2 01/17] mfd: add new driver for Sharp LoCoMo
From: Lee Jones @ 2015-05-13 9:41 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami, linux-arm-kernel, linux-gpio@vger.kernel.org,
linux-i2c, linux-input, linux-leds, linux-spi
In-Reply-To: <CALT56yP6FPhGRZFJZkyjjpqGJ6bofg+a1BTC9KcVAgpmt2htoA@mail.gmail.com>
On Tue, 12 May 2015, Dmitry Eremin-Solenikov wrote:
> 2015-04-28 21:45 GMT+03:00 Lee Jones <lee.jones@linaro.org>:
> > On Tue, 28 Apr 2015, Dmitry Eremin-Solenikov wrote:
> >
> >> LoCoMo is a GA used on Sharp Zaurus SL-5x00. Current driver does has
> >> several design issues (special bus instead of platform bus, doesn't use
> >> mfd-core, etc).
> >>
> >> Implement 'core' parts of locomo support as an mfd driver.
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >> ---
>
> Thanks for the review. I agree (and have implemented) with most of
> your comments.
> However I have few questions. See below.
>
> >
> >> +/* the following is the overall data for the locomo chip */
> >> +struct locomo {
> >> + struct device *dev;
> >> + unsigned int irq;
> >> + spinlock_t lock;
> >> + struct irq_domain *domain;
> >> + struct regmap *regmap;
> >> +};
> >> +
> >> +static struct resource locomo_kbd_resources[] = {
> >> + DEFINE_RES_IRQ(IRQ_LOCOMO_KEY),
> >> +};
> >> +
> >> +static struct resource locomo_gpio_resources[] = {
> >> + DEFINE_RES_IRQ(IRQ_LOCOMO_GPIO),
> >> +};
> >> +
> >> +/* Filled in locomo_probe() function. */
> >> +static struct locomo_gpio_platform_data locomo_gpio_pdata;
> >
> > I'd prefer you didn't use globals for this.
>
> Just for platform data, or for all the structures?
Just for this. The remainder are standard.
> >> +static struct resource locomo_lt_resources[] = {
> >> + DEFINE_RES_IRQ(IRQ_LOCOMO_LT),
> >> +};
> >> +
> >> +static struct resource locomo_spi_resources[] = {
> >> + DEFINE_RES_IRQ(IRQ_LOCOMO_SPI),
> >> +};
> >> +
> >> +/* Filled in locomo_probe() function. */
> >> +static struct locomo_lcd_platform_data locomo_lcd_pdata;
> >> +
> >> +static struct mfd_cell locomo_cells[] = {
> >> + {
> >> + .name = "locomo-kbd",
> >> + .resources = locomo_kbd_resources,
> >> + .num_resources = ARRAY_SIZE(locomo_kbd_resources),
> >> + },
> >> + {
> >> + .name = "locomo-gpio",
> >> + .resources = locomo_gpio_resources,
> >> + .num_resources = ARRAY_SIZE(locomo_gpio_resources),
> >> + .platform_data = &locomo_gpio_pdata,
> >> + .pdata_size = sizeof(locomo_gpio_pdata),
> >> + },
> >> + {
> >> + .name = "locomo-lt", /* Long time timer */
> >> + .resources = locomo_lt_resources,
> >> + .num_resources = ARRAY_SIZE(locomo_lt_resources),
> >> + },
> >> + {
> >> + .name = "locomo-spi",
> >> + .resources = locomo_spi_resources,
> >> + .num_resources = ARRAY_SIZE(locomo_spi_resources),
> >> + },
> >> + {
> >> + .name = "locomo-led",
> >> + },
> >> + {
> >> + .name = "locomo-backlight",
> >> + },
> >
> > Please make these:
> >
> >> + { .name = "locomo-led" },
> >> + { .name = "locomo-backlight" },
> >
> > ... and put them at the bottom.
>
> They will be populated by of_compatible lines, so it makes little sense
> to me. What about adding of compatibility lines to this patch?
Also fine.
Although if you assure me you will do it, you can add them separately.
> >> + while (1) {
> >> + regmap_read(lchip->regmap, LOCOMO_ICR, &req);
> >> + req &= 0x0f00;
> >
> > What is this magic number? Please #define it.
>
> Adding comments to this function instead.
Also acceptable.
> >> + if (!req)
> >> + break;
> >> +
> >> + irq = ffs(req) - 9;
> >
> > Minus another random number? Either define it or enter a comment.
> >
> >> +#ifdef CONFIG_PM_SLEEP
> >> +static int locomo_suspend(struct device *dev)
> >> +{
> >> + struct locomo *lchip = dev_get_drvdata(dev);
> >> +
> >> + /* AUDIO */
> >
> > WHY ARE YOU SHOUTING? Ironic eh? ;)
> >
> >> + regmap_write(lchip->regmap, LOCOMO_PAIF, 0x00);
> >> +
> >> + /*
> >> + * Original code disabled the clock depending on leds settings
> >> + * However we disable leds before suspend, thus it's safe
> >> + * to just assume this setting.
> >> + */
> >> + /* CLK32 off */
> >> + regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
> >> +
> >> + /* 22MHz/24MHz clock off */
> >> + regmap_write(lchip->regmap, LOCOMO_ACC, 0x00);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int locomo_resume(struct device *dev)
> >> +{
> >> + struct locomo *lchip = dev_get_drvdata(dev);
> >
> > Do audio and clk sort themselves out?
>
> PAIF and ACC registers are used only by audio parts of the device. However
> there is no current Linux driver for those parts. The registers are cleared
> in case the firmware has set something in them, but in future it will
> be the task
> of the audio driver to properly clear and restore them.
>
> >> + regmap_write(lchip->regmap, LOCOMO_C32K, 0x00);
> >> +
> >> + return 0;
> >> +}
> >> +
>
> [skipped]
> >> +
> >> + if (pdata) {
> >> + locomo_gpio_pdata.gpio_base = pdata->gpio_base;
> >> + locomo_lcd_pdata.comadj = pdata->comadj;
> >> + } else {
> >> + locomo_gpio_pdata.gpio_base = -1;
> >> + locomo_lcd_pdata.comadj = 128;
> >> + }
> >
> > struct locomo_gpio_platform_data locomo_gpio_pdata;
> >
> > locomo_gpio_pdata = devm_kzalloc(<blah>);
> >
> > locomo_cells[GPIO].platform_data = locomo_gpio_pdata;
>
> I do not quite agree with you at this place. The passed platform_data
> will be kmemdup()'ed inside platform core. So the whole struct will be
> duplicated twice inside kmallocate'd memory. Ideally I'd like to drop
> the whole platform_data busyness, but that requires switching to DTS
> first.
Sounds reasonable.
> >> diff --git a/include/linux/mfd/locomo.h b/include/linux/mfd/locomo.h
> >> new file mode 100644
> >> index 0000000..6729767
> >> --- /dev/null
> >> +++ b/include/linux/mfd/locomo.h
>
> >> +/* MCS decoder for boot selecting */
> >> +#define LOCOMO_MCSX0 0x10
> >> +#define LOCOMO_MCSX1 0x14
> >> +#define LOCOMO_MCSX2 0x18
> >> +#define LOCOMO_MCSX3 0x1c
> >
> > These are pretty cryptic. Any way of making them easier to identify.
>
> No way. The names are based on old Sharp code. The drivers do not use
> them, but I'd like to still keep the registers for the reference purposes.
So they are not used at all? Then why do you want to keep them?
> >> +struct locomo_gpio_platform_data {
> >> + unsigned int gpio_base;
> >> +};
> >
> > A struct for a single int seems overkill.
> >
> >> +struct locomo_lcd_platform_data {
> >> + u8 comadj;
> >> +};
> >> +
> >> +struct locomo_platform_data {
> >> + unsigned int gpio_base;
> >> + u8 comadj;
> >> +};
> >
> > Why do you need to pass gpio_base twice?
>
> First: machine file -> core driver
> Second: core driver -> gpio driver
>
> The other way to do the same would be:
>
> struct locomo_gpio_platform_data {
> unsigned int gpio_base;
> };
>
> struct locomo_lcd_platform_data {
> u8 comadj;
> };
>
> struct locomo_platform_data {
> struct locomo_gpio_platform_data gpio_pdata;
> struct locomo_lcd_platform_data lcd_pdata;
> };
>
> And to assign pointers to the passed data in the mfd_cells
> during locomo_probe. Does that look better to you?
Bingo.
--
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 v2 02/17] leds: port locomo leds driver to new locomo core
From: Jacek Anaszewski @ 2015-05-13 10:31 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami, linux-arm-kernel,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-input, linux-leds
In-Reply-To: <CALT56yNJWapNw1XLrzfbUDUz1LF_BB9DfF94H6GhbnBUEP80_w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
> 2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>> On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
>>>
>>> Adapt locomo leds driver to new locomo core setup.
>>>
>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>> ---
>>> drivers/leds/Kconfig | 1 -
>>> drivers/leds/leds-locomo.c | 119
>>> +++++++++++++++++++++++----------------------
>>> 2 files changed, 61 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>> index 966b960..4b4650b 100644
>>> --- a/drivers/leds/Kconfig
>>> +++ b/drivers/leds/Kconfig
>>> @@ -79,7 +79,6 @@ config LEDS_LM3642
>>> config LEDS_LOCOMO
>>> tristate "LED Support for Locomo device"
>>> depends on LEDS_CLASS
>>> - depends on SHARP_LOCOMO
>>
>>
>> Why do you remove this dependency?
>
> Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
> uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
> device and regmap interfaces, so there is no direct dependency on main
> LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
Ack. Shouldn't you also need "select REGMAP_MMIO" ?
>>
>>> help
>>> This option enables support for the LEDs on Sharp Locomo.
>>> Zaurus models SL-5500 and SL-5600.
>>> diff --git a/drivers/leds/leds-locomo.c b/drivers/leds/leds-locomo.c
>>> index 80ba048..7fde812 100644
>>> --- a/drivers/leds/leds-locomo.c
>>> +++ b/drivers/leds/leds-locomo.c
>>> @@ -9,89 +9,92 @@
>>> */
>>>
>>> #include <linux/kernel.h>
>>> -#include <linux/init.h>
>>> -#include <linux/module.h>
>>> -#include <linux/device.h>
>>> #include <linux/leds.h>
>>> -
>>> -#include <mach/hardware.h>
>>> -#include <asm/hardware/locomo.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/mfd/locomo.h>
>>
>>
>> Please keep alphabetical order.
>
> Ack
>
>>
>>> +
>>> +struct locomo_led {
>>> + struct led_classdev led;
>>> + struct regmap *regmap;
>>> + unsigned int reg;
>>> +};
>>>
>>> static void locomoled_brightness_set(struct led_classdev *led_cdev,
>>> - enum led_brightness value, int offset)
>>> + enum led_brightness value)
>>> {
>>> - struct locomo_dev *locomo_dev = LOCOMO_DEV(led_cdev->dev->parent);
>>> - unsigned long flags;
>>> + struct locomo_led *led = container_of(led_cdev, struct locomo_led,
>>> led);
>>>
>>> - local_irq_save(flags);
>>> - if (value)
>>> - locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
>>> offset);
>>> - else
>>> - locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
>>> offset);
>>> - local_irq_restore(flags);
>>> + regmap_write(led->regmap, led->reg,
>>> + value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
>>> }
>>
>>
>> Please use work queue for setting brightness. This is required for the
>> driver to be compatible with led triggers. You can refer to the
>> existing LED drivers on how to implement this.
>
> Hmm. Why? The regmap here uses MMIO access, so it is atomic operation
> and doesn't need to be wrapped into work queue, does it?
Triggers can call brightness_set op in the interrupt context, so it
cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus
I am inferring that it can sleep.
I am not sure if regmap implements its 'lock' op when using MMIO.
The best way to figure this out is turning "LED Timer Trigger" on
in the config and execute:
echo "timer" > trigger
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v2 02/17] leds: port locomo leds driver to new locomo core
From: Dmitry Eremin-Solenikov @ 2015-05-13 14:14 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami, linux-arm-kernel,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-input, linux-leds
In-Reply-To: <555327EA.5060402-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
> On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
>>
>> 2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>>>
>>> On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
>>>>
>>>>
>>>> Adapt locomo leds driver to new locomo core setup.
>>>>
>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>> ---
>>>> drivers/leds/Kconfig | 1 -
>>>> drivers/leds/leds-locomo.c | 119
>>>> +++++++++++++++++++++++----------------------
>>>> 2 files changed, 61 insertions(+), 59 deletions(-)
>>>>
>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>> index 966b960..4b4650b 100644
>>>> --- a/drivers/leds/Kconfig
>>>> +++ b/drivers/leds/Kconfig
>>>> @@ -79,7 +79,6 @@ config LEDS_LM3642
>>>> config LEDS_LOCOMO
>>>> tristate "LED Support for Locomo device"
>>>> depends on LEDS_CLASS
>>>> - depends on SHARP_LOCOMO
>>>
>>>
>>>
>>> Why do you remove this dependency?
>>
>>
>> Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
>> uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
>> device and regmap interfaces, so there is no direct dependency on main
>> LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
>
>
> Ack. Shouldn't you also need "select REGMAP_MMIO" ?
No. Maybe I should add "select REGMAP" instead.
>>>> static void locomoled_brightness_set(struct led_classdev *led_cdev,
>>>> - enum led_brightness value, int offset)
>>>> + enum led_brightness value)
>>>> {
>>>> - struct locomo_dev *locomo_dev >>>> LOCOMO_DEV(led_cdev->dev->parent);
>>>> - unsigned long flags;
>>>> + struct locomo_led *led = container_of(led_cdev, struct
>>>> locomo_led,
>>>> led);
>>>>
>>>> - local_irq_save(flags);
>>>> - if (value)
>>>> - locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
>>>> offset);
>>>> - else
>>>> - locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
>>>> offset);
>>>> - local_irq_restore(flags);
>>>> + regmap_write(led->regmap, led->reg,
>>>> + value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
>>>> }
>>>
>>>
>>>
>>> Please use work queue for setting brightness. This is required for the
>>> driver to be compatible with led triggers. You can refer to the
>>> existing LED drivers on how to implement this.
>>
>>
>> Hmm. Why? The regmap here uses MMIO access, so it is atomic operation
>> and doesn't need to be wrapped into work queue, does it?
>
>
> Triggers can call brightness_set op in the interrupt context, so it
> cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus
> I am inferring that it can sleep.
>
> I am not sure if regmap implements its 'lock' op when using MMIO.
>
> The best way to figure this out is turning "LED Timer Trigger" on
> in the config and execute:
>
> echo "timer" > trigger
It works without any "might sleep/sleeping in atomic context" warnings.
Technically I'd agree with you. If I'm using regmaps, ideally I should not
depend on the exact backend and put working with it to the work queue.
However as this is a driver for quite old IP block, it is not used with
regmap backends other than MMIO, I'd deduce, it's ok to do things
in a more relaxed way and to call regmap_write even from atomic
contexts.
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 02/17] leds: port locomo leds driver to new locomo core
From: Jacek Anaszewski @ 2015-05-13 14:53 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov
Cc: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami, linux-arm-kernel, linux-gpio@vger.kernel.org,
linux-i2c, linux-input, linux-leds
In-Reply-To: <CALT56yOJyZvWF9neC2A09Y_w1Ej41uwWvU=aOOM0idbzUkBiFQ@mail.gmail.com>
On 05/13/2015 04:14 PM, Dmitry Eremin-Solenikov wrote:
> 2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>> On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
>>>
>>> 2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>>>>
>>>> On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
>>>>>
>>>>>
>>>>> Adapt locomo leds driver to new locomo core setup.
>>>>>
>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>> ---
>>>>> drivers/leds/Kconfig | 1 -
>>>>> drivers/leds/leds-locomo.c | 119
>>>>> +++++++++++++++++++++++----------------------
>>>>> 2 files changed, 61 insertions(+), 59 deletions(-)
>>>>>
>>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>>> index 966b960..4b4650b 100644
>>>>> --- a/drivers/leds/Kconfig
>>>>> +++ b/drivers/leds/Kconfig
>>>>> @@ -79,7 +79,6 @@ config LEDS_LM3642
>>>>> config LEDS_LOCOMO
>>>>> tristate "LED Support for Locomo device"
>>>>> depends on LEDS_CLASS
>>>>> - depends on SHARP_LOCOMO
>>>>
>>>>
>>>>
>>>> Why do you remove this dependency?
>>>
>>>
>>> Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
>>> uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
>>> device and regmap interfaces, so there is no direct dependency on main
>>> LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
>>
>>
>> Ack. Shouldn't you also need "select REGMAP_MMIO" ?
>
> No. Maybe I should add "select REGMAP" instead.
REGMAP is enabled by default if REGMAP_MMIO is enabled. Having
REGMAP_MMIO selected would also allow to notice at first glance which
regmap backend is used. This would in turn provide a straightforward
explanation on why brightness_set op is not delegated to work queue.
>>>>> static void locomoled_brightness_set(struct led_classdev *led_cdev,
>>>>> - enum led_brightness value, int offset)
>>>>> + enum led_brightness value)
>>>>> {
>>>>> - struct locomo_dev *locomo_dev >>>>> LOCOMO_DEV(led_cdev->dev->parent);
>>>>> - unsigned long flags;
>>>>> + struct locomo_led *led = container_of(led_cdev, struct
>>>>> locomo_led,
>>>>> led);
>>>>>
>>>>> - local_irq_save(flags);
>>>>> - if (value)
>>>>> - locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase +
>>>>> offset);
>>>>> - else
>>>>> - locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase +
>>>>> offset);
>>>>> - local_irq_restore(flags);
>>>>> + regmap_write(led->regmap, led->reg,
>>>>> + value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
>>>>> }
>>>>
>>>>
>>>>
>>>> Please use work queue for setting brightness. This is required for the
>>>> driver to be compatible with led triggers. You can refer to the
>>>> existing LED drivers on how to implement this.
>>>
>>>
>>> Hmm. Why? The regmap here uses MMIO access, so it is atomic operation
>>> and doesn't need to be wrapped into work queue, does it?
>>
>>
>> Triggers can call brightness_set op in the interrupt context, so it
>> cannot sleep. I see "map->lock(map->lock_arg)" in regmap_write, thus
>> I am inferring that it can sleep.
>>
>> I am not sure if regmap implements its 'lock' op when using MMIO.
>>
>> The best way to figure this out is turning "LED Timer Trigger" on
>> in the config and execute:
>>
>> echo "timer" > trigger
>
> It works without any "might sleep/sleeping in atomic context" warnings.
>
> Technically I'd agree with you. If I'm using regmaps, ideally I should not
> depend on the exact backend and put working with it to the work queue.
> However as this is a driver for quite old IP block, it is not used with
> regmap backends other than MMIO, I'd deduce, it's ok to do things
> in a more relaxed way and to call regmap_write even from atomic
> contexts.
Ack.
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v2 02/17] leds: port locomo leds driver to new locomo core
From: Dmitry Torokhov @ 2015-05-13 16:42 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Dmitry Eremin-Solenikov, Russell King, Daniel Mack,
Robert Jarzmik, Linus Walleij, Alexandre Courbot, Wolfram Sang,
Bryan Wu, Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown,
Jingoo Han, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Liam Girdwood, Andrea Adami, linux-arm-kernel,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-input, linux-leds
In-Reply-To: <5553654F.4010608-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, May 13, 2015 at 04:53:03PM +0200, Jacek Anaszewski wrote:
> On 05/13/2015 04:14 PM, Dmitry Eremin-Solenikov wrote:
> >2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
> >>On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
> >>>
> >>>2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
> >>>>
> >>>>On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
> >>>>>
> >>>>>
> >>>>>Adapt locomo leds driver to new locomo core setup.
> >>>>>
> >>>>>Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >>>>>---
> >>>>> drivers/leds/Kconfig | 1 -
> >>>>> drivers/leds/leds-locomo.c | 119
> >>>>>+++++++++++++++++++++++----------------------
> >>>>> 2 files changed, 61 insertions(+), 59 deletions(-)
> >>>>>
> >>>>>diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> >>>>>index 966b960..4b4650b 100644
> >>>>>--- a/drivers/leds/Kconfig
> >>>>>+++ b/drivers/leds/Kconfig
> >>>>>@@ -79,7 +79,6 @@ config LEDS_LM3642
> >>>>> config LEDS_LOCOMO
> >>>>> tristate "LED Support for Locomo device"
> >>>>> depends on LEDS_CLASS
> >>>>>- depends on SHARP_LOCOMO
> >>>>
> >>>>
> >>>>
> >>>>Why do you remove this dependency?
> >>>
> >>>
> >>>Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
> >>>uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
> >>>device and regmap interfaces, so there is no direct dependency on main
> >>>LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
> >>
> >>
> >>Ack. Shouldn't you also need "select REGMAP_MMIO" ?
> >
> >No. Maybe I should add "select REGMAP" instead.
>
> REGMAP is enabled by default if REGMAP_MMIO is enabled. Having
That is unfortunately not how select works: it does not automatically
select parents for the selected symbol.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
From: Kim, Milo @ 2015-05-13 23:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1431376325-25741-1-git-send-email-seanpaul@chromium.org>
Hi Sean,
On 5/14/2015 4:33 AM, Sean Paul wrote:
>
>
> On Mon, May 11, 2015 at 3:48 PM, Kim, Milo <milo.kim@ti.com
> <mailto:milo.kim@ti.com>> wrote:
>
> Hi Sean,
>
> On 5/12/2015 5:32 AM, Sean Paul wrote:
>
> Don't clear the backlight level when we're going into suspend or
> blanking. Instead, just temporarily set the level to 0 so we retain
> the value when we resume.
>
>
> Could you describe what the problem is? I wrote same code in other
> backlight drivers. So maybe I need to look into them as well.
>
>
> Hi Milo,
> Well, I'm not sure what the expected behavior is, but our userspace
> expects the driver to maintain its brightness across suspend/resume. The
> way things are coded now, the driver will clear the brightness value on
> suspend such that when we resume, the backlight is restored to 0.
Got it. Your patch makes sense. Thanks for catching this.
Best regards,
Milo
^ permalink raw reply
* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
From: Kim, Milo @ 2015-05-13 23:32 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1431376325-25741-1-git-send-email-seanpaul@chromium.org>
On 5/12/2015 5:32 AM, Sean Paul wrote:
> Don't clear the backlight level when we're going into suspend or
> blanking. Instead, just temporarily set the level to 0 so we retain
> the value when we resume.
>
> Reported-by: Benson Leung <bleung@chromium.org>
> Tested-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Acked-by: Milo Kim <milo.kim@ti.com>
^ permalink raw reply
* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
From: Benson Leung @ 2015-05-13 23:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1431376325-25741-1-git-send-email-seanpaul@chromium.org>
On Mon, May 11, 2015 at 1:32 PM, Sean Paul <seanpaul@chromium.org> wrote:
> Don't clear the backlight level when we're going into suspend or
> blanking. Instead, just temporarily set the level to 0 so we retain
> the value when we resume.
>
> Reported-by: Benson Leung <bleung@chromium.org>
> Tested-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* Re: [PATCH v2 02/17] leds: port locomo leds driver to new locomo core
From: Jacek Anaszewski @ 2015-05-14 6:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Dmitry Eremin-Solenikov, Russell King, Daniel Mack,
Robert Jarzmik, Linus Walleij, Alexandre Courbot, Wolfram Sang,
Bryan Wu, Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown,
Jingoo Han, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
Liam Girdwood, Andrea Adami, linux-arm-kernel,
linux-gpio@vger.kernel.org, linux-i2c, linux-input, linux-leds
In-Reply-To: <20150513164248.GB11891@dtor-ws>
On 05/13/2015 06:42 PM, Dmitry Torokhov wrote:
> On Wed, May 13, 2015 at 04:53:03PM +0200, Jacek Anaszewski wrote:
>> On 05/13/2015 04:14 PM, Dmitry Eremin-Solenikov wrote:
>>> 2015-05-13 13:31 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>>>> On 05/12/2015 05:35 PM, Dmitry Eremin-Solenikov wrote:
>>>>>
>>>>> 2015-05-06 18:05 GMT+03:00 Jacek Anaszewski <j.anaszewski@samsung.com>:
>>>>>>
>>>>>> On 04/28/2015 01:55 AM, Dmitry Eremin-Solenikov wrote:
>>>>>>>
>>>>>>>
>>>>>>> Adapt locomo leds driver to new locomo core setup.
>>>>>>>
>>>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>>>> ---
>>>>>>> drivers/leds/Kconfig | 1 -
>>>>>>> drivers/leds/leds-locomo.c | 119
>>>>>>> +++++++++++++++++++++++----------------------
>>>>>>> 2 files changed, 61 insertions(+), 59 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>>>>> index 966b960..4b4650b 100644
>>>>>>> --- a/drivers/leds/Kconfig
>>>>>>> +++ b/drivers/leds/Kconfig
>>>>>>> @@ -79,7 +79,6 @@ config LEDS_LM3642
>>>>>>> config LEDS_LOCOMO
>>>>>>> tristate "LED Support for Locomo device"
>>>>>>> depends on LEDS_CLASS
>>>>>>> - depends on SHARP_LOCOMO
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why do you remove this dependency?
>>>>>
>>>>>
>>>>> Because SHARP_LOCOMO is a Kconfig symbol for the old driver. New driver
>>>>> uses MFD_LOCOMO Kconfig entry. Also the driver now uses generic platform
>>>>> device and regmap interfaces, so there is no direct dependency on main
>>>>> LoCoMo driver. And the policy (IIRC) was not to have such dependencies.
>>>>
>>>>
>>>> Ack. Shouldn't you also need "select REGMAP_MMIO" ?
>>>
>>> No. Maybe I should add "select REGMAP" instead.
>>
>> REGMAP is enabled by default if REGMAP_MMIO is enabled. Having
>
> That is unfortunately not how select works: it does not automatically
> select parents for the selected symbol.
Please look at config REGMAP declaration in the file
drivers/base/regmap/Kconfig:
config REGMAP
default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI ||
REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
select LZO_COMPRESS
select LZO_DECOMPRESS
select IRQ_DOMAIN if REGMAP_IRQ
bool
--
Best Regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH] backlight/lp855x: Don't clear level on suspend/blank
From: Lee Jones @ 2015-05-14 7:25 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1431376325-25741-1-git-send-email-seanpaul@chromium.org>
On Mon, 11 May 2015, Sean Paul wrote:
> Don't clear the backlight level when we're going into suspend or
> blanking. Instead, just temporarily set the level to 0 so we retain
> the value when we resume.
>
> Reported-by: Benson Leung <bleung@chromium.org>
> Tested-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/video/backlight/lp855x_bl.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
Applied with Acks, thanks.
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 08ae72f..74daf7c 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -257,21 +257,15 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> static int lp855x_bl_update_status(struct backlight_device *bl)
> {
> struct lp855x *lp = bl_get_data(bl);
> + int brightness = bl->props.brightness;
>
> if (bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> - bl->props.brightness = 0;
> + brightness = 0;
>
> - if (lp->mode = PWM_BASED) {
> - int br = bl->props.brightness;
> - int max_br = bl->props.max_brightness;
> -
> - lp855x_pwm_ctrl(lp, br, max_br);
> -
> - } else if (lp->mode = REGISTER_BASED) {
> - u8 val = bl->props.brightness;
> -
> - lp855x_write_byte(lp, lp->cfg->reg_brightness, val);
> - }
> + if (lp->mode = PWM_BASED)
> + lp855x_pwm_ctrl(lp, brightness, bl->props.max_brightness);
> + else if (lp->mode = REGISTER_BASED)
> + lp855x_write_byte(lp, lp->cfg->reg_brightness, (u8)brightness);
>
> return 0;
> }
--
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 v4] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-14 14:06 UTC (permalink / raw)
To: cocci
In-Reply-To: <20150510130903.GA30824@kroah.com>
On Sun, May 10, 2015 at 03:09:03PM +0200, Greg KH wrote:
> On Mon, May 04, 2015 at 05:15:51PM -0700, Luis R. Rodriguez wrote:
> > ---
> > drivers/staging/sm750fb/sm750.c | 36 ++++--------------------------------
> > drivers/staging/sm750fb/sm750.h | 3 ---
> > drivers/staging/sm750fb/sm750_hw.c | 3 +--
> > 3 files changed, 5 insertions(+), 37 deletions(-)
>
> Still doesn't apply to my tree:
I did base it on that day's staging tree, staging is just a fast
moving tree. In the future use of SmPL, when provided, to address merge
conflicts might help us on git.
Meanwhile I've addressed the conflict and will send v5 based on today's
tree outlook.
Luis
^ permalink raw reply
* [PATCH v5] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-14 14:07 UTC (permalink / raw)
To: cocci
From: "Luis R. Rodriguez" <mcgrof@suse.com>
The same area used for ioremap() is used for the MTRR area.
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Teddy Wang <teddy.wang@siliconmotion.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: devel@driverdev.osuosl.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/staging/sm750fb/sm750.c | 36 ++++--------------------------------
drivers/staging/sm750fb/sm750.h | 3 ---
drivers/staging/sm750fb/sm750_hw.c | 3 +--
3 files changed, 5 insertions(+), 37 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 5529905..97fe04a 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -16,9 +16,6 @@
#include<linux/vmalloc.h>
#include<linux/pagemap.h>
#include <linux/console.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
#include <asm/fb.h>
#include "sm750.h"
#include "sm750_hw.h"
@@ -47,9 +44,7 @@ typedef int (*PROC_SPEC_INITHW)(struct lynx_share*, struct pci_dev*);
/* common var for all device */
static int g_hwcursor = 1;
static int g_noaccel;
-#ifdef CONFIG_MTRR
static int g_nomtrr;
-#endif
static const char *g_fbmode[] = {NULL, NULL};
static const char *g_def_fbmode = "800x600-16@60";
static char *g_settings = NULL;
@@ -1126,11 +1121,8 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
pr_info("share->revid = %02x\n", share->revid);
share->pdev = pdev;
-#ifdef CONFIG_MTRR
share->mtrr_off = g_nomtrr;
share->mtrr.vram = 0;
- share->mtrr.vram_added = 0;
-#endif
share->accel_off = g_noaccel;
share->dual = g_dualview;
spin_lock_init(&share->slock);
@@ -1158,22 +1150,9 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
goto err_map;
}
-#ifdef CONFIG_MTRR
- if (!share->mtrr_off) {
- pr_info("enable mtrr\n");
- share->mtrr.vram = mtrr_add(share->vidmem_start,
- share->vidmem_size,
- MTRR_TYPE_WRCOMB, 1);
-
- if (share->mtrr.vram < 0) {
- /* don't block driver with the failure of MTRR */
- pr_err("Unable to setup MTRR.\n");
- } else {
- share->mtrr.vram_added = 1;
- pr_info("MTRR added successfully\n");
- }
- }
-#endif
+ if (!share->mtrr_off)
+ share->mtrr.vram = arch_phys_wc_add(share->vidmem_start,
+ share->vidmem_size);
memset_io(share->pvMem, 0, share->vidmem_size);
@@ -1274,12 +1253,7 @@ static void lynxfb_pci_remove(struct pci_dev *pdev)
/* release frame buffer */
framebuffer_release(info);
}
-#ifdef CONFIG_MTRR
- if (share->mtrr.vram_added)
- mtrr_del(share->mtrr.vram,
- share->vidmem_start,
- share->vidmem_size);
-#endif
+ arch_phys_wc_del(share->mtrr.vram);
iounmap(share->pvReg);
iounmap(share->pvMem);
@@ -1321,10 +1295,8 @@ static int __init lynxfb_setup(char *options)
/* options that mean for any lynx chips are configured here */
if (!strncmp(opt, "noaccel", strlen("noaccel")))
g_noaccel = 1;
-#ifdef CONFIG_MTRR
else if (!strncmp(opt, "nomtrr", strlen("nomtrr")))
g_nomtrr = 1;
-#endif
else if (!strncmp(opt, "dual", strlen("dual")))
g_dualview = 1;
else {
diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
index 0847d2b..5528912 100644
--- a/drivers/staging/sm750fb/sm750.h
+++ b/drivers/staging/sm750fb/sm750.h
@@ -51,13 +51,10 @@ struct lynx_share{
struct lynx_accel accel;
int accel_off;
int dual;
-#ifdef CONFIG_MTRR
int mtrr_off;
struct{
int vram;
- int vram_added;
}mtrr;
-#endif
/* all smi graphic adaptor got below attributes */
unsigned long vidmem_start;
unsigned long vidreg_start;
diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index 9f0d06d..4b77eb1 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -85,8 +85,7 @@ int hw_sm750_map(struct lynx_share* share, struct pci_dev* pdev)
}
#endif
- share->pvMem = ioremap(share->vidmem_start,
- share->vidmem_size);
+ share->pvMem = ioremap_wc(share->vidmem_start, share->vidmem_size);
if(!share->pvMem){
pr_err("Map video memory failed\n");
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v3 00/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-05-14 14:26 UTC (permalink / raw)
To: cocci
In-Reply-To: <CALCETrVdAG1NFA_dYHhOyCbwKR3msHZaaVs0MZMkW33_Sw=Kqg@mail.gmail.com>
On Tue, May 05, 2015 at 06:47:31AM -0700, Andy Lutomirski wrote:
> On Mon, May 4, 2015 at 3:33 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On 29/04/15 22:18, Luis R. Rodriguez wrote:
> >> On Tue, Apr 21, 2015 at 1:16 PM, Luis R. Rodriguez
> >> <mcgrof@do-not-panic.com> wrote:
> >>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >>>
> >>> This series addresses simple changes to framebuffer drivers to use
> >>> arch_phys_wc_add() and ioremap_wc() as well as fixing gbefb to add
> >>> missing mtrr_del() calls. These changes are pretty straight forward.
> >>>
> >>> Luis R. Rodriguez (17):
> >>> video: fbdev: radeonfb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: gbefb: add missing mtrr_del() calls
> >>> video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
> >>> video: fbdev: intelfb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: neofb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: nvidia: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: savagefb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: sisfb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: aty: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: i810: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: pm2fb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: pm3fb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: rivafb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: tdfxfb: use arch_phys_wc_add() and ioremap_wc()
> >>> video: fbdev: atmel_lcdfb: use ioremap_wc() for framebuffer
> >>> video: fbdev: geode gxfb: use ioremap_wc() for framebuffer
> >>
> >> Hey folks, these are all pretty straight forward, can anyone take them?
> >
> > I can take these to fbdev tree. Unfortunately I'm not familiar with x86
> > nor mtrr, so I can't really say much about the patches themselves.
> >
>
> I'm on vacation and there's no way I'll be able to usefully review
> these in the next two weeks. I can describe what's going on in case
> it helps:
>
> On x86 there are two ways to get a write-combining MMIO mapping. The
> sane way is ioremap_wc, and the ridiculous way is mtrr_add.
> ioremap_wc does exactly what it appears to do, whereas mtrr_add acts
> on physical addresses, requires power-of-two alignment, and is
> unreliable.
>
> On all recent hardware, ioremap_wc works, and mtrr_add is problematic
> on all hardware even if it often works. The silly thing is that
> mtrr_add pokes at the registers it uses even on hardware with working
> ioremap_wc. This causes problems (those resources are a very limited
> resource).
>
> The solution I came up with a couple years ago is a new API
> arch_phys_wc_add. It is a hint that a physical address range should
> be write-combining. On hardware with working ioremap_wc, it's a
> no-op. On x86 hardware without working ioremap_wc, it just calls
> mtrr_add.
>
> The upshot is that changing mtrr_add with a WC type to
> arch_phys_wc_add is OK as long as the driver also uses ioremap_wc or
> similar _wc APIs. Once everything has been converted, we can unexport
> mtrr_add, which will have all kinds of benefits.
Tomi,
Any chance you can consider this series again? I realize you say you are
not familiar with MTRR but given its prevalence use on framebuffer device
drivers and since you are a framebuffer subsystem maintainer I think its
perhaps fair to ask one of you become familiar with this stuff, or ask
questions if there are any. If its of any help this patch which adds
ioremap_uc() already got merged and the logic to phase MTRR is described
there too:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?idäb6be33c28923d8cde53023e0888b1c5d1a9027
That patch won't even be needed on this series, this series happens to be
straight forward: if you just ioremap_nocache() once and use mtrr_add()
then conver over to ioremap_wc() and use arch_phys_wc_add(). If it already
used ioremap_wc() just convert over from mtrr_add() to arch_phys_wc_add()
Not sure if you will get an Acked-by by x86 folks on this series as they
are busy, but I'm adding bp and hpa just in case.
Luis
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Mauro Carvalho Chehab @ 2015-05-14 21:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1429743853-10254-2-git-send-email-arun.ramamurthy@broadcom.com>
Em Wed, 22 Apr 2015 16:04:10 -0700
Arun Ramamurthy <arun.ramamurthy@broadcom.com> escreveu:
> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> is only recommended when the config is not visible, GENERIC_PHY is changed
> an invisible option. To maintain consistency, all phy providers are changed
> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> dependency, so it is left as "select".
>
> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
> ---
> drivers/ata/Kconfig | 1 -
> drivers/media/platform/exynos4-is/Kconfig | 2 +-
For media part:
Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> drivers/phy/Kconfig | 4 ++--
> drivers/usb/host/Kconfig | 4 ++--
> drivers/video/fbdev/exynos/Kconfig | 2 +-
> 5 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 5f60155..6d2e881 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -301,7 +301,6 @@ config SATA_MV
> tristate "Marvell SATA support"
> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> - select GENERIC_PHY
> help
> This option enables support for the Marvell Serial ATA family.
> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> index b7b2e47..b6f3eaa 100644
> --- a/drivers/media/platform/exynos4-is/Kconfig
> +++ b/drivers/media/platform/exynos4-is/Kconfig
> @@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> config VIDEO_S5P_MIPI_CSIS
> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> depends on REGULATOR
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> help
> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> receiver (MIPI-CSIS) devices.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2962de2..edecdb1 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -5,7 +5,7 @@
> menu "PHY Subsystem"
>
> config GENERIC_PHY
> - bool "PHY Core"
> + bool
> help
> Generic PHY support.
>
> @@ -72,7 +72,7 @@ config PHY_MIPHY365X
> config PHY_RCAR_GEN2
> tristate "Renesas R-Car generation 2 USB PHY driver"
> depends on ARCH_SHMOBILE
> - depends on GENERIC_PHY
> + select GENERIC_PHY
> help
> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 5ad60e4..e2197e2 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> config USB_EHCI_HCD_STI
> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> depends on ARCH_STI && OF
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> select USB_EHCI_HCD_PLATFORM
> help
> Enable support for the on-chip EHCI controller found on
> @@ -409,7 +409,7 @@ config USB_OHCI_HCD_SPEAR
> config USB_OHCI_HCD_STI
> tristate "Support for ST STiHxxx on-chip OHCI USB controller"
> depends on ARCH_STI && OF
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> select USB_OHCI_HCD_PLATFORM
> help
> Enable support for the on-chip OHCI controller found on
> diff --git a/drivers/video/fbdev/exynos/Kconfig b/drivers/video/fbdev/exynos/Kconfig
> index 1f16b46..6c53894 100644
> --- a/drivers/video/fbdev/exynos/Kconfig
> +++ b/drivers/video/fbdev/exynos/Kconfig
> @@ -16,7 +16,7 @@ if EXYNOS_VIDEO
>
> config EXYNOS_MIPI_DSI
> bool "EXYNOS MIPI DSI driver support."
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> help
> This enables support for MIPI-DSI device.
>
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Felipe Balbi @ 2015-05-15 0:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1429743853-10254-2-git-send-email-arun.ramamurthy@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 3072 bytes --]
Hi,
On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> is only recommended when the config is not visible, GENERIC_PHY is changed
> an invisible option. To maintain consistency, all phy providers are changed
> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> dependency, so it is left as "select".
>
> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
> ---
> drivers/ata/Kconfig | 1 -
> drivers/media/platform/exynos4-is/Kconfig | 2 +-
> drivers/phy/Kconfig | 4 ++--
> drivers/usb/host/Kconfig | 4 ++--
> drivers/video/fbdev/exynos/Kconfig | 2 +-
> 5 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 5f60155..6d2e881 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -301,7 +301,6 @@ config SATA_MV
> tristate "Marvell SATA support"
> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> - select GENERIC_PHY
> help
> This option enables support for the Marvell Serial ATA family.
> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> index b7b2e47..b6f3eaa 100644
> --- a/drivers/media/platform/exynos4-is/Kconfig
> +++ b/drivers/media/platform/exynos4-is/Kconfig
> @@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> config VIDEO_S5P_MIPI_CSIS
> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> depends on REGULATOR
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> help
> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> receiver (MIPI-CSIS) devices.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2962de2..edecdb1 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -5,7 +5,7 @@
> menu "PHY Subsystem"
>
> config GENERIC_PHY
> - bool "PHY Core"
> + bool
> help
> Generic PHY support.
>
> @@ -72,7 +72,7 @@ config PHY_MIPHY365X
> config PHY_RCAR_GEN2
> tristate "Renesas R-Car generation 2 USB PHY driver"
> depends on ARCH_SHMOBILE
> - depends on GENERIC_PHY
> + select GENERIC_PHY
so some you changed from depends to select...
> help
> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 5ad60e4..e2197e2 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> config USB_EHCI_HCD_STI
> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> depends on ARCH_STI && OF
> - select GENERIC_PHY
> + depends on GENERIC_PHY
while others you changed from select to depends.
NAK.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] backlight: pwm_bl: Simplify usage of devm_gpiod_get_optional
From: Axel Lin @ 2015-05-16 14:08 UTC (permalink / raw)
To: linux-pwm, linux-fbdev
Cc: Thierry Reding, Alexandre Courbot, Jingoo Han, Lee Jones
Since 39b2bbe3d715 (gpio: add flags argument to gpiod_get*() functions),
the gpiod_get* functions take an additional parameter that allows to
specify direction and initial value for output.
Simplify the usage of devm_gpiod_get_optional accordingly.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/video/backlight/pwm_bl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 3a145a6..57cb9ec 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -241,7 +241,8 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->dev = &pdev->dev;
pb->enabled = false;
- pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable");
+ pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+ GPIOD_OUT_HIGH);
if (IS_ERR(pb->enable_gpio)) {
ret = PTR_ERR(pb->enable_gpio);
goto err_alloc;
@@ -263,9 +264,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->enable_gpio = gpio_to_desc(data->enable_gpio);
}
- if (pb->enable_gpio)
- gpiod_direction_output(pb->enable_gpio, 1);
-
pb->power_supply = devm_regulator_get(&pdev->dev, "power");
if (IS_ERR(pb->power_supply)) {
ret = PTR_ERR(pb->power_supply);
--
2.1.0
^ permalink raw reply related
* [PATCH] staging: sm750fb: fix typo in fb_videomode Array
From: Michel von Czettritz @ 2015-05-16 19:31 UTC (permalink / raw)
To: linux-fbdev
It appears that 'FB_SYNC_HOR_HIGH_ACT' should be assined to 'sync'
and 'FB_VMODE_NONINTERLACED' is a flag for 'vmode'.
In line 96 these flags are ORed and assigned to 'sync'.
Since 'FB_VMODE_NONINTERLACED' is zero, 'sync' will stay the same,
and due to C99 'vmode' is initialized to zero too.
Therefore this doesn't have any implications for runtime-behavior,
but seems to be semantically wrong.
The Patch simply changes the | to a coma. Please check if this is correct.
Signed-off-by: Michel von Czettritz <michel.von.czettritz@gmail.com>
---
drivers/staging/sm750fb/sm750.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 77310ff..8ff29c6 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -94,7 +94,8 @@ static const struct fb_videomode lynx750_ext[] = {
FB_VMODE_NONINTERLACED},
{NULL, 60, 1360, 768, 11804, 208, 64, 23, 1, 144, 3,
- FB_SYNC_HOR_HIGH_ACT|FB_VMODE_NONINTERLACED},
+ FB_SYNC_HOR_HIGH_ACT,
+ FB_VMODE_NONINTERLACED},
/* 1360 x 768 [1.77083:1] */
{NULL, 60, 1360, 768, 11804, 208, 64, 23, 1, 144, 3,
--
2.3.7
^ permalink raw reply related
* [PATCH] uvesafb: make scaling configurable on Nvidia cards
From: Mikulas Patocka @ 2015-05-17 6:56 UTC (permalink / raw)
To: Michal Januszewski
Cc: linux-fbdev, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-kernel
uvesafb: make scaling configurable on Nvidia cards
Nvidia cards have a BIOS function 0x4f14 that allows to set flat panel
scaling. This patch adds a module parameter "scaling" that uses this
function to set the scaling. By default, the parameter is -1, so that the
driver doesn't attempt to call the scaling function.
This patch is useful when using the binary Nvidia graphics driver - in
that case, the console may be only in text mode or VESA mode. By default,
the video card does scaling that degrades font quality and changes aspect
ratio. This patch makes it possible to turn off the scaling and improve
font quality on the console.
The allowed values depend on VESA BIOS. On my card, the following values
are allowed:
-1 - do not change the scaling
0 - scale to full screen
1, 2 - don't scale
3 - scale and preserve aspect ratio
4 - scale with black border around
Example use:
echo 1 >/sys/module/uvesafb/parameters/scaling; fbset 1280x1024-60 -depth 32 -a
- this sets unscaled 1280x1024 video mode that has much sharper font than
the scaled modes.
I'm suggesting to backport this to stable kernels. The patch doesn't
really fix a bug, but it enhances user experience significantly - the
unscaled console font looks much better than the scaled font. By default,
the patch does nothing, so it can't break any existing system.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Index: linux-2.6/drivers/video/fbdev/uvesafb.c
=================================--- linux-2.6.orig/drivers/video/fbdev/uvesafb.c
+++ linux-2.6/drivers/video/fbdev/uvesafb.c
@@ -52,6 +52,7 @@ static u16 maxclk; /* maximum pixel clo
static u16 maxvf; /* maximum vertical frequency */
static u16 maxhf; /* maximum horizontal frequency */
static u16 vbemode; /* force use of a specific VBE mode */
+static short scaling = -1;
static char *mode_option;
static u8 dac_width = 6;
@@ -1245,7 +1246,22 @@ static int uvesafb_set_par(struct fb_inf
task = uvesafb_prep();
if (!task)
return -ENOMEM;
+
+ if (scaling > 0) {
+ /*
+ * We must first reset scaling state with 0. This is workaround
+ * for some supposed BIOS bug - without this, scaling mode 4
+ * could not be set.
+ */
+ uvesafb_reset(task);
+ task->t.regs.eax = 0x4f14;
+ task->t.regs.ebx = 0x0102;
+ task->t.regs.ecx = 0;
+ uvesafb_exec(task);
+ }
+
setmode:
+ uvesafb_reset(task);
task->t.regs.eax = 0x4f02;
task->t.regs.ebx = mode->mode_id | 0x4000; /* use LFB */
@@ -1296,7 +1312,6 @@ setmode:
printk(KERN_WARNING "uvesafb: mode switch failed "
"(eax=0x%x, err=%d). Trying again with "
"default timings.\n", task->t.regs.eax, err);
- uvesafb_reset(task);
kfree(crtc);
crtc = NULL;
info->var.pixclock = 0;
@@ -1330,6 +1345,14 @@ setmode:
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
info->fix.line_length = mode->bytes_per_scan_line;
+ if (scaling >= 0) {
+ uvesafb_reset(task);
+ task->t.regs.eax = 0x4f14;
+ task->t.regs.ebx = 0x0102;
+ task->t.regs.ecx = scaling;
+ uvesafb_exec(task);
+ }
+
out:
kfree(crtc);
uvesafb_free(task);
@@ -2019,6 +2042,9 @@ MODULE_PARM_DESC(vbemode,
"VBE mode number to set, overrides the 'mode' option");
module_param_string(v86d, v86d_path, PATH_MAX, 0660);
MODULE_PARM_DESC(v86d, "Path to the v86d userspace helper.");
+module_param(scaling, short, 0660);
+MODULE_PARM_DESC(scaling,
+ "Scaling option for nvidia (0 - scaled, 1 - unscaled centered, 2 - unscaled in left corner");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Michal Januszewski <spock@gentoo.org>");
Index: linux-2.6/Documentation/fb/uvesafb.txt
=================================--- linux-2.6.orig/Documentation/fb/uvesafb.txt
+++ linux-2.6/Documentation/fb/uvesafb.txt
@@ -128,6 +128,14 @@ v86d:path
need to use it and have uvesafb built into the kernel, use
uvesafb.v86d="path".
+scaling:n
+ Set scaling on Nvidia cards. Possible values depend on card's BIOS.
+ -1 - do not change the scaling
+ 0 - scale to full screen
+ 1, 2 - don't scale
+ 3 - scale and preserve aspect ratio
+ 4 - scale with black border around
+
Additionally, the following parameters may be provided. They all override the
EDID-provided values and BIOS defaults. Refer to your monitor's specs to get
the correct values for maxhf, maxvf and maxclk for your hardware.
^ permalink raw reply
* [PATCH v3 00/17] new LoCoMo driver set
From: Dmitry Eremin-Solenikov @ 2015-05-17 16:27 UTC (permalink / raw)
To: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel
Sharp Zaurus SL-5500 and SL-5600 use special companion Gate Array.
With this patchset I tried to modernise and restructure the LoCoMo drivers.
* Use platform bus and mfd core code
* Use GPIO API instead of custom locomo GPIO code
* Use irqdomains to manage IRQs
* Split mixed locomolcd driver to lcd and backlight parts
* Implement IRQ handling in GPIO driver
* Implement SPI driver used as a host for mmc_spi device to control SD cards
Changes since V2:
* Implemented most of review findings (lots of small changes)
* Added comments where the review feedback was ignored
The following changes since commit b787f68c36d49bb1d9236f403813641efa74a031:
Linux 4.1-rc1 (2015-04-26 17:59:10 -0700)
are available in the git repository at:
git://git.infradead.org/users/dbaryshkov/zaurus.git locomo-v3
for you to fetch changes up to c7405f110836863287c505650ac2133fecabea7b:
ARM: drop old LoCoMo driver (2015-05-17 19:23:20 +0300)
----------------------------------------------------------------
Dmitry Eremin-Solenikov (17):
mfd: add new driver for Sharp LoCoMo
leds: port locomo leds driver to new locomo core
input: convert LoCoMo keyboard driver to use new locomo core
input: make LoCoMo keyboard driver support both poodle and collie
video: backlight: add new locomo backlight driver
video: lcd: add LoCoMo LCD driver
gpio: port LoCoMo gpio support from old driver
gpio: locomo: implement per-pin irq handling
spi: add locomo SPI driver
i2c: add locomo i2c driver
ARM: sa1100: make collie use new locomo drivers
ARM: sa1100: don't preallocate IRQ space for locomo
ASoC: pxa: poodle: make use of new locomo GPIO interface
ARM: pxa: poodle: use new LoCoMo driver
ARM: pxa: poodle: don't preallocate IRQ space for locomo
video: backlight: drop old locomo bl/lcd driver
ARM: drop old LoCoMo driver
arch/arm/common/Kconfig | 3 -
arch/arm/common/Makefile | 1 -
arch/arm/common/locomo.c | 914 -----------------------------
arch/arm/include/asm/hardware/locomo.h | 221 -------
arch/arm/mach-pxa/Kconfig | 1 -
arch/arm/mach-pxa/include/mach/poodle.h | 8 +-
arch/arm/mach-pxa/poodle.c | 58 +-
arch/arm/mach-sa1100/Kconfig | 1 -
arch/arm/mach-sa1100/collie.c | 213 +++++--
arch/arm/mach-sa1100/include/mach/collie.h | 16 +-
arch/arm/mach-sa1100/include/mach/irqs.h | 19 +-
drivers/gpio/Kconfig | 13 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-locomo.c | 285 +++++++++
drivers/i2c/busses/Kconfig | 12 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-locomo.c | 135 +++++
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/locomokbd.c | 264 ++++-----
drivers/leds/Kconfig | 2 +-
drivers/leds/leds-locomo.c | 117 ++--
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/locomo.c | 346 +++++++++++
drivers/spi/Kconfig | 10 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-locomo.c | 332 +++++++++++
drivers/video/backlight/Kconfig | 16 +-
drivers/video/backlight/Makefile | 3 +-
drivers/video/backlight/locomo_bl.c | 153 +++++
drivers/video/backlight/locomo_lcd.c | 285 +++++++++
drivers/video/backlight/locomolcd.c | 255 --------
include/linux/mfd/locomo.h | 167 ++++++
sound/soc/pxa/poodle.c | 52 +-
34 files changed, 2216 insertions(+), 1702 deletions(-)
delete mode 100644 arch/arm/common/locomo.c
delete mode 100644 arch/arm/include/asm/hardware/locomo.h
create mode 100644 drivers/gpio/gpio-locomo.c
create mode 100644 drivers/i2c/busses/i2c-locomo.c
create mode 100644 drivers/mfd/locomo.c
create mode 100644 drivers/spi/spi-locomo.c
create mode 100644 drivers/video/backlight/locomo_bl.c
create mode 100644 drivers/video/backlight/locomo_lcd.c
delete mode 100644 drivers/video/backlight/locomolcd.c
create mode 100644 include/linux/mfd/locomo.h
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH v3 02/17] leds: port locomo leds driver to new locomo core
From: Dmitry Eremin-Solenikov @ 2015-05-17 16:27 UTC (permalink / raw)
To: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel
In-Reply-To: <1431880077-26321-1-git-send-email-dbaryshkov@gmail.com>
Adapt locomo leds driver to new locomo core setup.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
drivers/leds/Kconfig | 2 +-
drivers/leds/leds-locomo.c | 117 +++++++++++++++++++++++----------------------
2 files changed, 61 insertions(+), 58 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 966b960..d086aac 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -79,7 +79,7 @@ config LEDS_LM3642
config LEDS_LOCOMO
tristate "LED Support for Locomo device"
depends on LEDS_CLASS
- depends on SHARP_LOCOMO
+ depends on MFD_LOCOMO
help
This option enables support for the LEDs on Sharp Locomo.
Zaurus models SL-5500 and SL-5600.
diff --git a/drivers/leds/leds-locomo.c b/drivers/leds/leds-locomo.c
index 80ba048..a4286e9 100644
--- a/drivers/leds/leds-locomo.c
+++ b/drivers/leds/leds-locomo.c
@@ -9,89 +9,92 @@
*/
#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/device.h>
#include <linux/leds.h>
+#include <linux/mfd/locomo.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
-#include <mach/hardware.h>
-#include <asm/hardware/locomo.h>
+struct locomo_led {
+ struct led_classdev led;
+ struct regmap *regmap;
+ unsigned int reg;
+};
static void locomoled_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness value, int offset)
-{
- struct locomo_dev *locomo_dev = LOCOMO_DEV(led_cdev->dev->parent);
- unsigned long flags;
-
- local_irq_save(flags);
- if (value)
- locomo_writel(LOCOMO_LPT_TOFH, locomo_dev->mapbase + offset);
- else
- locomo_writel(LOCOMO_LPT_TOFL, locomo_dev->mapbase + offset);
- local_irq_restore(flags);
-}
-
-static void locomoled_brightness_set0(struct led_classdev *led_cdev,
enum led_brightness value)
{
- locomoled_brightness_set(led_cdev, value, LOCOMO_LPT0);
+ struct locomo_led *led = container_of(led_cdev, struct locomo_led, led);
+
+ regmap_write(led->regmap, led->reg,
+ value ? LOCOMO_LPT_TOFH : LOCOMO_LPT_TOFL);
}
-static void locomoled_brightness_set1(struct led_classdev *led_cdev,
- enum led_brightness value)
+static int locomo_led_register(
+ struct device *dev,
+ struct locomo_led *led,
+ const char *name,
+ const char *trigger,
+ struct regmap *regmap,
+ unsigned int reg)
{
- locomoled_brightness_set(led_cdev, value, LOCOMO_LPT1);
+ led->led.name = name;
+ led->led.flags = LED_CORE_SUSPENDRESUME;
+ led->led.default_trigger = trigger;
+ led->led.brightness_set = locomoled_brightness_set;
+ led->regmap = regmap;
+ led->reg = reg;
+
+ return devm_led_classdev_register(dev, &led->led);
}
-static struct led_classdev locomo_led0 = {
- .name = "locomo:amber:charge",
- .default_trigger = "main-battery-charging",
- .brightness_set = locomoled_brightness_set0,
-};
-
-static struct led_classdev locomo_led1 = {
- .name = "locomo:green:mail",
- .default_trigger = "nand-disk",
- .brightness_set = locomoled_brightness_set1,
-};
-
-static int locomoled_probe(struct locomo_dev *ldev)
+static int locomoled_probe(struct platform_device *pdev)
{
int ret;
-
- ret = led_classdev_register(&ldev->dev, &locomo_led0);
+ struct locomo_led *leds;
+ struct regmap *regmap;
+
+ leds = devm_kzalloc(&pdev->dev, 2 * sizeof(*leds), GFP_KERNEL);
+ if (!leds)
+ return -ENOMEM;
+
+ regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!regmap)
+ return -ENODEV;
+
+ ret = locomo_led_register(
+ &pdev->dev,
+ leds,
+ "locomo:amber:charge",
+ "main-battery-charging",
+ regmap,
+ LOCOMO_LPT0);
if (ret < 0)
return ret;
- ret = led_classdev_register(&ldev->dev, &locomo_led1);
+ ret = locomo_led_register(
+ &pdev->dev,
+ leds + 1,
+ "locomo:green:mail",
+ "mmc0",
+ regmap,
+ LOCOMO_LPT1);
if (ret < 0)
- led_classdev_unregister(&locomo_led0);
-
- return ret;
-}
+ return ret;
-static int locomoled_remove(struct locomo_dev *dev)
-{
- led_classdev_unregister(&locomo_led0);
- led_classdev_unregister(&locomo_led1);
return 0;
}
-static struct locomo_driver locomoled_driver = {
- .drv = {
- .name = "locomoled"
+static struct platform_driver locomoled_driver = {
+ .driver = {
+ .name = "locomo-led"
},
- .devid = LOCOMO_DEVID_LED,
.probe = locomoled_probe,
- .remove = locomoled_remove,
};
-static int __init locomoled_init(void)
-{
- return locomo_driver_register(&locomoled_driver);
-}
-module_init(locomoled_init);
+module_platform_driver(locomoled_driver);
MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
MODULE_DESCRIPTION("Locomo LED driver");
MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-led");
--
2.1.4
^ permalink raw reply related
* [PATCH v3 03/17] input: convert LoCoMo keyboard driver to use new locomo core
From: Dmitry Eremin-Solenikov @ 2015-05-17 16:27 UTC (permalink / raw)
To: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami
Cc: linux-arm-kernel, linux-gpio, linux-input, linux-leds, linux-spi,
linux-fbdev, alsa-devel
In-Reply-To: <1431880077-26321-1-git-send-email-dbaryshkov@gmail.com>
As LoCoMo is switching to new device model, adapt keyboard driver to
support new locomo core driver.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/locomokbd.c | 256 ++++++++++++++++++-------------------
2 files changed, 125 insertions(+), 133 deletions(-)
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 106fbac..9a20487 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -337,7 +337,7 @@ config KEYBOARD_LM8333
config KEYBOARD_LOCOMO
tristate "LoCoMo Keyboard Support"
- depends on SHARP_LOCOMO
+ depends on MFD_LOCOMO
help
Say Y here if you are running Linux on a Sharp Zaurus Collie or Poodle based PDA
diff --git a/drivers/input/keyboard/locomokbd.c b/drivers/input/keyboard/locomokbd.c
index c94d610..1657bc0 100644
--- a/drivers/input/keyboard/locomokbd.c
+++ b/drivers/input/keyboard/locomokbd.c
@@ -23,28 +23,25 @@
*
*/
-#include <linux/slab.h>
-#include <linux/module.h>
+#include <linux/delay.h>
#include <linux/init.h>
#include <linux/input.h>
-#include <linux/delay.h>
-#include <linux/device.h>
#include <linux/interrupt.h>
-#include <linux/ioport.h>
-
-#include <asm/hardware/locomo.h>
-#include <asm/irq.h>
-
-MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
-MODULE_DESCRIPTION("LoCoMo keyboard driver");
-MODULE_LICENSE("GPL");
-
-#define LOCOMOKBD_NUMKEYS 128
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/mfd/locomo.h>
#define KEY_ACTIVITY KEY_F16
#define KEY_CONTACT KEY_F18
#define KEY_CENTER KEY_F15
+#define KB_ROWS 16
+#define KB_COLS 8
+#define LOCOMOKBD_NUMKEYS (KB_ROWS * KB_COLS)
+#define SCANCODE(c, r) (((c)<<4) + (r) + 1)
+
static const unsigned char
locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
0, KEY_ESC, KEY_ACTIVITY, 0, 0, 0, 0, 0, 0, 0, /* 0 - 9 */
@@ -53,7 +50,7 @@ locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
0, 0, 0, KEY_CENTER, 0, KEY_MAIL, 0, 0, 0, 0, /* 30 - 39 */
0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_RIGHT, /* 40 - 49 */
KEY_UP, KEY_LEFT, 0, 0, KEY_P, 0, KEY_O, KEY_I, KEY_Y, KEY_T, /* 50 - 59 */
- KEY_E, KEY_W, 0, 0, 0, 0, KEY_DOWN, KEY_ENTER, 0, 0, /* 60 - 69 */
+ KEY_E, KEY_W, 0, 0, 0, 0, KEY_DOWN, KEY_KPENTER, 0, 0, /* 60 - 69 */
KEY_BACKSPACE, 0, KEY_L, KEY_U, KEY_H, KEY_R, KEY_D, KEY_Q, 0, 0, /* 70 - 79 */
0, 0, 0, 0, 0, 0, KEY_ENTER, KEY_RIGHTSHIFT, KEY_K, KEY_J, /* 80 - 89 */
KEY_G, KEY_F, KEY_X, KEY_S, 0, 0, 0, 0, 0, 0, /* 90 - 99 */
@@ -62,20 +59,14 @@ locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
KEY_M, KEY_SPACE, KEY_V, KEY_APOSTROPHE, KEY_SLASH, 0, 0, 0 /* 120 - 128 */
};
-#define KB_ROWS 16
-#define KB_COLS 8
-#define KB_ROWMASK(r) (1 << (r))
-#define SCANCODE(c,r) ( ((c)<<4) + (r) + 1 )
-
#define KB_DELAY 8
-#define SCAN_INTERVAL (HZ/10)
struct locomokbd {
unsigned char keycode[LOCOMOKBD_NUMKEYS];
struct input_dev *input;
- char phys[32];
- unsigned long base;
+ struct regmap *regmap;
+ int irq;
spinlock_t lock;
struct timer_list timer;
@@ -84,37 +75,33 @@ struct locomokbd {
};
/* helper functions for reading the keyboard matrix */
-static inline void locomokbd_charge_all(unsigned long membase)
+static void locomokbd_charge_all(struct locomokbd *locomokbd)
{
- locomo_writel(0x00FF, membase + LOCOMO_KSC);
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, 0x00ff);
}
-static inline void locomokbd_activate_all(unsigned long membase)
+static void locomokbd_activate_all(struct locomokbd *locomokbd)
{
- unsigned long r;
-
- locomo_writel(0, membase + LOCOMO_KSC);
- r = locomo_readl(membase + LOCOMO_KIC);
- r &= 0xFEFF;
- locomo_writel(r, membase + LOCOMO_KIC);
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, 0);
+ regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x100, 0);
}
-static inline void locomokbd_activate_col(unsigned long membase, int col)
+static void locomokbd_activate_col(struct locomokbd *locomokbd, int col)
{
unsigned short nset;
unsigned short nbset;
- nset = 0xFF & ~(1 << col);
+ nset = 0xFF & ~BIT(col);
nbset = (nset << 8) + nset;
- locomo_writel(nbset, membase + LOCOMO_KSC);
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, nbset);
}
-static inline void locomokbd_reset_col(unsigned long membase, int col)
+static void locomokbd_reset_col(struct locomokbd *locomokbd, int col)
{
unsigned short nbset;
- nbset = ((0xFF & ~(1 << col)) << 8) + 0xFF;
- locomo_writel(nbset, membase + LOCOMO_KSC);
+ nbset = ((0xFF & ~BIT(col)) << 8) + 0xFF;
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, nbset);
}
/*
@@ -129,24 +116,25 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
unsigned int row, col, rowd;
unsigned long flags;
unsigned int num_pressed;
- unsigned long membase = locomokbd->base;
+ bool esc_pressed = false;
spin_lock_irqsave(&locomokbd->lock, flags);
- locomokbd_charge_all(membase);
+ locomokbd_charge_all(locomokbd);
num_pressed = 0;
for (col = 0; col < KB_COLS; col++) {
-
- locomokbd_activate_col(membase, col);
+ udelay(KB_DELAY);
+ locomokbd_activate_col(locomokbd, col);
udelay(KB_DELAY);
- rowd = ~locomo_readl(membase + LOCOMO_KIB);
+ regmap_read(locomokbd->regmap, LOCOMO_KIB, &rowd);
+ rowd = ~rowd;
for (row = 0; row < KB_ROWS; row++) {
unsigned int scancode, pressed, key;
scancode = SCANCODE(col, row);
- pressed = rowd & KB_ROWMASK(row);
+ pressed = rowd & BIT(row);
key = locomokbd->keycode[scancode];
input_report_key(locomokbd->input, key, pressed);
@@ -158,30 +146,32 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
/* The "Cancel/ESC" key is labeled "On/Off" on
* Collie and Poodle and should suspend the device
* if it was pressed for more than a second. */
- if (unlikely(key = KEY_ESC)) {
- if (!time_after(jiffies,
- locomokbd->suspend_jiffies + HZ))
- continue;
- if (locomokbd->count_cancel++
- != (HZ/SCAN_INTERVAL + 1))
- continue;
- input_event(locomokbd->input, EV_PWR,
- KEY_SUSPEND, 1);
- locomokbd->suspend_jiffies = jiffies;
- } else
- locomokbd->count_cancel = 0;
+ if (unlikely(key = KEY_ESC))
+ esc_pressed = true;
}
- locomokbd_reset_col(membase, col);
+ locomokbd_reset_col(locomokbd, col);
}
- locomokbd_activate_all(membase);
+ locomokbd_activate_all(locomokbd);
input_sync(locomokbd->input);
/* if any keys are pressed, enable the timer */
if (num_pressed)
- mod_timer(&locomokbd->timer, jiffies + SCAN_INTERVAL);
+ mod_timer(&locomokbd->timer, jiffies + msecs_to_jiffies(100));
else
+ regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x10);
+
+
+ if (esc_pressed && time_after(jiffies,
+ locomokbd->suspend_jiffies + msecs_to_jiffies(1000))) {
+ if (locomokbd->count_cancel++ > 20) {
+ input_event(locomokbd->input, EV_PWR,
+ KEY_SUSPEND, 1);
+ locomokbd->suspend_jiffies = jiffies;
+ }
+ } else {
locomokbd->count_cancel = 0;
+ }
spin_unlock_irqrestore(&locomokbd->lock, flags);
}
@@ -192,18 +182,17 @@ static void locomokbd_scankeyboard(struct locomokbd *locomokbd)
static irqreturn_t locomokbd_interrupt(int irq, void *dev_id)
{
struct locomokbd *locomokbd = dev_id;
- u16 r;
+ unsigned int r;
- r = locomo_readl(locomokbd->base + LOCOMO_KIC);
+ regmap_read(locomokbd->regmap, LOCOMO_KIC, &r);
if ((r & 0x0001) = 0)
return IRQ_HANDLED;
- locomo_writel(r & ~0x0100, locomokbd->base + LOCOMO_KIC); /* Ack */
+ /* Mask and Ack */
+ regmap_write(locomokbd->regmap, LOCOMO_KIC, r & ~0x110);
- /** wait chattering delay **/
- udelay(100);
+ mod_timer(&locomokbd->timer, jiffies + msecs_to_jiffies(1));
- locomokbd_scankeyboard(locomokbd);
return IRQ_HANDLED;
}
@@ -220,47 +209,39 @@ static void locomokbd_timer_callback(unsigned long data)
static int locomokbd_open(struct input_dev *dev)
{
struct locomokbd *locomokbd = input_get_drvdata(dev);
- u16 r;
-
- r = locomo_readl(locomokbd->base + LOCOMO_KIC) | 0x0010;
- locomo_writel(r, locomokbd->base + LOCOMO_KIC);
- return 0;
+
+ return regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x10);
}
static void locomokbd_close(struct input_dev *dev)
{
struct locomokbd *locomokbd = input_get_drvdata(dev);
- u16 r;
-
- r = locomo_readl(locomokbd->base + LOCOMO_KIC) & ~0x0010;
- locomo_writel(r, locomokbd->base + LOCOMO_KIC);
+
+ regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x0);
+
+ del_timer_sync(&locomokbd->timer);
}
-static int locomokbd_probe(struct locomo_dev *dev)
+static int locomokbd_probe(struct platform_device *dev)
{
struct locomokbd *locomokbd;
struct input_dev *input_dev;
int i, err;
- locomokbd = kzalloc(sizeof(struct locomokbd), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!locomokbd || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ locomokbd = devm_kzalloc(&dev->dev, sizeof(struct locomokbd),
+ GFP_KERNEL);
+ if (!locomokbd)
+ return -ENOMEM;
- /* try and claim memory region */
- if (!request_mem_region((unsigned long) dev->mapbase,
- dev->length,
- LOCOMO_DRIVER_NAME(dev))) {
- err = -EBUSY;
- printk(KERN_ERR "locomokbd: Can't acquire access to io memory for keyboard\n");
- goto err_free_mem;
- }
+ locomokbd->regmap = dev_get_regmap(dev->dev.parent, NULL);
+ if (!locomokbd->regmap)
+ return -EINVAL;
- locomo_set_drvdata(dev, locomokbd);
+ locomokbd->irq = platform_get_irq(dev, 0);
+ if (locomokbd->irq < 0)
+ return -ENXIO;
- locomokbd->base = (unsigned long) dev->mapbase;
+ platform_set_drvdata(dev, locomokbd);
spin_lock_init(&locomokbd->lock);
@@ -270,11 +251,13 @@ static int locomokbd_probe(struct locomo_dev *dev)
locomokbd->suspend_jiffies = jiffies;
- locomokbd->input = input_dev;
- strcpy(locomokbd->phys, "locomokbd/input0");
+ input_dev = devm_input_allocate_device(&dev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+ locomokbd->input = input_dev;
input_dev->name = "LoCoMo keyboard";
- input_dev->phys = locomokbd->phys;
+ input_dev->phys = "locomokbd/input0";
input_dev->id.bustype = BUS_HOST;
input_dev->id.vendor = 0x0001;
input_dev->id.product = 0x0001;
@@ -291,72 +274,81 @@ static int locomokbd_probe(struct locomo_dev *dev)
input_set_drvdata(input_dev, locomokbd);
- memcpy(locomokbd->keycode, locomokbd_keycode, sizeof(locomokbd->keycode));
+ memcpy(locomokbd->keycode,
+ locomokbd_keycode,
+ sizeof(locomokbd->keycode));
+
for (i = 0; i < LOCOMOKBD_NUMKEYS; i++)
- set_bit(locomokbd->keycode[i], input_dev->keybit);
- clear_bit(0, input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, locomokbd->keycode[i]);
+ input_set_capability(input_dev, EV_PWR, KEY_SUSPEND);
+ __set_bit(EV_REP, input_dev->evbit);
+
+ regmap_write(locomokbd->regmap, LOCOMO_KCMD, 1);
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, 0x0);
+ regmap_write(locomokbd->regmap, LOCOMO_KIC, 0x0);
/* attempt to get the interrupt */
- err = request_irq(dev->irq[0], locomokbd_interrupt, 0, "locomokbd", locomokbd);
+ err = devm_request_irq(&dev->dev, locomokbd->irq, locomokbd_interrupt,
+ 0, "locomokbd", locomokbd);
if (err) {
- printk(KERN_ERR "locomokbd: Can't get irq for keyboard\n");
- goto err_release_region;
+ dev_err(&dev->dev, "locomokbd: Can't get irq for keyboard\n");
+ return err;
}
err = input_register_device(locomokbd->input);
if (err)
- goto err_free_irq;
+ return err;
return 0;
- err_free_irq:
- free_irq(dev->irq[0], locomokbd);
- err_release_region:
- release_mem_region((unsigned long) dev->mapbase, dev->length);
- locomo_set_drvdata(dev, NULL);
- err_free_mem:
- input_free_device(input_dev);
- kfree(locomokbd);
-
return err;
}
-static int locomokbd_remove(struct locomo_dev *dev)
+static int locomokbd_remove(struct platform_device *dev)
+{
+ return 0;
+}
+
+static int __maybe_unused locomokbd_suspend(struct device *dev)
{
- struct locomokbd *locomokbd = locomo_get_drvdata(dev);
+ struct locomokbd *locomokbd = dev_get_drvdata(dev);
- free_irq(dev->irq[0], locomokbd);
+ regmap_update_bits(locomokbd->regmap, LOCOMO_KIC, 0x10, 0x0);
del_timer_sync(&locomokbd->timer);
- input_unregister_device(locomokbd->input);
- locomo_set_drvdata(dev, NULL);
+ return 0;
+}
- release_mem_region((unsigned long) dev->mapbase, dev->length);
+static int __maybe_unused locomokbd_resume(struct device *dev)
+{
+ struct locomokbd *locomokbd = dev_get_drvdata(dev);
- kfree(locomokbd);
+ regmap_write(locomokbd->regmap, LOCOMO_KCMD, 1);
+ regmap_write(locomokbd->regmap, LOCOMO_KSC, 0);
+ regmap_write(locomokbd->regmap, LOCOMO_KIC, 0x0);
+
+ /* Rescan keyboard only if we are open by somebody */
+ if (locomokbd->input->users)
+ locomokbd_scankeyboard(locomokbd);
return 0;
}
-static struct locomo_driver keyboard_driver = {
- .drv = {
- .name = "locomokbd"
+static SIMPLE_DEV_PM_OPS(locomo_kbd_pm, locomokbd_suspend, locomokbd_resume);
+
+static struct platform_driver locomokbd_driver = {
+ .driver = {
+ .name = "locomo-kbd",
+ .pm = &locomo_kbd_pm,
},
- .devid = LOCOMO_DEVID_KEYBOARD,
.probe = locomokbd_probe,
.remove = locomokbd_remove,
};
-static int __init locomokbd_init(void)
-{
- return locomo_driver_register(&keyboard_driver);
-}
-
-static void __exit locomokbd_exit(void)
-{
- locomo_driver_unregister(&keyboard_driver);
-}
+module_platform_driver(locomokbd_driver);
-module_init(locomokbd_init);
-module_exit(locomokbd_exit);
+MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>");
+MODULE_DESCRIPTION("LoCoMo keyboard driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:locomo-kbd");
--
2.1.4
^ permalink raw reply related
* [PATCH v3 04/17] input: make LoCoMo keyboard driver support both poodle and collie
From: Dmitry Eremin-Solenikov @ 2015-05-17 16:27 UTC (permalink / raw)
To: Russell King, Daniel Mack, Robert Jarzmik, Linus Walleij,
Alexandre Courbot, Wolfram Sang, Dmitry Torokhov, Bryan Wu,
Richard Purdie, Samuel Ortiz, Lee Jones, Mark Brown, Jingoo Han,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Liam Girdwood,
Andrea Adami
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw
In-Reply-To: <1431880077-26321-1-git-send-email-dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Keyboards on collie and poodle differ only in wiring of 'Home' key.
Instead of complicating the driver with platform data, just check for
the machine for the time being. This will be converted to DTS property
sometime in the future.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
drivers/input/keyboard/locomokbd.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/locomokbd.c b/drivers/input/keyboard/locomokbd.c
index 1657bc0..b422608 100644
--- a/drivers/input/keyboard/locomokbd.c
+++ b/drivers/input/keyboard/locomokbd.c
@@ -33,6 +33,9 @@
#include <linux/slab.h>
#include <linux/mfd/locomo.h>
+/* There is one minor difference between mappings on poodle and collie */
+#include <asm/mach-types.h>
+
#define KEY_ACTIVITY KEY_F16
#define KEY_CONTACT KEY_F18
#define KEY_CENTER KEY_F15
@@ -45,7 +48,7 @@
static const unsigned char
locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
0, KEY_ESC, KEY_ACTIVITY, 0, 0, 0, 0, 0, 0, 0, /* 0 - 9 */
- 0, 0, 0, 0, 0, 0, 0, KEY_MENU, KEY_HOME, KEY_CONTACT, /* 10 - 19 */
+ 0, 0, 0, 0, 0, 0, 0, KEY_MENU, 0, KEY_CONTACT, /* 10 - 19 */
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 20 - 29 */
0, 0, 0, KEY_CENTER, 0, KEY_MAIL, 0, 0, 0, 0, /* 30 - 39 */
0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_RIGHT, /* 40 - 49 */
@@ -278,6 +281,11 @@ static int locomokbd_probe(struct platform_device *dev)
locomokbd_keycode,
sizeof(locomokbd->keycode));
+ if (machine_is_collie())
+ locomokbd->keycode[18] = KEY_HOME;
+ else
+ locomokbd->keycode[3] = KEY_HOME;
+
for (i = 0; i < LOCOMOKBD_NUMKEYS; i++)
input_set_capability(input_dev, EV_KEY, locomokbd->keycode[i]);
input_set_capability(input_dev, EV_PWR, KEY_SUSPEND);
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox