| Message ID | 1433797008-6246-1-git-send-email-dbaryshkov@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: > Old locomolcd driver is now completely obsolete by new locomo_bl and > locomo_lcd drivers, so let's drop it. > > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > --- > drivers/video/backlight/locomolcd.c | 255 ------------------------------------ > 1 file changed, 255 deletions(-) > delete mode 100644 drivers/video/backlight/locomolcd.c Acked-by: Lee Jones <lee.jones@linaro.org> > diff --git a/drivers/video/backlight/locomolcd.c b/drivers/video/backlight/locomolcd.c > deleted file mode 100644 > index 6c3ec42..0000000 > --- a/drivers/video/backlight/locomolcd.c > +++ /dev/null > @@ -1,255 +0,0 @@ > -/* > - * Backlight control code for Sharp Zaurus SL-5500 > - * > - * Copyright 2005 John Lenz <lenz@cs.wisc.edu> > - * Maintainer: Pavel Machek <pavel@ucw.cz> (unless John wants to :-) > - * GPL v2 > - * > - * This driver assumes single CPU. That's okay, because collie is > - * slightly old hardware, and no one is going to retrofit second CPU to > - * old PDA. > - */ > - > -/* LCD power functions */ > -#include <linux/module.h> > -#include <linux/init.h> > -#include <linux/delay.h> > -#include <linux/device.h> > -#include <linux/interrupt.h> > -#include <linux/fb.h> > -#include <linux/backlight.h> > - > -#include <asm/hardware/locomo.h> > -#include <asm/irq.h> > -#include <asm/mach/sharpsl_param.h> > -#include <asm/mach-types.h> > - > -#include "../../../arch/arm/mach-sa1100/generic.h" > - > -static struct backlight_device *locomolcd_bl_device; > -static struct locomo_dev *locomolcd_dev; > -static unsigned long locomolcd_flags; > -#define LOCOMOLCD_SUSPENDED 0x01 > - > -static void locomolcd_on(int comadj) > -{ > - locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 0); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 1); > - mdelay(2); > - > - locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 0); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 1); > - mdelay(2); > - > - locomo_m62332_senddata(locomolcd_dev, comadj, 0); > - mdelay(5); > - > - locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 0); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 1); > - mdelay(10); > - > - /* TFTCRST | CPSOUT=0 | CPSEN */ > - locomo_writel(0x01, locomolcd_dev->mapbase + LOCOMO_TC); > - > - /* Set CPSD */ > - locomo_writel(6, locomolcd_dev->mapbase + LOCOMO_CPSD); > - > - /* TFTCRST | CPSOUT=0 | CPSEN */ > - locomo_writel((0x04 | 0x01), locomolcd_dev->mapbase + LOCOMO_TC); > - mdelay(10); > - > - locomo_gpio_set_dir(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 0); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 1); > -} > - > -static void locomolcd_off(int comadj) > -{ > - /* TFTCRST=1 | CPSOUT=1 | CPSEN = 0 */ > - locomo_writel(0x06, locomolcd_dev->mapbase + LOCOMO_TC); > - mdelay(1); > - > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHA_ON, 0); > - mdelay(110); > - > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VEE_ON, 0); > - mdelay(700); > - > - /* TFTCRST=0 | CPSOUT=0 | CPSEN = 0 */ > - locomo_writel(0, locomolcd_dev->mapbase + LOCOMO_TC); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_MOD, 0); > - locomo_gpio_write(locomolcd_dev->dev.parent, LOCOMO_GPIO_LCD_VSHD_ON, 0); > -} > - > -void locomolcd_power(int on) > -{ > - int comadj = sharpsl_param.comadj; > - unsigned long flags; > - > - local_irq_save(flags); > - > - if (!locomolcd_dev) { > - local_irq_restore(flags); > - return; > - } > - > - /* read comadj */ > - if (comadj == -1 && machine_is_collie()) > - comadj = 128; > - if (comadj == -1 && machine_is_poodle()) > - comadj = 118; > - > - if (on) > - locomolcd_on(comadj); > - else > - locomolcd_off(comadj); > - > - local_irq_restore(flags); > -} > -EXPORT_SYMBOL(locomolcd_power); > - > -static int current_intensity; > - > -static int locomolcd_set_intensity(struct backlight_device *bd) > -{ > - int intensity = bd->props.brightness; > - > - if (bd->props.power != FB_BLANK_UNBLANK) > - intensity = 0; > - if (bd->props.fb_blank != FB_BLANK_UNBLANK) > - intensity = 0; > - if (locomolcd_flags & LOCOMOLCD_SUSPENDED) > - intensity = 0; > - > - switch (intensity) { > - /* > - * AC and non-AC are handled differently, > - * but produce same results in sharp code? > - */ > - case 0: > - locomo_frontlight_set(locomolcd_dev, 0, 0, 161); > - break; > - case 1: > - locomo_frontlight_set(locomolcd_dev, 117, 0, 161); > - break; > - case 2: > - locomo_frontlight_set(locomolcd_dev, 163, 0, 148); > - break; > - case 3: > - locomo_frontlight_set(locomolcd_dev, 194, 0, 161); > - break; > - case 4: > - locomo_frontlight_set(locomolcd_dev, 194, 1, 161); > - break; > - default: > - return -ENODEV; > - } > - current_intensity = intensity; > - return 0; > -} > - > -static int locomolcd_get_intensity(struct backlight_device *bd) > -{ > - return current_intensity; > -} > - > -static const struct backlight_ops locomobl_data = { > - .get_brightness = locomolcd_get_intensity, > - .update_status = locomolcd_set_intensity, > -}; > - > -#ifdef CONFIG_PM_SLEEP > -static int locomolcd_suspend(struct device *dev) > -{ > - locomolcd_flags |= LOCOMOLCD_SUSPENDED; > - locomolcd_set_intensity(locomolcd_bl_device); > - return 0; > -} > - > -static int locomolcd_resume(struct device *dev) > -{ > - locomolcd_flags &= ~LOCOMOLCD_SUSPENDED; > - locomolcd_set_intensity(locomolcd_bl_device); > - return 0; > -} > -#endif > - > -static SIMPLE_DEV_PM_OPS(locomolcd_pm_ops, locomolcd_suspend, locomolcd_resume); > - > -static int locomolcd_probe(struct locomo_dev *ldev) > -{ > - struct backlight_properties props; > - unsigned long flags; > - > - local_irq_save(flags); > - locomolcd_dev = ldev; > - > - locomo_gpio_set_dir(ldev->dev.parent, LOCOMO_GPIO_FL_VR, 0); > - > - /* > - * the poodle_lcd_power function is called for the first time > - * from fs_initcall, which is before locomo is activated. > - * We need to recall poodle_lcd_power here > - */ > - if (machine_is_poodle()) > - locomolcd_power(1); > - > - local_irq_restore(flags); > - > - memset(&props, 0, sizeof(struct backlight_properties)); > - props.type = BACKLIGHT_RAW; > - props.max_brightness = 4; > - locomolcd_bl_device = backlight_device_register("locomo-bl", > - &ldev->dev, NULL, > - &locomobl_data, &props); > - > - if (IS_ERR(locomolcd_bl_device)) > - return PTR_ERR(locomolcd_bl_device); > - > - /* Set up frontlight so that screen is readable */ > - locomolcd_bl_device->props.brightness = 2; > - locomolcd_set_intensity(locomolcd_bl_device); > - > - return 0; > -} > - > -static int locomolcd_remove(struct locomo_dev *dev) > -{ > - unsigned long flags; > - > - locomolcd_bl_device->props.brightness = 0; > - locomolcd_bl_device->props.power = 0; > - locomolcd_set_intensity(locomolcd_bl_device); > - > - backlight_device_unregister(locomolcd_bl_device); > - local_irq_save(flags); > - locomolcd_dev = NULL; > - local_irq_restore(flags); > - return 0; > -} > - > -static struct locomo_driver poodle_lcd_driver = { > - .drv = { > - .name = "locomo-backlight", > - .pm = &locomolcd_pm_ops, > - }, > - .devid = LOCOMO_DEVID_BACKLIGHT, > - .probe = locomolcd_probe, > - .remove = locomolcd_remove, > -}; > - > -static int __init locomolcd_init(void) > -{ > - return locomo_driver_register(&poodle_lcd_driver); > -} > - > -static void __exit locomolcd_exit(void) > -{ > - locomo_driver_unregister(&poodle_lcd_driver); > -} > - > -module_init(locomolcd_init); > -module_exit(locomolcd_exit); > - > -MODULE_AUTHOR("John Lenz <lenz@cs.wisc.edu>, Pavel Machek <pavel@ucw.cz>"); > -MODULE_DESCRIPTION("Collie LCD driver"); > -MODULE_LICENSE("GPL");
On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: > Adapt locomo leds driver to new locomo core setup. > > Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> This is missing Bryan's Ack. > --- > 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");
2015-06-09 9:57 GMT+03:00 Lee Jones <lee.jones@linaro.org>: > On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: > >> Adapt locomo leds driver to new locomo core setup. >> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> >> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> > > This is missing Bryan's Ack. Jacek is listed as a co-maintainer of the LEDS subsystem, so I assumed that his acked-by is enough. > >> --- >> 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"); > > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog
On Tue, 09 Jun 2015, Dmitry Eremin-Solenikov wrote: > 2015-06-09 9:57 GMT+03:00 Lee Jones <lee.jones@linaro.org>: > > On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: > > > >> Adapt locomo leds driver to new locomo core setup. > >> > >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > >> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> > > > > This is missing Bryan's Ack. > > Jacek is listed as a co-maintainer of the LEDS subsystem, so I assumed > that his acked-by is enough. Ah, this is new. Congratulations Jacek. Ignore me then. > >> --- > >> 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"); > > > > >
On 06/09/2015 01:11 PM, Lee Jones wrote: > On Tue, 09 Jun 2015, Dmitry Eremin-Solenikov wrote: > >> 2015-06-09 9:57 GMT+03:00 Lee Jones <lee.jones@linaro.org>: >>> On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: >>> >>>> Adapt locomo leds driver to new locomo core setup. >>>> >>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> >>>> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> >>> >>> This is missing Bryan's Ack. >> >> Jacek is listed as a co-maintainer of the LEDS subsystem, so I assumed >> that his acked-by is enough. > > Ah, this is new. > > Congratulations Jacek. Thanks. > Ignore me then. I think that it should go via mfd tree, as this patch adds dependency on header linux/mfd/locomo.h, which is added in one of the preceding patches [1] in this series. [1] [PATCH v3 01/17] mfd: add new driver for Sharp LoCoMo >>>> --- >>>> 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"); >>> >> >> >> >
On Tue, 09 Jun 2015, Jacek Anaszewski wrote: > On 06/09/2015 01:11 PM, Lee Jones wrote: > >On Tue, 09 Jun 2015, Dmitry Eremin-Solenikov wrote: > > > >>2015-06-09 9:57 GMT+03:00 Lee Jones <lee.jones@linaro.org>: > >>>On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: > >>> > >>>>Adapt locomo leds driver to new locomo core setup. > >>>> > >>>>Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> > >>>>Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> > >>> > >>>This is missing Bryan's Ack. > >> > >>Jacek is listed as a co-maintainer of the LEDS subsystem, so I assumed > >>that his acked-by is enough. > > > >Ah, this is new. > > > >Congratulations Jacek. > > Thanks. > > >Ignore me then. > > I think that it should go via mfd tree, as this patch adds > dependency on header linux/mfd/locomo.h, which is added > in one of the preceding patches [1] in this series. Yep, that's the plan. > [1] [PATCH v3 01/17] mfd: add new driver for Sharp LoCoMo > > >>>>--- > >>>> 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"); > >>> > >> > >> > >> > > > >
Hello, 2015-06-08 23:56 GMT+03:00 Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>: > Hello, > > Russell, I understand that you are quite busy. Could you please find > a small amount of time to review and comment/ack the sa1100/collie-related > parts of the patchset. It looks like this serie misses only your blessing. Russell? Can we please have any feedback? > > Lee have agreed to merge this through the MFD tree, and the respective > subsystem maintainers are also fine with this. > > Changes since V4: > * Added all Acked-by lines. > * Dropped the SPI patch - no reaction from Mark Brown onto the latest > version of the patch. It can go separately, as it adds new functionality. > > 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-v5 > > for you to fetch changes up to 736e2f3b9c71c8baa0a260fcabe9a979e9bd8d5c: > > ARM: drop old LoCoMo driver (2015-06-08 23:36:10 +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: locomokbd: differentiate between two Enter keys > 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 > 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 | 262 ++++----- > drivers/leds/Kconfig | 2 +- > drivers/leds/leds-locomo.c | 117 ++-- > drivers/mfd/Kconfig | 10 + > drivers/mfd/Makefile | 1 + > drivers/mfd/locomo.c | 336 +++++++++++ > 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 | 169 ++++++ > sound/soc/pxa/poodle.c | 52 +- > 31 files changed, 1862 insertions(+), 1703 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/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 >
On Mon, Jun 08, 2015 at 11:56:42PM +0300, Dmitry Eremin-Solenikov wrote: > +static struct gpio collie_uart_gpio[] = { > + { COLLIE_GPIO_CTS, GPIOF_IN, "CTS" }, > + { COLLIE_GPIO_RTS, GPIOF_OUT_INIT_LOW, "RTS" }, > + { COLLIE_GPIO_DTR, GPIOF_OUT_INIT_LOW, "DTR" }, > + { COLLIE_GPIO_DSR, GPIOF_IN, "DSR" }, > +}; These should probably be given a better name rather than just "CTS" - maybe something that gives a clue as to what UART they refer to? > static void collie_uart_set_mctrl(struct uart_port *port, u_int mctrl) > { > - if (mctrl & TIOCM_RTS) > - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_RTS, 0); > - else > - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_RTS, 1); > - > - if (mctrl & TIOCM_DTR) > - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_DTR, 0); > - else > - locomo_gpio_write(&collie_locomo_device.dev, LOCOMO_GPIO_DTR, 1); > + if (!collie_uart_gpio_ok) { > + int rc = gpio_request_array(collie_uart_gpio, > + ARRAY_SIZE(collie_uart_gpio)); > + if (rc) > + pr_err("collie_uart_set_mctrl: gpio request %d\n", rc); > + else > + collie_uart_gpio_ok = true; > + } > + > + if (collie_uart_gpio_ok) { This seems to be a repeated chunk of code. Maybe: static bool collie_mctrl_present(void) { static bool collie_uart_mctrl_claimed; if (!collie_uart_mctrl_claimed) { int rc = gpio_request_array(collie_uart_gpio, ARRAY_SIZE(collie_uart_gpio)); if (rc) pr_err("%s: gpio_request_array() failed: %d\n", __func__, rc); else collie_uart_mctrl_claimed = true; } return collie_uart_mctrl_claimed; } static void collie_uart_set_mctrl(struct uart_port *port, u_int mctrl) { if (collie_mctrl_present()) { > + gpio_set_value(COLLIE_GPIO_RTS, !(mctrl & TIOCM_RTS)); > + gpio_set_value(COLLIE_GPIO_DTR, !(mctrl & TIOCM_DTR)); > + } > } > > static u_int collie_uart_get_mctrl(struct uart_port *port) > { > int ret = TIOCM_CD; > - unsigned int r; > > - r = locomo_gpio_read_output(&collie_locomo_device.dev, LOCOMO_GPIO_CTS & LOCOMO_GPIO_DSR); > - if (r == -ENODEV) > + if (!collie_uart_gpio_ok) { > + int rc = gpio_request_array(collie_uart_gpio, > + ARRAY_SIZE(collie_uart_gpio)); > + if (rc) > + pr_err("collie_uart_get_mctrl: gpio request %d\n", rc); > + else > + collie_uart_gpio_ok = true; > + } > + > + if (!collie_uart_gpio_ok) > return ret; > - if (r & LOCOMO_GPIO_CTS) > + > + if (gpio_get_value(COLLIE_GPIO_CTS)) > ret |= TIOCM_CTS; > - if (r & LOCOMO_GPIO_DSR) > + if (gpio_get_value(COLLIE_GPIO_DSR)) > ret |= TIOCM_DSR; > > return ret; And this would become: int TIOCM_CD; if (collie_mctrl_present()) { if (gpio_get_value(COLLIE_GPIO_CTS)) ret |= TIOCM_CTS; if (gpio_get_value(COLLIE_GPIO_DSR)) ret |= TIOCM_DSR; } return ret; which kind'a looks neater, and avoids duplicating the GPIO claiming. > @@ -191,33 +216,35 @@ static struct sa1100_port_fns collie_port_fns __initdata = { > .get_mctrl = collie_uart_get_mctrl, > }; > > -static int collie_uart_probe(struct locomo_dev *dev) > -{ > - return 0; > -} > - > -static int collie_uart_remove(struct locomo_dev *dev) > -{ > - return 0; > -} > +static struct regulator_consumer_supply collie_amp_on_consumer_supplies[] = { > + REGULATOR_SUPPLY("VCC", "1-004e"), > +}; > > -static struct locomo_driver collie_uart_driver = { > - .drv = { > - .name = "collie_uart", > +static struct regulator_init_data collie_amp_on_init_data = { > + .constraints = { > + .name = "AMP_ON", > + .valid_ops_mask = REGULATOR_CHANGE_STATUS, > }, > - .devid = LOCOMO_DEVID_UART, > - .probe = collie_uart_probe, > - .remove = collie_uart_remove, > + .consumer_supplies = collie_amp_on_consumer_supplies, > + .num_consumer_supplies = ARRAY_SIZE(collie_amp_on_consumer_supplies), > }; > > -static int __init collie_uart_init(void) > -{ > - return locomo_driver_register(&collie_uart_driver); > -} > -device_initcall(collie_uart_init); > - > -#endif > +static struct fixed_voltage_config collie_amp_on_data = { > + .supply_name = "amp_on", > + .microvolts = 3300000, > + .gpio = COLLIE_GPIO_AMP2_ON, > + .startup_delay = 5, > + .enable_high = 1, > + .init_data = &collie_amp_on_init_data, > +}; > > +static struct platform_device collie_amp_on_device = { > + .name = "reg-fixed-voltage", > + .id = -1, > + .dev = { > + .platform_data = &collie_amp_on_data, > + }, > +}; > > static struct resource locomo_resources[] = { > [0] = DEFINE_RES_MEM(0x40000000, SZ_8K), > @@ -225,14 +252,15 @@ static struct resource locomo_resources[] = { > }; > > static struct locomo_platform_data locomo_info = { > - .irq_base = IRQ_BOARD_START, > + .gpio_base = COLLIE_LOCOMO_GPIO_BASE, > + .comadj = 128, Using spaces for what looks like pointless indentation. Please either align the = signs using tabs, or don't bother at all. As the rest of the file's style is to align the = signs using tabs, please remain consistent with the rest of the file unless you are intending to reformat it - in which case, the reformatting should happen as the very first patch. > }; > > -struct platform_device collie_locomo_device = { > +static struct platform_device collie_locomo_device = { > .name = "locomo", > .id = 0, > .dev = { > - .platform_data = &locomo_info, > + .platform_data = &locomo_info, You seem to replace a tab with two spaces here... > }, > .num_resources = ARRAY_SIZE(locomo_resources), > .resource = locomo_resources, > @@ -270,7 +298,55 @@ static struct platform_device collie_gpio_keys_device = { > }, > }; > > +static int collie_mmc_init(struct device *dev, > + irqreturn_t (*isr)(int, void*), void *mmc) > +{ > + int ret; > + > + ret = gpio_request(COLLIE_GPIO_CARD_POWER, "MMC power"); > + if (!ret) > + ret = gpio_direction_output(COLLIE_GPIO_CARD_POWER, 0); > + if (ret) > + gpio_free(COLLIE_GPIO_CARD_POWER); > + return ret; Why not use gpio_request_one() here? This whole function could be collapsed to just: return gpio_request_one(COLLIE_GPIO_CARD_POWER, GPIOF_OUT_INIT_LOW, "MMC power"); > +} > + > +static void collie_mmc_exit(struct device *dev, void *mmc) > +{ > + gpio_free(COLLIE_GPIO_CARD_POWER); > +} > + > +static void collie_mmc_setpower(struct device *dev, unsigned int mask) > +{ > + gpio_set_value(COLLIE_GPIO_CARD_POWER, !!mask); > +} > + > +static struct mmc_spi_platform_data collie_mmc_data = { > + .init = collie_mmc_init, > + .exit = collie_mmc_exit, > + .setpower = collie_mmc_setpower, > + .detect_delay = 200, > + .powerup_msecs = 200, > + .ocr_mask = MMC_VDD_32_33 | MMC_VDD_33_34, > + .flags = MMC_SPI_USE_CD_GPIO | MMC_SPI_USE_RO_GPIO, > + .cd_gpio = COLLIE_GPIO_CARD_DETECT, > + .ro_gpio = COLLIE_GPIO_CARD_RO, > + .caps2 = MMC_CAP2_RO_ACTIVE_HIGH, > +}; > + > +static struct spi_board_info collie_spi_board_info[] __initdata = { > + { > + .modalias = "mmc_spi", > + .platform_data = &collie_mmc_data, > + .max_speed_hz = 25000000, > + .bus_num = 0, > + .chip_select = 0, > + .mode = SPI_MODE_0, > + }, > +}; > + > static struct platform_device *devices[] __initdata = { > + &collie_amp_on_device, > &collie_locomo_device, > &colliescoop_device, > &collie_power_device, > @@ -347,10 +423,39 @@ static struct sa1100fb_mach_info collie_lcd_info = { > > .lccr0 = LCCR0_Color | LCCR0_Sngl | LCCR0_Act, > .lccr3 = LCCR3_OutEnH | LCCR3_PixRsEdg | LCCR3_ACBsDiv(2), > +}; > > -#ifdef CONFIG_BACKLIGHT_LOCOMO > - .lcd_power = locomolcd_power > -#endif > +static struct iio_map locomo_iio_map[] = { > + { > + .consumer_dev_name = "locomo-lcd.0", > + .consumer_channel = "comadj", > + .adc_channel_label = "CH0", > + }, > + { } > +}; > + > +static struct i2c_board_info locomo_i2c_devs[] __initdata = { > + { > + I2C_BOARD_INFO("m62332", 0x4e), > + .platform_data = locomo_iio_map, > + }, > +}; > + > +static struct gpiod_lookup_table collie_bl_gpios_table = { > + .dev_id = "locomo-backlight.0", > + .table = { > + GPIO_LOOKUP("locomo-gpio", 9, "flvr", GPIO_ACTIVE_HIGH), > + { }, > + }, > +}, collie_lcd_gpios_table = { Please don't do this. Please phrase this instead as: }; static struct gpiod_lookup_table collie_lcd_gpios_table = { > + .dev_id = "locomo-lcd.0", > + .table = { > + GPIO_LOOKUP("locomo-gpio", 4, "VSHA", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 5, "VSHD", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 6, "Vee", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 7, "MOD", GPIO_ACTIVE_HIGH), > + { }, > + }, > }; ...
On Mon, Jun 08, 2015 at 11:56:45PM +0300, Dmitry Eremin-Solenikov wrote: > @@ -179,7 +181,8 @@ static struct resource locomo_resources[] = { > }; > > static struct locomo_platform_data locomo_info = { > - .irq_base = IRQ_BOARD_START, > + .gpio_base = -1, > + .comadj = 118, More space vs tabs madness. Please keep to the original file's style when editing, it's important. > }; > > struct platform_device poodle_locomo_device = { > @@ -192,8 +195,6 @@ struct platform_device poodle_locomo_device = { > }, > }; > > -EXPORT_SYMBOL(poodle_locomo_device); > - > #if defined(CONFIG_SPI_PXA2XX) || defined(CONFIG_SPI_PXA2XX_MODULE) > static struct pxa2xx_spi_master poodle_spi_info = { > .num_chipselect = 1, > @@ -424,6 +425,47 @@ static struct i2c_board_info __initdata poodle_i2c_devices[] = { > { I2C_BOARD_INFO("wm8731", 0x1b) }, > }; > > +static struct iio_map locomo_iio_map[] = { > + { > + .consumer_dev_name = "locomo-lcd.0", > + .consumer_channel = "comadj", > + .adc_channel_label = "CH0", > + }, > + { } > +}; > + > +static struct i2c_board_info locomo_i2c_devs[] __initdata = { > + { > + I2C_BOARD_INFO("m62332", 0x4e), > + .platform_data = locomo_iio_map, > + }, > +}; > + > +static struct gpiod_lookup_table poodle_audio_gpios_table = { > + .dev_id = "poodle-audio", > + .table = { > + GPIO_LOOKUP("locomo-gpio", 10, "mute-l", GPIO_ACTIVE_LOW), > + GPIO_LOOKUP("locomo-gpio", 11, "mute-r", GPIO_ACTIVE_LOW), > + GPIO_LOOKUP("locomo-gpio", 8, "amp-on", GPIO_ACTIVE_LOW), > + { }, > + }, > +}, poodle_bl_gpios_table = { Same comment as previous patch... > + .dev_id = "locomo-backlight.0", > + .table = { > + GPIO_LOOKUP("locomo-gpio", 9, "flvr", GPIO_ACTIVE_HIGH), > + { }, > + }, > +}, poodle_lcd_gpios_table = { Ditto... > + .dev_id = "locomo-lcd.0", > + .table = { > + GPIO_LOOKUP("locomo-gpio", 4, "VSHA", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 5, "VSHD", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 6, "Vee", GPIO_ACTIVE_HIGH), > + GPIO_LOOKUP("locomo-gpio", 7, "MOD", GPIO_ACTIVE_HIGH), > + { }, > + }, > +}; > + > static void poodle_poweroff(void) > { > pxa_restart(REBOOT_HARD, NULL); > @@ -445,6 +487,13 @@ static void __init poodle_init(void) > > platform_scoop_config = &poodle_pcmcia_config; > > + if (sharpsl_param.comadj != -1) > + locomo_info.comadj = sharpsl_param.comadj; > + > + gpiod_add_lookup_table(&poodle_audio_gpios_table); > + gpiod_add_lookup_table(&poodle_bl_gpios_table); > + gpiod_add_lookup_table(&poodle_lcd_gpios_table); > + > ret = platform_add_devices(devices, ARRAY_SIZE(devices)); > if (ret) > pr_warn("poodle: Unable to register LoCoMo device\n"); > @@ -455,6 +504,7 @@ static void __init poodle_init(void) > pxa_set_ficp_info(&poodle_ficp_platform_data); > pxa_set_i2c_info(NULL); > i2c_register_board_info(0, ARRAY_AND_SIZE(poodle_i2c_devices)); > + i2c_register_board_info(1, ARRAY_AND_SIZE(locomo_i2c_devs)); > poodle_init_spi(); > regulator_has_full_constraints(); > } > -- > 2.1.4 >
On Mon, Jun 08, 2015 at 11:56:36PM +0300, Dmitry Eremin-Solenikov wrote: > @@ -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; We had decided that we weren't allowing any new machine_is_xxx() in drivers. Why can't this difference be encoded via platform data, so it can be later encoded in DT if sa11x0 moves in that direction?
On Mon, Jun 08, 2015 at 11:56:37PM +0300, Dmitry Eremin-Solenikov wrote: > Add new simple backlight driver - it cares only about PWM/frontlight > part of LoCoMo, it does not touch TFT settings and does not export TFT > power control. Is there a reason not to model the PWM as a PWM device, and use pwm_bl.c ? pwm_bl.c can be used in non-DT setups too.
On Mon, Jun 08, 2015 at 11:56:38PM +0300, Dmitry Eremin-Solenikov wrote: > +int locomo_lcd_set_power(struct lcd_device *ldev, int power) > +{ > + struct locomo_lcd *lcd = lcd_get_data(ldev); > + > + dev_dbg(&ldev->dev, "LCD power %d (is %d)\n", power, lcd->power); > + > + if (!power && lcd->power) > + locomo_lcd_on(lcd); > + > + if (power && !lcd->power) > + locomo_lcd_off(lcd); Is this correct? You want to turn the LCD _off_ if power is non-zero, but turn it _on_ if power is _zero_ ? Just wondering if this would be clearer: if (!power != !lcd->power) { /* Power state is different */ if (!power) locomo_lcd_on(lcd); else locomo_lcd_off(lcd); lcd->power = power; } At least less prone to getting the power state wrong... > +static int locomo_lcd_remove(struct platform_device *dev) > +{ > + struct lcd_device *ldev = platform_get_drvdata(dev); > + struct locomo_lcd *lcd = lcd_get_data(ldev); > + > + locomo_lcd_off(lcd); You're careful above not to turn the LCD off if it's already off, but here that doesn't matter? What if the LCD is already off? Does this need protection against double-off? > + > + locomo_lcd_disable_adsync(lcd); > + > + iio_channel_release(lcd->comadj); > + > + return 0; > +} > + > +static void locomo_lcd_shutdown(struct platform_device *dev) > +{ > + struct lcd_device *ldev = platform_get_drvdata(dev); > + struct locomo_lcd *lcd = lcd_get_data(ldev); > + > + locomo_lcd_off(lcd); Ditto.
On Mon, Jun 08, 2015 at 11:56:39PM +0300, Dmitry Eremin-Solenikov wrote: > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index f71bb97..98655ae 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -42,6 +42,7 @@ obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o > obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o > obj-$(CONFIG_ARCH_KS8695) += gpio-ks8695.o > obj-$(CONFIG_GPIO_INTEL_MID) += gpio-intel-mid.o > +obj-$(CONFIG_GPIO_LOCOMO) += gpio-locomo.o > obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o > obj-$(CONFIG_GPIO_LP3943) += gpio-lp3943.o > obj-$(CONFIG_ARCH_LPC32XX) += gpio-lpc32xx.o > diff --git a/drivers/gpio/gpio-locomo.c b/drivers/gpio/gpio-locomo.c > new file mode 100644 > index 0000000..dd9a1ca > --- /dev/null > +++ b/drivers/gpio/gpio-locomo.c > @@ -0,0 +1,170 @@ > +/* > + * Sharp LoCoMo support for GPIO > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This file contains all generic LoCoMo support. > + * > + * All initialization functions provided here are intended to be called > + * from machine specific code with proper arguments when required. > + */ > +#include <linux/module.h> > +#include <linux/kernel.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/bitops.h> > +#include <linux/err.h> > +#include <linux/gpio.h> > +#include <linux/io.h> > +#include <linux/regmap.h> > +#include <linux/mfd/locomo.h> > + > +struct locomo_gpio { > + struct regmap *regmap; > + > + struct gpio_chip gpio; > + > + u16 rising_edge; > + u16 falling_edge; > + > + unsigned int save_gpo; > + unsigned int save_gpe; > +}; > + > +static int locomo_gpio_get(struct gpio_chip *chip, > + unsigned offset) > +{ > + struct locomo_gpio *lg = container_of(chip, struct locomo_gpio, gpio); > + unsigned int gpl; > + > + regmap_read(lg->regmap, LOCOMO_GPL, &gpl); > + > + return gpl & BIT(offset); Aren't gpio_get() functions supposed to return a 0/1 status, not a 0/BIT(n) status? > +#ifdef CONFIG_PM_SLEEP > +static int locomo_gpio_suspend(struct device *dev) > +{ > + struct locomo_gpio *lg = dev_get_drvdata(dev); > + > + regmap_read(lg->regmap, LOCOMO_GPO, &lg->save_gpo); > + regmap_write(lg->regmap, LOCOMO_GPO, 0x00); > + regmap_read(lg->regmap, LOCOMO_GPE, &lg->save_gpe); > + regmap_write(lg->regmap, LOCOMO_GPE, 0x00); Are you sure this is correct? If there are any output signals which are pulled up, this will make them glitch as you seem to first write zero to the output register (which will pull anything that is configured as an output low), and then program all lines as inputs. I haven't checked what the existing code does though... > +static int locomo_gpio_probe(struct platform_device *pdev) > +{ > + struct locomo_gpio *lg; > + int ret; > + struct locomo_gpio_platform_data *pdata = dev_get_platdata(&pdev->dev); > + > + lg = devm_kzalloc(&pdev->dev, sizeof(struct locomo_gpio), > + GFP_KERNEL); > + if (!lg) > + return -ENOMEM; > + > + lg->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!lg->regmap) > + return -EINVAL; > + > + platform_set_drvdata(pdev, lg); > + > + regmap_write(lg->regmap, LOCOMO_GPO, 0x00); > + regmap_write(lg->regmap, LOCOMO_GPE, 0x00); > + regmap_write(lg->regmap, LOCOMO_GPD, 0x00); > + regmap_write(lg->regmap, LOCOMO_GIE, 0x00); Do we really want to initialise all outputs like this? It suffers from the problem I mention above - it sets all outputs to zero, then sets them as inputs, which can lead to glitching. What if (eg) the LCD was left enabled? This would mean you're not going through the proper power-down sequence for the LCD (for example). TBH, I think GPIO needs to have a way to claim output GPIOs _without_ programming their current state for situations like this. That seems to have been an oversight in the model for a while now - it's impossible to claim GPIO outputs without setting their initial value, and it may be important in case you don't know whether the attached LCD is enabled or disabled, and the attached LCD may require a specific sequence of power removal manipulated by GPIOs. I think this is something for the GPIO people to think about rather than you though...
Hello, 2015-06-14 18:13 GMT+03:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Mon, Jun 08, 2015 at 11:56:37PM +0300, Dmitry Eremin-Solenikov wrote: >> Add new simple backlight driver - it cares only about PWM/frontlight >> part of LoCoMo, it does not touch TFT settings and does not export TFT >> power control. > > Is there a reason not to model the PWM as a PWM device, and use pwm_bl.c ? > pwm_bl.c can be used in non-DT setups too. I thought about it too. The problem is the FLVR pin, which adds another (extra) brighness level after the PWM is configured to 100% duty cycle. Whether to use PWM subsystem or not for handling this PWM is an open question. I decided not to use it (at least for now), because that would bring complexity with no directly added value.
Hello, 2015-06-14 18:11 GMT+03:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Mon, Jun 08, 2015 at 11:56:36PM +0300, Dmitry Eremin-Solenikov wrote: >> @@ -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; > > We had decided that we weren't allowing any new machine_is_xxx() in drivers. > Why can't this difference be encoded via platform data, so it can be later > encoded in DT if sa11x0 moves in that direction? I'd agree with you. However this looks rather long path: * Introduce platform data with just one preference (for home key). * Switch poodle(pxa) to dts * Switch collie(sa1100) to dts * Introduce special property (or use compatibility string) * Kill platform data. By allowing machine_is_xx() here we can forget about this temporary platform data. The change is quite isolated. If you completely disagree with this patch, I'd prefer to just drop it for now (as it is an added feature rather than just refactoring).
2015-06-14 18:18 GMT+03:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Mon, Jun 08, 2015 at 11:56:38PM +0300, Dmitry Eremin-Solenikov wrote: >> +int locomo_lcd_set_power(struct lcd_device *ldev, int power) >> +{ >> + struct locomo_lcd *lcd = lcd_get_data(ldev); >> + >> + dev_dbg(&ldev->dev, "LCD power %d (is %d)\n", power, lcd->power); >> + >> + if (!power && lcd->power) >> + locomo_lcd_on(lcd); >> + >> + if (power && !lcd->power) >> + locomo_lcd_off(lcd); > > Is this correct? You want to turn the LCD _off_ if power is non-zero, but > turn it _on_ if power is _zero_ ? Yes, this is a crazy part of fb/backlight/LCD, where power is a FB_BLANK_* value, and FB_BLANK_UNBLANK is equal to 0.
On Sun, Jun 14, 2015 at 07:28:50PM +0300, Dmitry Eremin-Solenikov wrote: > 2015-06-14 18:18 GMT+03:00 Russell King - ARM Linux <linux@arm.linux.org.uk>: > > On Mon, Jun 08, 2015 at 11:56:38PM +0300, Dmitry Eremin-Solenikov wrote: > >> +int locomo_lcd_set_power(struct lcd_device *ldev, int power) > >> +{ > >> + struct locomo_lcd *lcd = lcd_get_data(ldev); > >> + > >> + dev_dbg(&ldev->dev, "LCD power %d (is %d)\n", power, lcd->power); > >> + > >> + if (!power && lcd->power) > >> + locomo_lcd_on(lcd); > >> + > >> + if (power && !lcd->power) > >> + locomo_lcd_off(lcd); > > > > Is this correct? You want to turn the LCD _off_ if power is non-zero, but > > turn it _on_ if power is _zero_ ? > > Yes, this is a crazy part of fb/backlight/LCD, where power is a FB_BLANK_* > value, and FB_BLANK_UNBLANK is equal to 0. In which case, the code is probably wrong anyway. This would make it much clearer: bool power_on = power == FB_BLANK_UNBLANK; if (power_on != lcd->power_is_on) { if (power_on) locomo_lcd_on(lcd); else locomo_lcd_off(lcd); lcd->power_is_on = power_on; } with lcd->power_is_on also being a bool. The above is almost English... if the power state is different from last time, then if we want power on call locomo_lcd_on otherwise call locomo_lcd_off remember power state
On Tue, Jun 9, 2015 at 1:08 AM, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote: > 2015-06-09 9:57 GMT+03:00 Lee Jones <lee.jones@linaro.org>: >> On Mon, 08 Jun 2015, Dmitry Eremin-Solenikov wrote: >> >>> Adapt locomo leds driver to new locomo core setup. >>> >>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> >>> Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com> >> >> This is missing Bryan's Ack. > > Jacek is listed as a co-maintainer of the LEDS subsystem, so I assumed > that his acked-by is enough. > Yeah, Jacke's Acked-by is good enough. Thanks, -Bryan >> >>> --- >>> 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"); >> >> -- >> Lee Jones >> Linaro STMicroelectronics Landing Team Lead >> Linaro.org │ Open source software for ARM SoCs >> Follow Linaro: Facebook | Twitter | Blog > > > > -- > With best wishes > Dmitry
On Sun, Jun 14, 2015 at 5:27 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > [Dmitry] >> + regmap_write(lg->regmap, LOCOMO_GPO, 0x00); >> + regmap_write(lg->regmap, LOCOMO_GPE, 0x00); >> + regmap_write(lg->regmap, LOCOMO_GPD, 0x00); >> + regmap_write(lg->regmap, LOCOMO_GIE, 0x00); > > Do we really want to initialise all outputs like this? It suffers from > the problem I mention above - it sets all outputs to zero, then sets > them as inputs, which can lead to glitching. What if (eg) the LCD was > left enabled? This would mean you're not going through the proper > power-down sequence for the LCD (for example). This looks dangerous indeed, why did I miss that before. Is it possible to just leave these registers as-is, or, in case you want to know at all times their actual states, implement the .get_direction() callback? They the kernel can explore the direction and value of the lines. > TBH, I think GPIO needs to have a way to claim output GPIOs _without_ > programming their current state for situations like this. That seems > to have been an oversight in the model for a while now - it's impossible > to claim GPIO outputs without setting their initial value, It is possible in the new descriptor API, you can claim a GPIO for a device with (see <linux/gpio/consumer.h> enum gpiod_flags { GPIOD_ASIS = 0, (...) I understand it may be a bit thick to ask to migrate the whole SA1100 codebase to GPIO descriptors though, I don't know how many machines and drivers that would affect. I'd consider a patch retrofitting this into the old API if it would see some real use case. Yours, Linus Walleij
Hello, Russell, I understand that you are quite busy. Could you please find a small amount of time to review and comment/ack the sa1100/collie-related parts of the patchset. It looks like this serie misses only your blessing. Lee have agreed to merge this through the MFD tree, and the respective subsystem maintainers are also fine with this. Changes since V4: * Added all Acked-by lines. * Dropped the SPI patch - no reaction from Mark Brown onto the latest version of the patch. It can go separately, as it adds new functionality. 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-v5 for you to fetch changes up to 736e2f3b9c71c8baa0a260fcabe9a979e9bd8d5c: ARM: drop old LoCoMo driver (2015-06-08 23:36:10 +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: locomokbd: differentiate between two Enter keys 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 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 | 262 ++++----- drivers/leds/Kconfig | 2 +- drivers/leds/leds-locomo.c | 117 ++-- drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile | 1 + drivers/mfd/locomo.c | 336 +++++++++++ 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 | 169 ++++++ sound/soc/pxa/poodle.c | 52 +- 31 files changed, 1862 insertions(+), 1703 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/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