| Message ID | 20240906093630.2428329-8-bigfoot@classfun.cn (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Introduce Photonicat power management MCU driver | expand |
On Fri, 06 Sep 2024, Junhao Xie wrote: > Photonicat has a network status LED that can be controlled by system. > The LED status can be set through command 0x19. > > Signed-off-by: Junhao Xie <bigfoot@classfun.cn> > --- > drivers/leds/Kconfig | 11 +++++ > drivers/leds/Makefile | 1 + > drivers/leds/leds-photonicat.c | 75 ++++++++++++++++++++++++++++++++++ > 3 files changed, 87 insertions(+) > create mode 100644 drivers/leds/leds-photonicat.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 8d9d8da376e4..539adb5944e6 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -381,6 +381,17 @@ config LEDS_PCA9532_GPIO > To use a pin as gpio pca9532_type in pca9532_platform data needs to > set to PCA9532_TYPE_GPIO. > > +config LEDS_PHOTONICAT_PMU > + tristate "LED Support for Photonicat PMU" > + depends on LEDS_CLASS > + depends on MFD_PHOTONICAT_PMU > + help > + Photonicat has a network status LED that can be controlled by system, "the system" > + this option enables support for LEDs connected to the Photonicat PMU. > + > + To compile this driver as a module, choose M here: the > + module will be called leds-photonicat. > + > config LEDS_GPIO > tristate "LED Support for GPIO connected LEDs" > depends on LEDS_CLASS > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 18afbb5a23ee..dcd5312aee12 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_PCA9532) += leds-pca9532.o > obj-$(CONFIG_LEDS_PCA955X) += leds-pca955x.o > obj-$(CONFIG_LEDS_PCA963X) += leds-pca963x.o > obj-$(CONFIG_LEDS_PCA995X) += leds-pca995x.o > +obj-$(CONFIG_LEDS_PHOTONICAT_PMU) += leds-photonicat.o > obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o > obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o > obj-$(CONFIG_LEDS_PWM) += leds-pwm.o > diff --git a/drivers/leds/leds-photonicat.c b/drivers/leds/leds-photonicat.c > new file mode 100644 > index 000000000000..3aa5ce525b83 > --- /dev/null > +++ b/drivers/leds/leds-photonicat.c > @@ -0,0 +1,75 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2024 Junhao Xie <bigfoot@classfun.cn> > + */ > + > +#include <linux/mfd/photonicat-pmu.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/leds.h> Alphabetical. > +struct pcat_leds { > + struct device *dev; Where is this used? > + struct pcat_pmu *pmu; Why do you need to store this? Can't you get this at the call-site by: dev_get_drvdata(cdev->dev->parent) > + struct led_classdev cdev; > +}; > + > +static int pcat_led_status_set(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct pcat_leds *leds = container_of(cdev, struct pcat_leds, cdev); > + struct pcat_data_cmd_led_setup setup = { 0, 0, 0 }; > + > + if (brightness) > + setup.on_time = 100; > + else > + setup.down_time = 100; > + return pcat_pmu_write_data(leds->pmu, PCAT_CMD_NET_STATUS_LED_SETUP, > + &setup, sizeof(setup)); > +} > + > +static int pcat_leds_probe(struct platform_device *pdev) > +{ > + int ret; Small sized variables at the bottom please. > + struct device *dev = &pdev->dev; > + struct pcat_leds *leds; > + const char *label; > + > + leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL); > + if (!leds) > + return -ENOMEM; > + > + leds->dev = dev; Where is this used? > + leds->pmu = dev_get_drvdata(dev->parent); > + platform_set_drvdata(pdev, leds); Where do you platform_get_drvdata() > + ret = of_property_read_string(dev->of_node, "label", &label); > + if (ret) > + return dev_err_probe(dev, ret, "No label property\n"); > + > + leds->cdev.name = label; > + leds->cdev.max_brightness = 1; > + leds->cdev.brightness_set_blocking = pcat_led_status_set; > + > + return devm_led_classdev_register(dev, &leds->cdev); > +} > + > +static const struct of_device_id pcat_leds_dt_ids[] = { > + { .compatible = "ariaboard,photonicat-pmu-leds", }, How many LEDs are there? > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, pcat_leds_dt_ids); > + > +static struct platform_driver pcat_leds_driver = { > + .driver = { > + .name = "photonicat-leds", > + .of_match_table = pcat_leds_dt_ids, > + }, > + .probe = pcat_leds_probe, > +}; > +module_platform_driver(pcat_leds_driver); > + > +MODULE_AUTHOR("Junhao Xie <bigfoot@classfun.cn>"); > +MODULE_DESCRIPTION("Photonicat PMU Status LEDs"); > +MODULE_LICENSE("GPL"); > -- > 2.46.0 >
On 2024/10/2 23:35, Lee Jones wrote: > On Fri, 06 Sep 2024, Junhao Xie wrote: > >> Photonicat has a network status LED that can be controlled by system. >> The LED status can be set through command 0x19. [...] >> +config LEDS_PHOTONICAT_PMU >> + tristate "LED Support for Photonicat PMU" >> + depends on LEDS_CLASS >> + depends on MFD_PHOTONICAT_PMU >> + help >> + Photonicat has a network status LED that can be controlled by system, > > "the system" > >> + this option enables support for LEDs connected to the Photonicat PMU. [...] >> +++ b/drivers/leds/leds-photonicat.c >> @@ -0,0 +1,75 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2024 Junhao Xie <bigfoot@classfun.cn> >> + */ >> + >> +#include <linux/mfd/photonicat-pmu.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/leds.h> > > Alphabetical. > >> +struct pcat_leds { >> + struct device *dev; > > Where is this used? I used it to print logs, but now it doesn't, I will remove it. > >> + struct pcat_pmu *pmu; > > Why do you need to store this? > > Can't you get this at the call-site by: > > dev_get_drvdata(cdev->dev->parent) Yes, I will change it. >> + struct led_classdev cdev; >> +}; [...] >> +static int pcat_leds_probe(struct platform_device *pdev) >> +{ >> + int ret; > > Small sized variables at the bottom please. > >> + struct device *dev = &pdev->dev; >> + struct pcat_leds *leds; >> + const char *label; >> + >> + leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL); >> + if (!leds) >> + return -ENOMEM; >> + >> + leds->dev = dev; > > Where is this used? > >> + leds->pmu = dev_get_drvdata(dev->parent); >> + platform_set_drvdata(pdev, leds); > > Where do you platform_get_drvdata() > >> + ret = of_property_read_string(dev->of_node, "label", &label); [...] >> +static const struct of_device_id pcat_leds_dt_ids[] = { >> + { .compatible = "ariaboard,photonicat-pmu-leds", }, > > How many LEDs are there? Photonicat has three LEDs: - system operation status indicator - charging status indicator - network status indicator and currently only one LED (network status indicator) can be controlled. >> + { /* sentinel */ } >> +}; [...] >> -- >> 2.46.0 Thanks for your review, I will fix all problems in next version! Best regards, Junhao
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 8d9d8da376e4..539adb5944e6 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -381,6 +381,17 @@ config LEDS_PCA9532_GPIO To use a pin as gpio pca9532_type in pca9532_platform data needs to set to PCA9532_TYPE_GPIO. +config LEDS_PHOTONICAT_PMU + tristate "LED Support for Photonicat PMU" + depends on LEDS_CLASS + depends on MFD_PHOTONICAT_PMU + help + Photonicat has a network status LED that can be controlled by system, + this option enables support for LEDs connected to the Photonicat PMU. + + To compile this driver as a module, choose M here: the + module will be called leds-photonicat. + config LEDS_GPIO tristate "LED Support for GPIO connected LEDs" depends on LEDS_CLASS diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 18afbb5a23ee..dcd5312aee12 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_PCA9532) += leds-pca9532.o obj-$(CONFIG_LEDS_PCA955X) += leds-pca955x.o obj-$(CONFIG_LEDS_PCA963X) += leds-pca963x.o obj-$(CONFIG_LEDS_PCA995X) += leds-pca995x.o +obj-$(CONFIG_LEDS_PHOTONICAT_PMU) += leds-photonicat.o obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o obj-$(CONFIG_LEDS_POWERNV) += leds-powernv.o obj-$(CONFIG_LEDS_PWM) += leds-pwm.o diff --git a/drivers/leds/leds-photonicat.c b/drivers/leds/leds-photonicat.c new file mode 100644 index 000000000000..3aa5ce525b83 --- /dev/null +++ b/drivers/leds/leds-photonicat.c @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2024 Junhao Xie <bigfoot@classfun.cn> + */ + +#include <linux/mfd/photonicat-pmu.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/leds.h> + +struct pcat_leds { + struct device *dev; + struct pcat_pmu *pmu; + struct led_classdev cdev; +}; + +static int pcat_led_status_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct pcat_leds *leds = container_of(cdev, struct pcat_leds, cdev); + struct pcat_data_cmd_led_setup setup = { 0, 0, 0 }; + + if (brightness) + setup.on_time = 100; + else + setup.down_time = 100; + return pcat_pmu_write_data(leds->pmu, PCAT_CMD_NET_STATUS_LED_SETUP, + &setup, sizeof(setup)); +} + +static int pcat_leds_probe(struct platform_device *pdev) +{ + int ret; + struct device *dev = &pdev->dev; + struct pcat_leds *leds; + const char *label; + + leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL); + if (!leds) + return -ENOMEM; + + leds->dev = dev; + leds->pmu = dev_get_drvdata(dev->parent); + platform_set_drvdata(pdev, leds); + + ret = of_property_read_string(dev->of_node, "label", &label); + if (ret) + return dev_err_probe(dev, ret, "No label property\n"); + + leds->cdev.name = label; + leds->cdev.max_brightness = 1; + leds->cdev.brightness_set_blocking = pcat_led_status_set; + + return devm_led_classdev_register(dev, &leds->cdev); +} + +static const struct of_device_id pcat_leds_dt_ids[] = { + { .compatible = "ariaboard,photonicat-pmu-leds", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, pcat_leds_dt_ids); + +static struct platform_driver pcat_leds_driver = { + .driver = { + .name = "photonicat-leds", + .of_match_table = pcat_leds_dt_ids, + }, + .probe = pcat_leds_probe, +}; +module_platform_driver(pcat_leds_driver); + +MODULE_AUTHOR("Junhao Xie <bigfoot@classfun.cn>"); +MODULE_DESCRIPTION("Photonicat PMU Status LEDs"); +MODULE_LICENSE("GPL");
Photonicat has a network status LED that can be controlled by system. The LED status can be set through command 0x19. Signed-off-by: Junhao Xie <bigfoot@classfun.cn> --- drivers/leds/Kconfig | 11 +++++ drivers/leds/Makefile | 1 + drivers/leds/leds-photonicat.c | 75 ++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 drivers/leds/leds-photonicat.c