* [PATCH 05/10] backlight: ti-lmu-backlight: Add LM3633 driver
From: Milo Kim @ 2014-02-14 6:31 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
LM3633 has 3 backlight strings and 11 bit dimming is supported.
PWM brightness control is also supported.
Common backlight driver is controlled by TI LMU backlight driver.
Only LM3633 specific code is implemented here.
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
drivers/video/backlight/Kconfig | 9 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/lm3633_bl.c | 244 +++++++++++++++++++++++++++++++++++
3 files changed, 254 insertions(+)
create mode 100644 drivers/video/backlight/lm3633_bl.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a43a015..aa012a3 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -402,6 +402,15 @@ config BACKLIGHT_LM3631
Up to 2 backlight strings and 11 bit dimming is supported.
PWM brightness control is also supported.
+config BACKLIGHT_LM3633
+ tristate "Backlight driver for TI LM3633"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
+ select TI_LMU_BACKLIGHT
+ help
+ Say Y to enable the backlight driver for TI LM3633.
+ Up to 3 backlight strings and 11 bit dimming is supported.
+ PWM brightness control is also supported.
+
config TI_LMU_BACKLIGHT
tristate "Backlight driver for TI LMU"
depends on BACKLIGHT_LM3532 || BACKLIGHT_LM3631 || BACKLIGHT_LM3633 || BACKLIGHT_LM3695 || BACKLIGHT_LM3697
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 2a17f8b..842a256 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_BACKLIGHT_LM3630A) += lm3630a_bl.o
obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o
obj-$(CONFIG_BACKLIGHT_LM3532) += lm3532_bl.o
obj-$(CONFIG_BACKLIGHT_LM3631) += lm3631_bl.o
+obj-$(CONFIG_BACKLIGHT_LM3633) += lm3633_bl.o
obj-$(CONFIG_TI_LMU_BACKLIGHT) += ti-lmu-backlight.o
obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
diff --git a/drivers/video/backlight/lm3633_bl.c b/drivers/video/backlight/lm3633_bl.c
new file mode 100644
index 0000000..c12fd42
--- /dev/null
+++ b/drivers/video/backlight/lm3633_bl.c
@@ -0,0 +1,244 @@
+/*
+ * TI LM3633 Backlight Driver
+ *
+ * Copyright 2014 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * 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.
+ *
+ * LM3633 backlight driver consists of two parts
+ *
+ * 1) LM3633 chip specific part: this file
+ * Define device specific operations
+ * Register LMU backlight driver
+ *
+ * 2) LMU backlight common driver: ti-lmu-backlight
+ * General backlight subsystem control
+ *
+ * 3) LMU effect driver
+ * Backlight slope time configuration
+ */
+
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-effect.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+
+#include "ti-lmu-backlight.h"
+
+#define LM3633_BOOST_OVP_40V 0x6
+#define LM3633_BL_MAX_STRINGS 3
+#define LM3633_BL_MAX_BRIGHTNESS 2047
+
+static int lm3633_bl_init(struct ti_lmu_bl_chip *chip)
+{
+ /* Configure ramp selection for each bank */
+ return ti_lmu_update_bits(chip->lmu, LM3633_REG_BL_RAMP_CONF,
+ LM3633_BL_RAMP_MASK, LM3633_BL_RAMP_EACH);
+}
+
+static int lm3633_bl_enable(struct ti_lmu_bl *lmu_bl, int enable)
+{
+ return ti_lmu_update_bits(lmu_bl->chip->lmu, LM3633_REG_ENABLE,
+ BIT(lmu_bl->bank_id),
+ enable << lmu_bl->bank_id);
+}
+
+static int lm3633_bl_set_brightness(struct ti_lmu_bl *lmu_bl, int brightness)
+{
+ int ret;
+ u8 data;
+ u8 reg_lsb[] = { LM3633_REG_BRT_HVLED_A_LSB,
+ LM3633_REG_BRT_HVLED_B_LSB, };
+ u8 reg_msb[] = { LM3633_REG_BRT_HVLED_A_MSB,
+ LM3633_REG_BRT_HVLED_B_MSB, };
+
+ if (lmu_bl->mode == BL_PWM_BASED) {
+ /*
+ * PWM can start from any non-zero code and dim down to zero.
+ * So, brightness register should be updated even in PWM mode.
+ */
+ if (brightness > 0)
+ brightness = LM3633_BL_MAX_BRIGHTNESS;
+ else
+ brightness = 0;
+ }
+
+ data = brightness & LM3633_BRT_HVLED_LSB_MASK;
+ ret = ti_lmu_update_bits(lmu_bl->chip->lmu, reg_lsb[lmu_bl->bank_id],
+ LM3633_BRT_HVLED_LSB_MASK, data);
+ if (ret)
+ return ret;
+
+ data = (brightness >> LM3633_BRT_HVLED_MSB_SHIFT) & 0xFF;
+ return ti_lmu_write_byte(lmu_bl->chip->lmu, reg_msb[lmu_bl->bank_id],
+ data);
+}
+
+static int lm3633_bl_boost_configure(struct ti_lmu_bl *lmu_bl)
+{
+ return ti_lmu_update_bits(lmu_bl->chip->lmu, LM3633_REG_BOOST_CFG,
+ LM3633_BOOST_OVP_MASK, LM3633_BOOST_OVP_40V);
+}
+
+static int lm3633_bl_set_ctrl_mode(struct ti_lmu_bl *lmu_bl)
+{
+ int bank_id = lmu_bl->bank_id;
+
+ if (lmu_bl->mode == BL_PWM_BASED)
+ return ti_lmu_update_bits(lmu_bl->chip->lmu,
+ LM3633_REG_PWM_CFG,
+ BIT(bank_id), 1 << bank_id);
+
+ return 0;
+}
+
+static int lm3633_bl_string_configure(struct ti_lmu_bl *lmu_bl)
+{
+ struct ti_lmu *lmu = lmu_bl->chip->lmu;
+ int is_detected = 0;
+ int i, ret;
+
+ /* Assign control bank from backlight string configuration */
+ for (i = 0; i < LM3633_BL_MAX_STRINGS; i++) {
+ if (test_bit(i, &lmu_bl->bl_pdata->bl_string)) {
+ ret = ti_lmu_update_bits(lmu,
+ LM3633_REG_HVLED_OUTPUT_CFG,
+ BIT(i), lmu_bl->bank_id << i);
+ if (ret)
+ return ret;
+
+ is_detected = 1;
+ }
+ }
+
+ if (!is_detected) {
+ dev_err(lmu_bl->chip->dev, "No backlight string found\n");
+ return -EINVAL;
+ }
+
+ if (lmu_bl->mode == BL_PWM_BASED) {
+ snprintf(lmu_bl->pwm_name, sizeof(lmu_bl->pwm_name),
+ "%s", LMU_BL_DEFAULT_PWM_NAME);
+ }
+
+ return 0;
+}
+
+static int lm3633_bl_set_current(struct ti_lmu_bl *lmu_bl)
+{
+ u8 reg[] = { LM3633_REG_IMAX_HVLED_A, LM3633_REG_IMAX_HVLED_B, };
+
+ return ti_lmu_write_byte(lmu_bl->chip->lmu, reg[lmu_bl->bank_id],
+ lmu_bl->bl_pdata->imax);
+}
+
+static void lm3633_bl_effect_request(enum lmu_effect_request_id id,
+ struct ti_lmu_bl *lmu_bl)
+{
+ const char *name[][2] = {
+ { LM3633_EFFECT_BL0_RAMPUP, LM3633_EFFECT_BL0_RAMPDOWN },
+ { LM3633_EFFECT_BL1_RAMPUP, LM3633_EFFECT_BL1_RAMPDOWN },
+ };
+
+ ti_lmu_effect_request(name[lmu_bl->bank_id][id],
+ ti_lmu_backlight_effect_callback, id, lmu_bl);
+}
+
+static int lm3633_bl_configure(struct ti_lmu_bl *lmu_bl)
+{
+ int ret;
+
+ ret = lm3633_bl_boost_configure(lmu_bl);
+ if (ret)
+ return ret;
+
+ ret = lm3633_bl_set_ctrl_mode(lmu_bl);
+ if (ret)
+ return ret;
+
+ ret = lm3633_bl_string_configure(lmu_bl);
+ if (ret)
+ return ret;
+
+ ret = lm3633_bl_set_current(lmu_bl);
+ if (ret)
+ return ret;
+
+ /* LMU effect is optional, checking return value is not required */
+ if (lmu_bl->bl_pdata->ramp_up_ms)
+ lm3633_bl_effect_request(BL_EFFECT_RAMPUP, lmu_bl);
+
+ if (lmu_bl->bl_pdata->ramp_down_ms)
+ lm3633_bl_effect_request(BL_EFFECT_RAMPDN, lmu_bl);
+
+ return 0;
+}
+
+static const struct ti_lmu_bl_ops lm3633_lmu_ops = {
+ .init = lm3633_bl_init,
+ .configure = lm3633_bl_configure,
+ .update_brightness = lm3633_bl_set_brightness,
+ .bl_enable = lm3633_bl_enable,
+ .max_brightness = LM3633_BL_MAX_BRIGHTNESS,
+};
+
+static int lm3633_bl_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct ti_lmu_bl *lmu_bl;
+ struct ti_lmu_bl_chip *chip;
+
+ chip = ti_lmu_backlight_init_device(&pdev->dev, lmu, &lm3633_lmu_ops);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ lmu_bl = ti_lmu_backlight_register(chip, lmu->pdata->bl_pdata,
+ lmu->pdata->num_backlights);
+ if (IS_ERR(lmu_bl))
+ return PTR_ERR(lmu_bl);
+
+ platform_set_drvdata(pdev, lmu_bl);
+
+ return 0;
+}
+
+static int lm3633_bl_remove(struct platform_device *pdev)
+{
+ struct ti_lmu_bl *lmu_bl = platform_get_drvdata(pdev);
+
+ return ti_lmu_backlight_unregister(lmu_bl);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lm3633_bl_of_match[] = {
+ { .compatible = "ti,lm3633-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3633_bl_of_match);
+#endif
+
+static struct platform_driver lm3633_bl_driver = {
+ .probe = lm3633_bl_probe,
+ .remove = lm3633_bl_remove,
+ .driver = {
+ .name = "lm3633-backlight",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lm3633_bl_of_match),
+ },
+};
+module_platform_driver(lm3633_bl_driver);
+
+MODULE_DESCRIPTION("TI LM3633 Backlight Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lm3633-backlight");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 06/10] backlight: ti-lmu-backlight: Add LM3695 driver
From: Milo Kim @ 2014-02-14 6:31 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
LM3695 has 2 backlight strings and 11 bit dimming is supported.
Common backlight driver is controlled by TI LMU backlight driver.
Only LM3695 specific code is implemented here.
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
drivers/video/backlight/Kconfig | 8 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/lm3695_bl.c | 143 +++++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
create mode 100644 drivers/video/backlight/lm3695_bl.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index aa012a3..b169c8d 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -411,6 +411,14 @@ config BACKLIGHT_LM3633
Up to 3 backlight strings and 11 bit dimming is supported.
PWM brightness control is also supported.
+config BACKLIGHT_LM3695
+ tristate "Backlight driver for TI LM3695"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
+ select TI_LMU_BACKLIGHT
+ help
+ Say Y to enable the backlight driver for TI LM3695.
+ Up to 2 backlight strings and 11 bit dimming is supported.
+
config TI_LMU_BACKLIGHT
tristate "Backlight driver for TI LMU"
depends on BACKLIGHT_LM3532 || BACKLIGHT_LM3631 || BACKLIGHT_LM3633 || BACKLIGHT_LM3695 || BACKLIGHT_LM3697
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 842a256..5427162 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_BACKLIGHT_LM3639) += lm3639_bl.o
obj-$(CONFIG_BACKLIGHT_LM3532) += lm3532_bl.o
obj-$(CONFIG_BACKLIGHT_LM3631) += lm3631_bl.o
obj-$(CONFIG_BACKLIGHT_LM3633) += lm3633_bl.o
+obj-$(CONFIG_BACKLIGHT_LM3695) += lm3695_bl.o
obj-$(CONFIG_TI_LMU_BACKLIGHT) += ti-lmu-backlight.o
obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
diff --git a/drivers/video/backlight/lm3695_bl.c b/drivers/video/backlight/lm3695_bl.c
new file mode 100644
index 0000000..959e72a
--- /dev/null
+++ b/drivers/video/backlight/lm3695_bl.c
@@ -0,0 +1,143 @@
+/*
+ * TI LM3695 Backlight Driver
+ *
+ * Copyright 2014 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * 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.
+ *
+ * LM3695 backlight driver consists of two parts
+ *
+ * 1) LM3695 chip specific part: this file
+ * Define device specific operations
+ * Register LMU backlight driver
+ *
+ * 2) LMU backlight common driver: ti-lmu-backlight
+ * General backlight subsystem control
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "ti-lmu-backlight.h"
+
+#define LM3695_FULL_STRINGS (LMU_HVLED1 | LMU_HVLED2)
+#define LM3695_MAX_BRIGHTNESS 2047
+
+static int lm3695_bl_enable(struct ti_lmu_bl *lmu_bl, int enable)
+{
+ int ret;
+
+ ret = ti_lmu_update_bits(lmu_bl->chip->lmu, LM3695_REG_GP,
+ LM3695_BL_EN_MASK, enable);
+ if (ret)
+ return ret;
+
+ /* Wait time for brightness register wake up */
+ usleep_range(600, 700);
+
+ return 0;
+}
+
+static int lm3695_bl_set_brightness(struct ti_lmu_bl *lmu_bl, int brightness)
+{
+ u8 data;
+ int ret;
+
+ data = brightness & LM3695_BRT_LSB_MASK;
+ ret = ti_lmu_update_bits(lmu_bl->chip->lmu, LM3695_REG_BRT_LSB,
+ LM3695_BRT_LSB_MASK, data);
+ if (ret)
+ return ret;
+
+ data = (brightness >> LM3695_BRT_MSB_SHIFT) & 0xFF;
+ return ti_lmu_write_byte(lmu_bl->chip->lmu, LM3695_REG_BRT_MSB,
+ data);
+}
+
+static int lm3695_bl_init(struct ti_lmu_bl_chip *chip)
+{
+ return ti_lmu_update_bits(chip->lmu, LM3695_REG_GP,
+ LM3695_BRT_RW_MASK, LM3695_BRT_SET_RW);
+}
+
+static int lm3695_bl_configure(struct ti_lmu_bl *lmu_bl)
+{
+ u8 val;
+
+ if (lmu_bl->bl_pdata->bl_string == LM3695_FULL_STRINGS)
+ val = LM3695_BL_TWO_STRINGS;
+ else
+ val = LM3695_BL_ONE_STRING;
+
+ return ti_lmu_update_bits(lmu_bl->chip->lmu, LM3695_REG_GP,
+ LM3695_BL_STRING_MASK, val);
+}
+
+static const struct ti_lmu_bl_ops lm3695_lmu_ops = {
+ .init = lm3695_bl_init,
+ .configure = lm3695_bl_configure,
+ .update_brightness = lm3695_bl_set_brightness,
+ .bl_enable = lm3695_bl_enable,
+ .max_brightness = LM3695_MAX_BRIGHTNESS,
+};
+
+static int lm3695_bl_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct ti_lmu_bl *lmu_bl;
+ struct ti_lmu_bl_chip *chip;
+
+ chip = ti_lmu_backlight_init_device(&pdev->dev, lmu, &lm3695_lmu_ops);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ lmu_bl = ti_lmu_backlight_register(chip, lmu->pdata->bl_pdata, 1);
+ if (IS_ERR(lmu_bl))
+ return PTR_ERR(lmu_bl);
+
+ platform_set_drvdata(pdev, lmu_bl);
+
+ return 0;
+}
+
+static int lm3695_bl_remove(struct platform_device *pdev)
+{
+ struct ti_lmu_bl *lmu_bl = platform_get_drvdata(pdev);
+
+ return ti_lmu_backlight_unregister(lmu_bl);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lm3695_bl_of_match[] = {
+ { .compatible = "ti,lm3695-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3695_bl_of_match);
+#endif
+
+static struct platform_driver lm3695_bl_driver = {
+ .probe = lm3695_bl_probe,
+ .remove = lm3695_bl_remove,
+ .driver = {
+ .name = "lm3695-backlight",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lm3695_bl_of_match),
+ },
+};
+module_platform_driver(lm3695_bl_driver);
+
+MODULE_DESCRIPTION("TI LM3695 Backlight Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lm3695-backlight");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 07/10] backlight: ti-lmu-backlight: Add LM3697 driver
From: Milo Kim @ 2014-02-14 6:32 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
LM3697 has 3 backlight strings and 11 bit dimming is supported.
PWM brightness control is also supported.
Common backlight driver is controlled by TI LMU backlight driver.
Only LM3697 specific code is implemented here.
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
drivers/video/backlight/Kconfig | 9 ++
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/lm3697_bl.c | 224 +++++++++++++++++++++++++++++++++++
3 files changed, 234 insertions(+)
create mode 100644 drivers/video/backlight/lm3697_bl.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index b169c8d..3c39b30 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -419,6 +419,15 @@ config BACKLIGHT_LM3695
Say Y to enable the backlight driver for TI LM3695.
Up to 2 backlight strings and 11 bit dimming is supported.
+config BACKLIGHT_LM3697
+ tristate "Backlight driver for TI LM3697"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_TI_LMU
+ select TI_LMU_BACKLIGHT
+ help
+ Say Y to enable the backlight driver for TI LM3697.
+ Up to 3 backlight strings and 11 bit dimming is supported.
+ PWM brightness control is also supported.
+
config TI_LMU_BACKLIGHT
tristate "Backlight driver for TI LMU"
depends on BACKLIGHT_LM3532 || BACKLIGHT_LM3631 || BACKLIGHT_LM3633 || BACKLIGHT_LM3695 || BACKLIGHT_LM3697
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 5427162..16e6ad7 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_BACKLIGHT_LM3532) += lm3532_bl.o
obj-$(CONFIG_BACKLIGHT_LM3631) += lm3631_bl.o
obj-$(CONFIG_BACKLIGHT_LM3633) += lm3633_bl.o
obj-$(CONFIG_BACKLIGHT_LM3695) += lm3695_bl.o
+obj-$(CONFIG_BACKLIGHT_LM3697) += lm3697_bl.o
obj-$(CONFIG_TI_LMU_BACKLIGHT) += ti-lmu-backlight.o
obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o
diff --git a/drivers/video/backlight/lm3697_bl.c b/drivers/video/backlight/lm3697_bl.c
new file mode 100644
index 0000000..18b866a
--- /dev/null
+++ b/drivers/video/backlight/lm3697_bl.c
@@ -0,0 +1,224 @@
+/*
+ * TI LM3697 Backlight Driver
+ *
+ * Copyright 2014 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * 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.
+ *
+ * LM3697 backlight driver consists of three parts
+ *
+ * 1) LM3697 chip specific part: this file
+ * Define device specific operations
+ * Register LMU backlight driver
+ *
+ * 2) LMU backlight common driver: ti-lmu-backlight
+ * General backlight subsystem control
+ *
+ * 3) LMU effect driver
+ * Backlight ramp time configuration
+ */
+
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-effect.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+
+#include "ti-lmu-backlight.h"
+
+#define LM3697_BL_MAX_STRINGS 3
+#define LM3697_MAX_BRIGHTNESS 2047
+
+static int lm3697_bl_init(struct ti_lmu_bl_chip *chip)
+{
+ /* Configure ramp selection for each bank */
+ return ti_lmu_update_bits(chip->lmu, LM3697_REG_RAMP_CONF,
+ LM3697_RAMP_MASK, LM3697_RAMP_EACH);
+}
+
+static int lm3697_bl_enable(struct ti_lmu_bl *lmu_bl, int enable)
+{
+ return ti_lmu_update_bits(lmu_bl->chip->lmu, LM3697_REG_ENABLE,
+ BIT(lmu_bl->bank_id),
+ enable << lmu_bl->bank_id);
+}
+
+static int lm3697_bl_set_brightness(struct ti_lmu_bl *lmu_bl, int brightness)
+{
+ int ret;
+ u8 data;
+ u8 reg_lsb[] = { LM3697_REG_BRT_A_LSB, LM3697_REG_BRT_B_LSB, };
+ u8 reg_msb[] = { LM3697_REG_BRT_A_MSB, LM3697_REG_BRT_B_MSB, };
+
+ data = brightness & LM3697_BRT_LSB_MASK;
+ ret = ti_lmu_update_bits(lmu_bl->chip->lmu, reg_lsb[lmu_bl->bank_id],
+ LM3697_BRT_LSB_MASK, data);
+ if (ret)
+ return ret;
+
+ data = (brightness >> LM3697_BRT_MSB_SHIFT) & 0xFF;
+ return ti_lmu_write_byte(lmu_bl->chip->lmu, reg_msb[lmu_bl->bank_id],
+ data);
+}
+
+static int lm3697_bl_set_ctrl_mode(struct ti_lmu_bl *lmu_bl)
+{
+ int bank_id = lmu_bl->bank_id;
+
+ if (lmu_bl->mode == BL_PWM_BASED)
+ return ti_lmu_update_bits(lmu_bl->chip->lmu,
+ LM3697_REG_PWM_CFG,
+ BIT(bank_id), 1 << bank_id);
+
+ return 0;
+}
+
+static int lm3697_bl_string_configure(struct ti_lmu_bl *lmu_bl)
+{
+ struct ti_lmu *lmu = lmu_bl->chip->lmu;
+ int bank_id = lmu_bl->bank_id;
+ int is_detected = 0;
+ int i, ret;
+
+ /* Assign control bank from backlight string configuration */
+ for (i = 0; i < LM3697_BL_MAX_STRINGS; i++) {
+ if (test_bit(i, &lmu_bl->bl_pdata->bl_string)) {
+ ret = ti_lmu_update_bits(lmu,
+ LM3697_REG_HVLED_OUTPUT_CFG,
+ BIT(i), bank_id << i);
+ if (ret)
+ return ret;
+
+ is_detected = 1;
+ }
+ }
+
+ if (!is_detected) {
+ dev_err(lmu_bl->chip->dev, "No backlight string found\n");
+ return -EINVAL;
+ }
+
+ if (lmu_bl->mode == BL_PWM_BASED) {
+ snprintf(lmu_bl->pwm_name, sizeof(lmu_bl->pwm_name),
+ "%s", LMU_BL_DEFAULT_PWM_NAME);
+
+ return ti_lmu_update_bits(lmu, LM3697_REG_PWM_CFG,
+ BIT(bank_id), 1 << bank_id);
+ }
+
+ return 0;
+}
+
+static int lm3697_bl_set_current(struct ti_lmu_bl *lmu_bl)
+{
+ u8 reg[] = { LM3697_REG_IMAX_A, LM3697_REG_IMAX_B, };
+
+ return ti_lmu_write_byte(lmu_bl->chip->lmu, reg[lmu_bl->bank_id],
+ lmu_bl->bl_pdata->imax);
+}
+
+static void lm3697_bl_effect_request(enum lmu_effect_request_id id,
+ struct ti_lmu_bl *lmu_bl)
+{
+ const char *name[][2] = {
+ { LM3697_EFFECT_BL0_RAMPUP, LM3697_EFFECT_BL0_RAMPDOWN },
+ { LM3697_EFFECT_BL1_RAMPUP, LM3697_EFFECT_BL1_RAMPDOWN },
+ };
+
+ ti_lmu_effect_request(name[lmu_bl->bank_id][id],
+ ti_lmu_backlight_effect_callback, id, lmu_bl);
+}
+
+static int lm3697_bl_configure(struct ti_lmu_bl *lmu_bl)
+{
+ int ret;
+
+ ret = lm3697_bl_set_ctrl_mode(lmu_bl);
+ if (ret)
+ return ret;
+
+ ret = lm3697_bl_string_configure(lmu_bl);
+ if (ret)
+ return ret;
+
+ ret = lm3697_bl_set_current(lmu_bl);
+ if (ret)
+ return ret;
+
+ /* LMU effect is optional, checking return value is not required */
+ if (lmu_bl->bl_pdata->ramp_up_ms)
+ lm3697_bl_effect_request(BL_EFFECT_RAMPUP, lmu_bl);
+
+ if (lmu_bl->bl_pdata->ramp_down_ms)
+ lm3697_bl_effect_request(BL_EFFECT_RAMPDN, lmu_bl);
+
+ return 0;
+}
+
+static const struct ti_lmu_bl_ops lm3697_lmu_ops = {
+ .init = lm3697_bl_init,
+ .configure = lm3697_bl_configure,
+ .update_brightness = lm3697_bl_set_brightness,
+ .bl_enable = lm3697_bl_enable,
+ .max_brightness = LM3697_MAX_BRIGHTNESS,
+};
+
+static int lm3697_bl_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct ti_lmu_bl *lmu_bl;
+ struct ti_lmu_bl_chip *chip;
+
+ chip = ti_lmu_backlight_init_device(&pdev->dev, lmu, &lm3697_lmu_ops);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ lmu_bl = ti_lmu_backlight_register(chip, lmu->pdata->bl_pdata,
+ lmu->pdata->num_backlights);
+ if (IS_ERR(lmu_bl))
+ return PTR_ERR(lmu_bl);
+
+ platform_set_drvdata(pdev, lmu_bl);
+
+ return 0;
+}
+
+static int lm3697_bl_remove(struct platform_device *pdev)
+{
+ struct ti_lmu_bl *lmu_bl = platform_get_drvdata(pdev);
+
+ return ti_lmu_backlight_unregister(lmu_bl);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lm3697_bl_of_match[] = {
+ { .compatible = "ti,lm3697-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3697_bl_of_match);
+#endif
+
+static struct platform_driver lm3697_bl_driver = {
+ .probe = lm3697_bl_probe,
+ .remove = lm3697_bl_remove,
+ .driver = {
+ .name = "lm3697-backlight",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lm3697_bl_of_match),
+ },
+};
+module_platform_driver(lm3697_bl_driver);
+
+MODULE_DESCRIPTION("TI LM3697 Backlight Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lm3697-backlight");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 08/10] leds: Add LM3633 driver
From: Milo Kim @ 2014-02-14 6:32 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
LM3633 LED driver supports generic LED functions and pattern generation.
Pattern is generated by using LMU effect driver APIs.
Sysfs documentation is added.
Cc: Bryan Wu <cooloney@gmail.com>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
Documentation/leds/leds-lm3633.txt | 38 +++
drivers/leds/Kconfig | 10 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-lm3633.c | 661 ++++++++++++++++++++++++++++++++++++
4 files changed, 710 insertions(+)
create mode 100644 Documentation/leds/leds-lm3633.txt
create mode 100644 drivers/leds/leds-lm3633.c
diff --git a/Documentation/leds/leds-lm3633.txt b/Documentation/leds/leds-lm3633.txt
new file mode 100644
index 0000000..a5a59c3
--- /dev/null
+++ b/Documentation/leds/leds-lm3633.txt
@@ -0,0 +1,38 @@
+LM3633 LED Driver
+=================
+
+LM3633 LED driver supports not only LED functions but also programmable pattern.
+A pattern is generated via the sysfs.
+
+LED Pattern Generator
+
+ (3)
+ (a) ---------------> ___________
+ / \
+ (2) / \ (4)
+ (b) ----> _________/ \_________ ...
+ (1) (5)
+
+ |<----- period -----> |
+
+ Time dimension
+ (1) delay : 0 ~ 10 sec
+ (2) rise : 0 ~ 16 sec
+ (3) high : 0 ~ 10 sec
+ (4) fall : 0 ~ 16 sec
+ (5) low : 0 ~ 16 sec
+
+ Level dimension - channel current
+ (a) low : 0 ~ 255
+ (b) high : 0 ~ 255
+
+Example)
+Time : No delay, rise 500ms, high 1000ms, fall 400ms, low 2000ms
+Level : Brightness 0 and 255
+
+echo 0 500 1000 400 2000 > /sys/class/leds/<LED NAME>/pattern_times
+echo 0 255 > /sys/class/leds/<LED NAME>/pattern_levels
+echo 1 > /sys/class/leds/<LED NAME>/run_pattern
+
+To stop running pattern,
+echo 0 > /sys/class/leds/<LED NAME>/run_pattern
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 72156c1..ed659be 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -63,6 +63,16 @@ config LEDS_LM3533
hardware-accelerated blinking with maximum on and off periods of 9.8
and 77 seconds respectively.
+config LEDS_LM3633
+ tristate "LED support for the TI LM3633 LMU"
+ depends on LEDS_CLASS
+ depends on MFD_TI_LMU
+ help
+ This option enables support for the LEDs on the LM3633.
+ LM3633 has 6 low voltage indicator LEDs.
+ All low voltage current sinks can have a programmable pattern
+ modulated onto LED output strings.
+
config LEDS_LM3642
tristate "LED support for LM3642 Chip"
depends on LEDS_CLASS && I2C
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 3cd76db..96f55fe 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o
obj-$(CONFIG_LEDS_LOCOMO) += leds-locomo.o
obj-$(CONFIG_LEDS_LM3530) += leds-lm3530.o
obj-$(CONFIG_LEDS_LM3533) += leds-lm3533.o
+obj-$(CONFIG_LEDS_LM3633) += leds-lm3633.o
obj-$(CONFIG_LEDS_LM3642) += leds-lm3642.o
obj-$(CONFIG_LEDS_MIKROTIK_RB532) += leds-rb532.o
obj-$(CONFIG_LEDS_S3C24XX) += leds-s3c24xx.o
diff --git a/drivers/leds/leds-lm3633.c b/drivers/leds/leds-lm3633.c
new file mode 100644
index 0000000..13a43bf
--- /dev/null
+++ b/drivers/leds/leds-lm3633.c
@@ -0,0 +1,661 @@
+/*
+ * TI LM3633 LED driver
+ *
+ * Copyright 2014 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * 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.
+ *
+ * LM3633 LED driver has features below.
+ *
+ * - Generic LED subsystem control
+ * - LED string configuration
+ * - Pattern programming via the sysfs
+ * - Platform data configuration from the device tree nodes
+ *
+ * Pattern generated by using LMU effect driver APIs.
+ *
+ */
+
+#include <linux/leds.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-effect.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define LM3633_LED_MAX_BRIGHTNESS 255
+#define LM3633_DEFAULT_LED_NAME "indicator"
+
+enum lm3633_led_bank_id {
+ LM3633_LED_BANK_C,
+ LM3633_LED_BANK_D,
+ LM3633_LED_BANK_E,
+ LM3633_LED_BANK_F,
+ LM3633_LED_BANK_G,
+ LM3633_LED_BANK_H,
+ LM3633_MAX_LEDS,
+};
+
+struct lm3633_pattern_time {
+ unsigned int delay;
+ unsigned int rise;
+ unsigned int high;
+ unsigned int fall;
+ unsigned int low;
+};
+
+struct lm3633_pattern_level {
+ u8 low;
+ u8 high;
+};
+
+/* One LED chip can have multiple LED strings (max: 6) */
+struct ti_lmu_led_chip {
+ struct device *dev;
+ struct ti_lmu *lmu;
+ struct mutex lock;
+ int num_leds;
+};
+
+/* LED string structure */
+struct ti_lmu_led {
+ enum lm3633_led_bank_id bank_id;
+
+ struct led_classdev cdev;
+ struct ti_lmu_led_chip *chip;
+ struct ti_lmu_led_platform_data *led_pdata;
+
+ struct work_struct work;
+ enum led_brightness brightness;
+
+ /* Pattern specific data */
+ struct lm3633_pattern_time time;
+ struct lm3633_pattern_level level;
+};
+
+static struct ti_lmu_led *to_ti_lmu_led(struct device *dev)
+{
+ struct led_classdev *_cdev = dev_get_drvdata(dev);
+ return container_of(_cdev, struct ti_lmu_led, cdev);
+}
+
+static int lm3633_led_config_bank(struct ti_lmu_led *lmu_led)
+{
+ u8 val;
+ int i, ret;
+ u8 group_led[] = { 0, BIT(0), BIT(0), 0, BIT(3), BIT(3), };
+ enum lm3633_led_bank_id default_id[] = {
+ LM3633_LED_BANK_C, LM3633_LED_BANK_C, LM3633_LED_BANK_C,
+ LM3633_LED_BANK_F, LM3633_LED_BANK_F, LM3633_LED_BANK_F,
+ };
+ enum lm3633_led_bank_id separate_id[] = {
+ LM3633_LED_BANK_C, LM3633_LED_BANK_D, LM3633_LED_BANK_E,
+ LM3633_LED_BANK_F, LM3633_LED_BANK_G, LM3633_LED_BANK_H,
+ };
+
+ /*
+ * Check configured LED string and assign control bank
+ *
+ * Each LED is tied with other LEDS (group):
+ * the default control bank is assigned
+ *
+ * Otherwise:
+ * separate bank is assigned
+ */
+
+ for (i = 0; i < LM3633_MAX_LEDS; i++) {
+ /* LED 1 and LED 4 are fixed, so no assignment is required */
+ if (i == 0 || i == 3)
+ continue;
+
+ if (test_bit(i, &lmu_led->led_pdata->led_string)) {
+ if (lmu_led->led_pdata->led_string & group_led[i]) {
+ lmu_led->bank_id = default_id[i];
+ val = 0;
+ } else {
+ lmu_led->bank_id = separate_id[i];
+ val = BIT(i);
+ }
+
+ ret = ti_lmu_update_bits(lmu_led->chip->lmu,
+ LM3633_REG_BANK_SEL,
+ BIT(i), val);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int lm3633_led_enable_bank(struct ti_lmu_led *lmu_led, bool on)
+{
+ u8 mask = 1 << (lmu_led->bank_id + LM3633_LED_BANK_OFFSET);
+
+ if (on)
+ return ti_lmu_update_bits(lmu_led->chip->lmu,
+ LM3633_REG_ENABLE, mask, mask);
+ else
+ return ti_lmu_update_bits(lmu_led->chip->lmu,
+ LM3633_REG_ENABLE, mask, 0);
+}
+
+/*
+ * This callback function is invoked in case the LMU effect driver is
+ * requested successfully.
+ */
+static void lm3633_led_effect_cb(struct ti_lmu_effect *lmu_effect,
+ int req_id, void *data)
+{
+ struct ti_lmu_led *lmu_led = data;
+ u8 reg_offset = lmu_led->bank_id * LM3633_PATTERN_REG_OFFSET;
+
+ switch (req_id) {
+ case LED_EFFECT_DELAY:
+ ti_lmu_effect_set_time(lmu_effect, lmu_led->time.delay,
+ reg_offset);
+ break;
+ case LED_EFFECT_HIGHTIME:
+ ti_lmu_effect_set_time(lmu_effect, lmu_led->time.high,
+ reg_offset);
+ break;
+ case LED_EFFECT_LOWTIME:
+ ti_lmu_effect_set_time(lmu_effect, lmu_led->time.low,
+ reg_offset);
+ break;
+ case LED_EFFECT_PTN0_RAMPUP:
+ case LED_EFFECT_PTN1_RAMPUP:
+ ti_lmu_effect_set_ramp(lmu_effect, lmu_led->time.rise);
+ break;
+ case LED_EFFECT_PTN0_RAMPDN:
+ case LED_EFFECT_PTN1_RAMPDN:
+ ti_lmu_effect_set_ramp(lmu_effect, lmu_led->time.fall);
+ break;
+ case LED_EFFECT_LOWBRT:
+ ti_lmu_effect_set_level(lmu_effect, lmu_led->level.low,
+ reg_offset);
+ break;
+ case LED_EFFECT_HIGHBRT:
+ ti_lmu_effect_set_level(lmu_effect, lmu_led->level.high,
+ lmu_led->bank_id);
+ break;
+ default:
+ break;
+ }
+}
+
+static int lm3633_led_effect_request(enum lmu_effect_request_id id,
+ struct ti_lmu_led *lmu_led)
+{
+ const char *name[] = {
+ [LED_EFFECT_DELAY] = LM3633_EFFECT_PTN_DELAY,
+ [LED_EFFECT_HIGHTIME] = LM3633_EFFECT_PTN_HIGHTIME,
+ [LED_EFFECT_LOWTIME] = LM3633_EFFECT_PTN_LOWTIME,
+ [LED_EFFECT_PTN0_RAMPUP] = LM3633_EFFECT_PTN0_RAMPUP,
+ [LED_EFFECT_PTN0_RAMPDN] = LM3633_EFFECT_PTN0_RAMPDOWN,
+ [LED_EFFECT_PTN1_RAMPUP] = LM3633_EFFECT_PTN1_RAMPUP,
+ [LED_EFFECT_PTN1_RAMPDN] = LM3633_EFFECT_PTN1_RAMPDOWN,
+ [LED_EFFECT_LOWBRT] = LM3633_EFFECT_PTN_LOWBRT,
+ [LED_EFFECT_HIGHBRT] = LM3633_EFFECT_PTN_HIGHBRT,
+ };
+
+ return ti_lmu_effect_request(name[id], lm3633_led_effect_cb, id,
+ lmu_led);
+}
+
+static ssize_t lm3633_led_show_pattern_times(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ti_lmu_led *lmu_led = to_ti_lmu_led(dev);
+
+ return sprintf(buf, "delay: %u, rise: %u, high:%u, fall:%u, low: %u\n",
+ lmu_led->time.delay, lmu_led->time.rise,
+ lmu_led->time.high, lmu_led->time.fall,
+ lmu_led->time.low);
+}
+
+static ssize_t lm3633_led_store_pattern_times(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ti_lmu_led *lmu_led = to_ti_lmu_led(dev);
+ int ret;
+
+ /*
+ * Sequence
+ *
+ * 1) Read pattern time data (unit: msec)
+ * 2) Update DELAY register
+ * 3) Update HIGH TIME register
+ * 4) Update LOW TIME register
+ * 5) Update RAMP TIME register
+ */
+
+ ret = sscanf(buf, "%u %u %u %u %u", &lmu_led->time.delay,
+ &lmu_led->time.rise, &lmu_led->time.high,
+ &lmu_led->time.fall, &lmu_led->time.low);
+ if (ret != 5)
+ return -EINVAL;
+
+ mutex_lock(&lmu_led->chip->lock);
+
+ ret = lm3633_led_effect_request(LED_EFFECT_DELAY, lmu_led);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_HIGHTIME, lmu_led);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_LOWTIME, lmu_led);
+ if (ret)
+ goto skip;
+
+ switch (lmu_led->bank_id) {
+ case LM3633_LED_BANK_C:
+ case LM3633_LED_BANK_D:
+ case LM3633_LED_BANK_E:
+ ret = lm3633_led_effect_request(LED_EFFECT_PTN0_RAMPUP,
+ lmu_led);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_PTN0_RAMPDN,
+ lmu_led);
+ if (ret)
+ goto skip;
+ break;
+ case LM3633_LED_BANK_F:
+ case LM3633_LED_BANK_G:
+ case LM3633_LED_BANK_H:
+ ret = lm3633_led_effect_request(LED_EFFECT_PTN1_RAMPUP,
+ lmu_led);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_PTN1_RAMPDN,
+ lmu_led);
+ if (ret)
+ goto skip;
+ break;
+ default:
+ break;
+ }
+
+ mutex_unlock(&lmu_led->chip->lock);
+ return len;
+skip:
+ mutex_unlock(&lmu_led->chip->lock);
+ return ret;
+}
+
+static ssize_t lm3633_led_show_pattern_levels(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct ti_lmu_led *lmu_led = to_ti_lmu_led(dev);
+
+ return sprintf(buf, "low brightness: %u, high brightness: %u\n",
+ lmu_led->level.low, lmu_led->level.high);
+}
+
+static ssize_t lm3633_led_store_pattern_levels(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ti_lmu_led *lmu_led = to_ti_lmu_led(dev);
+ unsigned int low, high;
+ int ret;
+
+ /*
+ * Sequence
+ *
+ * 1) Read pattern level data
+ * 2) Disable a bank before programming a pattern
+ * 3) Update LOW BRIGHTNESS register
+ * 4) Update HIGH BRIGHTNESS register
+ * 5) Update PATTERN ENABLE register
+ * 6) Enable the bank if required
+ */
+
+ ret = sscanf(buf, "%u %u", &low, &high);
+ if (ret != 2)
+ return -EINVAL;
+
+ low = min_t(unsigned int, low, LM3633_LED_MAX_BRIGHTNESS);
+ high = min_t(unsigned int, high, LM3633_LED_MAX_BRIGHTNESS);
+ lmu_led->level.low = (u8)low;
+ lmu_led->level.high = (u8)high;
+
+ mutex_lock(&lmu_led->chip->lock);
+ ret = lm3633_led_enable_bank(lmu_led, false);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_LOWBRT, lmu_led);
+ if (ret)
+ goto skip;
+
+ ret = lm3633_led_effect_request(LED_EFFECT_HIGHBRT, lmu_led);
+ if (ret)
+ goto skip;
+
+ mutex_unlock(&lmu_led->chip->lock);
+ return len;
+skip:
+ mutex_unlock(&lmu_led->chip->lock);
+ return ret;
+}
+
+static ssize_t lm3633_led_run_pattern(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct ti_lmu_led *lmu_led = to_ti_lmu_led(dev);
+ struct ti_lmu_led_chip *chip = lmu_led->chip;
+ u8 offset = lmu_led->bank_id + LM3633_LED_BANK_OFFSET;
+ u8 mask = LM3633_PATTERN_EN << offset;
+ unsigned long enable;
+ int ret;
+
+ if (kstrtoul(buf, 0, &enable))
+ return -EINVAL;
+
+ mutex_lock(&chip->lock);
+ if (enable)
+ ret = ti_lmu_update_bits(chip->lmu, LM3633_REG_PATTERN, mask,
+ mask);
+ else
+ ret = ti_lmu_update_bits(chip->lmu, LM3633_REG_PATTERN, mask,
+ 0);
+ if (ret) {
+ mutex_unlock(&chip->lock);
+ return ret;
+ }
+
+ if (enable)
+ lm3633_led_enable_bank(lmu_led, true);
+
+ mutex_unlock(&chip->lock);
+
+ return len;
+}
+
+static DEVICE_ATTR(pattern_times, S_IRUGO | S_IWUSR,
+ lm3633_led_show_pattern_times,
+ lm3633_led_store_pattern_times);
+static DEVICE_ATTR(pattern_levels, S_IRUGO | S_IWUSR,
+ lm3633_led_show_pattern_levels,
+ lm3633_led_store_pattern_levels);
+static DEVICE_ATTR(run_pattern, S_IWUSR, NULL,
+ lm3633_led_run_pattern);
+
+static struct attribute *lm3633_led_attributes[] = {
+ &dev_attr_pattern_times.attr,
+ &dev_attr_pattern_levels.attr,
+ &dev_attr_run_pattern.attr,
+ NULL,
+};
+
+static struct attribute_group lm3633_led_attr_group = {
+ .attrs = lm3633_led_attributes
+};
+
+static int lm3633_led_set_max_current(struct ti_lmu_led *lmu_led)
+{
+ u8 reg = LM3633_REG_IMAX_LVLED_BASE + lmu_led->bank_id;
+ enum ti_lmu_max_current imax = lmu_led->led_pdata->imax;
+
+ return ti_lmu_write_byte(lmu_led->chip->lmu, reg, imax);
+}
+
+static void lm3633_led_work(struct work_struct *_work)
+{
+ struct ti_lmu_led *lmu_led;
+ struct ti_lmu_led_chip *chip;
+
+ lmu_led = container_of(_work, struct ti_lmu_led, work);
+ chip = lmu_led->chip;
+
+ mutex_lock(&chip->lock);
+
+ ti_lmu_write_byte(chip->lmu,
+ LM3633_REG_BRT_LVLED_BASE + lmu_led->bank_id,
+ lmu_led->brightness);
+
+ if (lmu_led->brightness == 0)
+ lm3633_led_enable_bank(lmu_led, false);
+ else
+ lm3633_led_enable_bank(lmu_led, true);
+
+ mutex_unlock(&chip->lock);
+}
+
+static void lm3633_led_brightness_set(struct led_classdev *_cdev,
+ enum led_brightness brt_val)
+{
+ struct ti_lmu_led *lmu_led;
+
+ lmu_led = container_of(_cdev, struct ti_lmu_led, cdev);
+ lmu_led->brightness = brt_val;
+
+ schedule_work(&lmu_led->work);
+}
+
+static int lm3633_led_init(struct ti_lmu_led *lmu_led, int bank_id)
+{
+ struct device *dev = lmu_led->chip->dev;
+ char name[12];
+ int ret;
+
+ /*
+ * Sequence
+ *
+ * 1) Configure LED bank which is used for brightness control
+ * 2) Set max current for each output channel
+ * 3) Add LED device
+ * 4) Add sysfs attributes for LED pattern
+ */
+
+ ret = lm3633_led_config_bank(lmu_led);
+ if (ret) {
+ dev_err(dev, "Output bank register err: %d\n", ret);
+ return ret;
+ }
+
+ ret = lm3633_led_set_max_current(lmu_led);
+ if (ret) {
+ dev_err(dev, "Set max current err: %d\n", ret);
+ return ret;
+ }
+
+ lmu_led->cdev.max_brightness = LM3633_LED_MAX_BRIGHTNESS;
+ lmu_led->cdev.brightness_set = lm3633_led_brightness_set;
+ if (lmu_led->led_pdata->name) {
+ lmu_led->cdev.name = lmu_led->led_pdata->name;
+ } else {
+ snprintf(name, sizeof(name), "%s:%d", LM3633_DEFAULT_LED_NAME,
+ bank_id);
+ lmu_led->cdev.name = name;
+ }
+
+ ret = led_classdev_register(dev, &lmu_led->cdev);
+ if (ret) {
+ dev_err(dev, "LED register err: %d\n", ret);
+ return ret;
+ }
+
+ ret = sysfs_create_group(&lmu_led->cdev.dev->kobj,
+ &lm3633_led_attr_group);
+ if (ret) {
+ dev_err(dev, "LED sysfs err: %d\n", ret);
+ return ret;
+ }
+
+ INIT_WORK(&lmu_led->work, lm3633_led_work);
+
+ return 0;
+}
+
+static int lm3633_led_parse_dt(struct device *dev, struct ti_lmu *lmu)
+{
+ struct ti_lmu_led_platform_data *pdata;
+ struct device_node *node = dev->of_node;
+ struct device_node *child;
+ int num_leds;
+ int i = 0;
+ u8 imax_mA;
+
+ if (!node) {
+ dev_err(dev, "No device node exists\n");
+ return -ENODEV;
+ }
+
+ num_leds = of_get_child_count(node);
+ if (num_leds == 0) {
+ dev_err(dev, "No LED channels\n");
+ return -EINVAL;
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata) * num_leds, GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ for_each_child_of_node(node, child) {
+ of_property_read_string(child, "chan-name", &pdata[i].name);
+
+ /* Make LED strings */
+ pdata[i].led_string = 0;
+ if (of_find_property(child, "lvled1-used", NULL))
+ pdata[i].led_string |= LMU_LVLED1;
+ if (of_find_property(child, "lvled2-used", NULL))
+ pdata[i].led_string |= LMU_LVLED2;
+ if (of_find_property(child, "lvled3-used", NULL))
+ pdata[i].led_string |= LMU_LVLED3;
+ if (of_find_property(child, "lvled4-used", NULL))
+ pdata[i].led_string |= LMU_LVLED4;
+ if (of_find_property(child, "lvled5-used", NULL))
+ pdata[i].led_string |= LMU_LVLED5;
+ if (of_find_property(child, "lvled6-used", NULL))
+ pdata[i].led_string |= LMU_LVLED6;
+
+ of_property_read_u8(child, "max-current-milliamp", &imax_mA);
+ pdata[i].imax = ti_lmu_get_current_code(imax_mA);
+
+ i++;
+ }
+
+ lmu->pdata->led_pdata = pdata;
+ lmu->pdata->num_leds = num_leds;
+
+ return 0;
+}
+
+static int lm3633_led_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct ti_lmu_led_chip *chip;
+ struct ti_lmu_led *lmu_led, *each;
+ struct device *dev = &pdev->dev;
+ int i, ret, num_leds;
+
+ if (!lmu->pdata->led_pdata) {
+ if (IS_ENABLED(CONFIG_OF))
+ ret = lm3633_led_parse_dt(dev, lmu);
+ else
+ return -ENODEV;
+
+ if (ret)
+ return ret;
+ }
+
+ num_leds = lmu->pdata->num_leds;
+ if (num_leds > LM3633_MAX_LEDS || num_leds <= 0) {
+ dev_err(dev, "Invalid num_leds: %d\n", num_leds);
+ return -EINVAL;
+ }
+
+ chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->dev = dev;
+ chip->lmu = lmu;
+
+ lmu_led = devm_kzalloc(dev, sizeof(*lmu_led) * num_leds, GFP_KERNEL);
+ if (!lmu_led)
+ return -ENOMEM;
+
+ for (i = 0; i < num_leds; i++) {
+ each = lmu_led + i;
+ each->bank_id = 0;
+ each->chip = chip;
+ each->led_pdata = lmu->pdata->led_pdata + i;
+
+ ret = lm3633_led_init(each, i);
+ if (ret) {
+ dev_err(dev, "Initialize a LED err: %d\n", ret);
+ goto cleanup_leds;
+ }
+ }
+
+ chip->num_leds = num_leds;
+ mutex_init(&chip->lock);
+ platform_set_drvdata(pdev, lmu_led);
+
+ return 0;
+
+cleanup_leds:
+ while (--i >= 0) {
+ each = lmu_led + i;
+ led_classdev_unregister(&each->cdev);
+ }
+ return ret;
+}
+
+static int lm3633_led_remove(struct platform_device *pdev)
+{
+ struct ti_lmu_led *lmu_led = platform_get_drvdata(pdev);
+ struct ti_lmu_led *each;
+ int i;
+
+ for (i = 0; i < lmu_led->chip->num_leds; i++) {
+ each = lmu_led + i;
+ led_classdev_unregister(&each->cdev);
+ flush_work(&each->work);
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lm3633_led_of_match[] = {
+ { .compatible = "ti,lm3633-leds", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3633_led_of_match);
+#endif
+
+static struct platform_driver lm3633_led_driver = {
+ .probe = lm3633_led_probe,
+ .remove = lm3633_led_remove,
+ .driver = {
+ .name = "lm3633-leds",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lm3633_led_of_match),
+ },
+};
+module_platform_driver(lm3633_led_driver);
+
+MODULE_DESCRIPTION("TI LM3633 LED Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lm3633-leds");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 09/10] regulator: Add LM3631 driver
From: Milo Kim @ 2014-02-14 6:32 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
LM3631 regulator driver has 5 regulators.
One boost output and four LDOs are used for the display module.
Boost voltage is configurable but always on.
Supported operations for LDOs are enabled/disabled and voltage change.
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
drivers/regulator/Kconfig | 8 +
drivers/regulator/Makefile | 1 +
drivers/regulator/lm3631-regulator.c | 285 ++++++++++++++++++++++++++++++++++
3 files changed, 294 insertions(+)
create mode 100644 drivers/regulator/lm3631-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6a79328..9809d7b 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -221,6 +221,14 @@ config REGULATOR_ISL6271A
help
This driver supports ISL6271A voltage regulator chip.
+config REGULATOR_LM3631
+ tristate "TI LM3631 voltage regulators"
+ depends on MFD_TI_LMU
+ help
+ This driver supports LM3631 voltage regulators for the LCD bias.
+ One boost output voltage is configurable and always on.
+ Four LDOs are used for the display module.
+
config REGULATOR_LP3971
tristate "National Semiconductors LP3971 PMIC regulator driver"
depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 979f9dd..b20070d 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o
obj-$(CONFIG_REGULATOR_GPIO) += gpio-regulator.o
obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
+obj-$(CONFIG_REGULATOR_LM3631) += lm3631-regulator.o
obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
obj-$(CONFIG_REGULATOR_LP3972) += lp3972.o
obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o
diff --git a/drivers/regulator/lm3631-regulator.c b/drivers/regulator/lm3631-regulator.c
new file mode 100644
index 0000000..930bad4
--- /dev/null
+++ b/drivers/regulator/lm3631-regulator.c
@@ -0,0 +1,285 @@
+/*
+ * TI LM3631 Regulator Driver
+ *
+ * Copyright 2014 Texas Instruments
+ *
+ * Author: Milo Kim <milo.kim@ti.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+
+#define ENABLE_TIME_USEC 1000
+
+enum lm3631_regulator_id {
+ LM3631_BOOST, /* Boost output */
+ LM3631_LDO_CONT, /* Display panel controller */
+ LM3631_LDO_OREF, /* Gamma reference */
+ LM3631_LDO_POS, /* Positive display bias output */
+ LM3631_LDO_NEG, /* Negative display bias output */
+};
+
+struct lm3631_regulator {
+ struct ti_lmu *lmu;
+ struct regulator_desc *desc;
+ struct regulator_dev *regulator;
+};
+
+static const int lm3631_boost_vtbl[] = {
+ 4500000, 4550000, 4600000, 4650000, 4700000, 4750000, 4800000, 4850000,
+ 4900000, 4950000, 5000000, 5050000, 5100000, 5150000, 5200000, 5250000,
+ 5300000, 5350000, 5400000, 5450000, 5500000, 5550000, 5600000, 5650000,
+ 5700000, 5750000, 5800000, 5850000, 5900000, 5950000, 6000000, 6050000,
+ 6100000, 6150000, 6200000, 6250000, 6300000, 6350000,
+};
+
+static const int lm3631_ldo_cont_vtbl[] = {
+ 1800000, 2300000, 2800000, 3300000,
+};
+
+static const int lm3631_ldo_target_vtbl[] = {
+ 4000000, 4050000, 4100000, 4150000, 4200000, 4250000, 4300000, 4350000,
+ 4400000, 4450000, 4500000, 4550000, 4600000, 4650000, 4700000, 4750000,
+ 4800000, 4850000, 4900000, 4950000, 5000000, 5050000, 5100000, 5150000,
+ 5200000, 5250000, 5300000, 5350000, 5400000, 5450000, 5500000, 5550000,
+ 5600000, 5650000, 5700000, 5750000, 5800000, 5850000, 5900000, 5950000,
+ 6000000,
+};
+
+const int ldo_cont_enable_time[] = {
+ 0, 2000, 5000, 10000, 20000, 50000, 100000, 200000,
+};
+
+static int lm3631_regulator_enable_time(struct regulator_dev *rdev)
+{
+ struct lm3631_regulator *lm3631_regulator = rdev_get_drvdata(rdev);
+ enum lm3631_regulator_id id = rdev_get_id(rdev);
+ u8 val, addr, mask;
+
+ switch (id) {
+ case LM3631_LDO_CONT:
+ addr = LM3631_REG_ENTIME_VCONT;
+ mask = LM3631_ENTIME_CONT_MASK;
+ break;
+ case LM3631_LDO_OREF:
+ addr = LM3631_REG_ENTIME_VOREF;
+ mask = LM3631_ENTIME_MASK;
+ break;
+ case LM3631_LDO_POS:
+ addr = LM3631_REG_ENTIME_VPOS;
+ mask = LM3631_ENTIME_MASK;
+ break;
+ case LM3631_LDO_NEG:
+ addr = LM3631_REG_ENTIME_VNEG;
+ mask = LM3631_ENTIME_MASK;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (ti_lmu_read_byte(lm3631_regulator->lmu, addr, &val))
+ return -EINVAL;
+
+ val = (val & mask) >> LM3631_ENTIME_SHIFT;
+
+ if (id == LM3631_LDO_CONT)
+ return ldo_cont_enable_time[val];
+ else
+ return ENABLE_TIME_USEC * val;
+}
+
+static struct regulator_ops lm3631_boost_voltage_table_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static struct regulator_ops lm3631_regulator_voltage_table_ops = {
+ .list_voltage = regulator_list_voltage_table,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable_time = lm3631_regulator_enable_time,
+};
+
+static struct regulator_desc lm3631_regulator_desc[] = {
+ {
+ .name = "vboost",
+ .id = LM3631_BOOST,
+ .ops = &lm3631_boost_voltage_table_ops,
+ .n_voltages = ARRAY_SIZE(lm3631_boost_vtbl),
+ .volt_table = lm3631_boost_vtbl,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .vsel_reg = LM3631_REG_VOUT_BOOST,
+ .vsel_mask = LM3631_VOUT_MASK,
+ },
+ {
+ .name = "ldo_cont",
+ .id = LM3631_LDO_CONT,
+ .ops = &lm3631_regulator_voltage_table_ops,
+ .n_voltages = ARRAY_SIZE(lm3631_ldo_cont_vtbl),
+ .volt_table = lm3631_ldo_cont_vtbl,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .vsel_reg = LM3631_REG_VOUT_CONT,
+ .vsel_mask = LM3631_VOUT_CONT_MASK,
+ .enable_reg = LM3631_REG_LDO_CTRL2,
+ .enable_mask = LM3631_EN_CONT_MASK,
+ },
+ {
+ .name = "ldo_oref",
+ .id = LM3631_LDO_OREF,
+ .ops = &lm3631_regulator_voltage_table_ops,
+ .n_voltages = ARRAY_SIZE(lm3631_ldo_target_vtbl),
+ .volt_table = lm3631_ldo_target_vtbl,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .vsel_reg = LM3631_REG_VOUT_OREF,
+ .vsel_mask = LM3631_VOUT_MASK,
+ .enable_reg = LM3631_REG_LDO_CTRL1,
+ .enable_mask = LM3631_EN_OREF_MASK,
+ },
+ {
+ .name = "ldo_vpos",
+ .id = LM3631_LDO_POS,
+ .ops = &lm3631_regulator_voltage_table_ops,
+ .n_voltages = ARRAY_SIZE(lm3631_ldo_target_vtbl),
+ .volt_table = lm3631_ldo_target_vtbl,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .vsel_reg = LM3631_REG_VOUT_POS,
+ .vsel_mask = LM3631_VOUT_MASK,
+ .enable_reg = LM3631_REG_LDO_CTRL1,
+ .enable_mask = LM3631_EN_VPOS_MASK,
+ },
+ {
+ .name = "ldo_vneg",
+ .id = LM3631_LDO_NEG,
+ .ops = &lm3631_regulator_voltage_table_ops,
+ .n_voltages = ARRAY_SIZE(lm3631_ldo_target_vtbl),
+ .volt_table = lm3631_ldo_target_vtbl,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .vsel_reg = LM3631_REG_VOUT_NEG,
+ .vsel_mask = LM3631_VOUT_MASK,
+ .enable_reg = LM3631_REG_LDO_CTRL1,
+ .enable_mask = LM3631_EN_VNEG_MASK,
+ },
+};
+
+static struct of_regulator_match lm3631_regulator_matches[] = {
+ { .name = "vboost", .driver_data = (void *)LM3631_BOOST, },
+ { .name = "vcont", .driver_data = (void *)LM3631_LDO_CONT, },
+ { .name = "voref", .driver_data = (void *)LM3631_LDO_OREF, },
+ { .name = "vpos", .driver_data = (void *)LM3631_LDO_POS, },
+ { .name = "vneg", .driver_data = (void *)LM3631_LDO_NEG, },
+};
+
+static struct regulator_init_data *
+lm3631_regulator_parse_dt(struct device *dev,
+ struct lm3631_regulator *lm3631_regulator, int id)
+{
+ struct device_node *node = dev->of_node;
+ int count;
+
+ count = of_regulator_match(dev, node, &lm3631_regulator_matches[id], 1);
+ if (count <= 0)
+ return ERR_PTR(-ENODEV);
+
+ return lm3631_regulator_matches[id].init_data;
+}
+
+static int lm3631_regulator_probe(struct platform_device *pdev)
+{
+ struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct lm3631_regulator *lm3631_regulator;
+ struct regulator_init_data *init_data;
+ struct regulator_config cfg = { };
+ struct regulator_dev *rdev;
+ int id = pdev->id;
+ int ret;
+
+ lm3631_regulator = devm_kzalloc(&pdev->dev, sizeof(*lm3631_regulator),
+ GFP_KERNEL);
+ if (!lm3631_regulator)
+ return -ENOMEM;
+
+ lm3631_regulator->lmu = lmu;
+
+ init_data = lmu->pdata->regulator_data[id];
+ if (!init_data) {
+ if (IS_ENABLED(CONFIG_OF)) {
+ init_data = lm3631_regulator_parse_dt(&pdev->dev,
+ lm3631_regulator,
+ id);
+ if (IS_ERR(init_data))
+ return PTR_ERR(init_data);
+ }
+ }
+
+ cfg.dev = pdev->dev.parent;
+ cfg.init_data = init_data;
+ cfg.driver_data = lm3631_regulator;
+ cfg.regmap = lmu->regmap;
+
+ rdev = regulator_register(&lm3631_regulator_desc[id], &cfg);
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);
+ dev_err(&pdev->dev, "[%d] regulator register err: %d\n",
+ id + 1, ret);
+ return ret;
+ }
+
+ lm3631_regulator->regulator = rdev;
+ platform_set_drvdata(pdev, lm3631_regulator);
+
+ return 0;
+}
+
+static int lm3631_regulator_remove(struct platform_device *pdev)
+{
+ struct lm3631_regulator *lm3631_regulator = platform_get_drvdata(pdev);
+
+ regulator_unregister(lm3631_regulator->regulator);
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lm3631_regulator_of_match[] = {
+ { .compatible = "ti,lm3631-regulator", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3631_regulator_of_match);
+#endif
+
+static struct platform_driver lm3631_regulator_driver = {
+ .probe = lm3631_regulator_probe,
+ .remove = lm3631_regulator_remove,
+ .driver = {
+ .name = "lm3631-regulator",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(lm3631_regulator_of_match),
+ },
+};
+
+module_platform_driver(lm3631_regulator_driver);
+
+MODULE_DESCRIPTION("TI LM3631 Regulator Driver");
+MODULE_AUTHOR("Milo Kim");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:lm3631-regulator");
--
1.7.9.5
^ permalink raw reply related
* [PATCH 10/10] Documentation: Add device tree bindings for TI LMU devices
From: Milo Kim @ 2014-02-14 6:32 UTC (permalink / raw)
To: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown
Cc: linux-kernel, devicetree, Samuel Ortiz, Milo Kim
Bindings for TI LMU, backlight, LM3631 regulator and LM3633 LED are added.
Cc: devicetree@vger.kernel.org
Cc: Bryan Wu <cooloney@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
.../devicetree/bindings/leds/leds-lm3633.txt | 39 +++++
Documentation/devicetree/bindings/mfd/ti-lmu.txt | 182 ++++++++++++++++++++
.../bindings/regulator/lm3631-regulator.txt | 49 ++++++
.../bindings/video/backlight/ti-lmu-backlight.txt | 127 ++++++++++++++
4 files changed, 397 insertions(+)
create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt
create mode 100644 Documentation/devicetree/bindings/mfd/ti-lmu.txt
create mode 100644 Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
create mode 100644 Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
new file mode 100644
index 0000000..4adeb62
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
@@ -0,0 +1,39 @@
+TI LMU LM3633 LED device tree bindings
+
+Required properties:
+ - compatible: "ti,lm3633-leds"
+ - lvled1-used, lvled2-used, lvled3-used, lvled4-used, lvled5-used, lvled6-used
+ : LED string configuration. Each child node should include this information
+ about which LED string is used.
+
+Optional properties:
+ - chan-name: LED channel name
+ - max-current-milliamp: Max current setting. Unit is mA.
+
+Example:
+
+lm3633@36 {
+ compatible = "ti,lm3633";
+ reg = <0x36>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ leds {
+ compatible = "ti,lm3633-leds";
+
+ chan2 {
+ chan-name = "status";
+ lvled2-used;
+ max-current-milliamp = /bits/ 8 <6>;
+ };
+
+ chan456 {
+ chan-name = "rgb";
+ lvled4-used;
+ lvled5-used;
+ lvled6-used;
+
+ max-current-milliamp = /bits/ 8 <5>;
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
new file mode 100644
index 0000000..2b3ecca
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -0,0 +1,182 @@
+TI LMU(Lighting Management Unit) device tree bindings
+
+TI LMU driver supports lighting devices belows.
+
+ Name Device tree properties
+ ------ ------------------------
+ LM3532 Backlight
+ LM3631 Backlight and regulator
+ LM3633 Backlight and LED
+ LM3695 Backlight
+ LM3697 Backlight
+
+Those have shared device tree properties.
+
+Required properties:
+ - compatible: "ti,lm3532", "ti,lm3631", "ti,lm3633", "ti,lm3695", "ti,lm3697"
+ - reg: I2C slave address.
+ 0x38 is LM3532
+ 0x29 is LM3631
+ 0x36 is LM3633, LM3697
+ 0x63 is LM3695
+ - ti,enable-gpio: GPIO number of hardware enable pin
+
+For the TI LMU backlight properties, please refer to:
+Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
+
+For the LM3631 regulator properties, please refer to:
+Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
+
+For the LM3633 LED properties, please refer to:
+Documentation/devicetree/bindings/leds/leds-lm3633.txt
+
+Examples:
+
+lm3532@38 {
+ compatible = "ti,lm3532";
+ reg = <0x38>;
+
+ /* GPIO134 for HWEN pin */
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3532-backlight";
+
+ lcd {
+ hvled1-used;
+ hvled2-used;
+ hvled3-used;
+
+ max-current-milliamp = /bits/ 8 <20>;
+ ramp-up = <1>;
+ ramp-down = <1>;
+ };
+ };
+};
+
+lm3631@29 {
+ compatible = "ti,lm3631";
+ reg = <0x29>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ /* Only Vpos and Vneg are used with LCD boost */
+ regulators {
+ compatible = "ti,lm3631-regulator";
+
+ vboost {
+ regulator-name = "lcd_boost";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <6350000>;
+ regulator-always-on;
+ };
+
+ vpos {
+ regulator-name = "lcd_vpos";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6000000>;
+ regulator-boot-on;
+ };
+
+ vneg {
+ regulator-name = "lcd_vneg";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6000000>;
+ regulator-boot-on;
+ };
+ };
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3631-backlight";
+
+ lcd_bl {
+ bl-name = "lcd";
+ hvled1-used;
+ hvled2-used;
+ ramp-up = <100>;
+ };
+ };
+};
+
+lm3633@36 {
+ compatible = "ti,lm3633";
+ reg = <0x36>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3633-backlight";
+
+ main {
+ bl-name = "main_lcd";
+ hvled2-used;
+ hvled3-used;
+ max-current-milliamp = /bits/ 8 <20>;
+ };
+
+ front {
+ bl-name = "front_lcd";
+ hvled1-used;
+ max-current-milliamp = /bits/ 8 <10>;
+ };
+ };
+
+ leds {
+ compatible = "ti,lm3633-leds";
+
+ chan2 {
+ chan-name = "status";
+ lvled2-used;
+ max-current-milliamp = /bits/ 8 <6>;
+ };
+
+ chan456 {
+ chan-name = "rgb";
+ lvled4-used;
+ lvled5-used;
+ lvled6-used;
+
+ max-current-milliamp = /bits/ 8 <5>;
+ };
+ };
+};
+
+lm3695@63 {
+ compatible = "ti,lm3695";
+ reg = <0x63>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3695-backlight";
+
+ lcd {
+ hvled1-used;
+ hvled2-used;
+ };
+ };
+};
+
+lm3697@36 {
+ compatible = "ti,lm3697";
+ reg = <0x36>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3697-backlight";
+
+ lcd_bl {
+ bl-name = "lcd";
+ hvled1-used;
+ hvled2-used;
+ hvled3-used;
+
+ max-current-milliamp = /bits/ 8 <20>;
+ initial-brightness = /bits/ 8 <10>;
+
+ ramp-up = <500>;
+ ramp-down = <500>;
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/regulator/lm3631-regulator.txt b/Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
new file mode 100644
index 0000000..e090076
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
@@ -0,0 +1,49 @@
+TI LMU LM3631 regulator device tree bindings
+
+Required properties:
+ - compatible: "ti,lm3631-regulator"
+
+Optional properties:
+ - regulator-name
+ - regulator-min-microvolt
+ - regulator-max-microvolt
+ - regulator-always-on
+ - regulator-boot-on
+
+ For those properties, please refer to:
+ Documentation/devicetree/bindings/regulator/regulator.txt
+
+Example:
+
+lm3631@29 {
+ compatible = "ti,lm3631";
+ reg = <0x29>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ /* Only VPOS and VNEG are used with LCD boost */
+ regulators {
+ compatible = "ti,lm3631-regulator";
+
+ vboost {
+ regulator-name = "lcd_boost";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <6350000>;
+ regulator-always-on;
+ };
+
+ vpos {
+ regulator-name = "lcd_vpos";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6000000>;
+ regulator-boot-on;
+ };
+
+ vneg {
+ regulator-name = "lcd_vneg";
+ regulator-min-microvolt = <4000000>;
+ regulator-max-microvolt = <6000000>;
+ regulator-boot-on;
+ };
+ };
+};
diff --git a/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
new file mode 100644
index 0000000..554ddca
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
@@ -0,0 +1,127 @@
+TI LMU backlight device tree bindings
+
+Required properties:
+ - compatible: One of lists below with "ti,lmu-backlight" should be set.
+ "ti,lm3532-backlight"
+ "ti,lm3631-backlight"
+ "ti,lm3633-backlight"
+ "ti,lm3695-backlight"
+ "ti,lm3697-backlight"
+ - hvled1-used, hvled2-used, hvled3-used: Backlight string configuration.
+ Each backlight child node should include this information about
+ which backlight string is used.
+
+Optional properties
+ - bl-name: Backlight device name
+ - max-current-milliamp: Max current setting. Unit is mA.
+ - initial-brightness: Backlight initial brightness
+ - ramp-up: Light effect for ramp up rate. Unit is msec.
+ - ramp-down: Light effect for ramp down rate. Unit is msec.
+ - pwm-period: PWM period. Only valid for PWM brightness control mode.
+ - pwms, pwm-names: For the PWM user nodes, please refer to
+ Documentation/devicetree/bindings/pwm/pwm.txt
+
+Examples:
+
+lm3532@38 {
+ compatible = "ti,lm3532";
+ reg = <0x38>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3532-backlight";
+
+ lcd {
+ hvled1-used;
+ hvled2-used;
+ hvled3-used;
+
+ max-current-milliamp = /bits/ 8 <20>;
+ };
+ };
+};
+
+lm3631@29 {
+ compatible = "ti,lm3631";
+ reg = <0x29>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3631-backlight";
+
+ lcd_bl {
+ bl-name = "lcd";
+ hvled1-used;
+ hvled2-used;
+ ramp-up = <100>;
+ };
+ };
+};
+
+lm3633@36 {
+ compatible = "ti,lm3633";
+ reg = <0x36>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3633-backlight";
+
+ main {
+ bl-name = "main_lcd";
+ hvled2-used;
+ hvled3-used;
+ max-current-milliamp = /bits/ 8 <20>;
+ };
+
+ front {
+ bl-name = "front_lcd";
+ hvled1-used;
+ max-current-milliamp = /bits/ 8 <10>;
+ };
+ };
+};
+
+lm3695@63 {
+ compatible = "ti,lm3695";
+ reg = <0x63>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3695-backlight";
+
+ lcd {
+ hvled1-used;
+ hvled2-used;
+ };
+ };
+};
+
+lm3697@36 {
+ compatible = "ti,lm3697";
+ reg = <0x36>;
+
+ ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
+
+ pwms = <&pwm3943 1 10000>;
+ pwm-names = "lmu-backlight";
+ backlight {
+ compatible = "ti,lmu-backlight", "ti,lm3697-backlight";
+
+ lcd {
+ hvled1-used;
+ hvled2-used;
+ hvled3-used;
+
+ max-current-milliamp = /bits/ 8 <20>;
+ initial-brightness = /bits/ 8 <10>;
+
+ ramp-up = <500>;
+ ramp-down = <500>;
+ pwm-period = <10000>;
+ };
+ };
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next v5 00/10] Support for the Broadcom GENET driver
From: Florian Fainelli @ 2014-02-14 7:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, devicetree, cernekee, mark.rutland, romieu
In-Reply-To: <20140214.002901.293996041341946875.davem@davemloft.net>
Le 13/02/2014 21:29, David Miller a écrit :
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Thu, 13 Feb 2014 16:08:41 -0800
>
>> This patchset adds support for the Broadcom GENET Gigabit Ethernet MAC
>> controller. This controller is found on the Broadcom BCM7xxx Set Top Box
>> System-on-a-chips.
>>
>> Changes since v4:
>> - add dependency on CONFIG_OF
>>
>> Changes since v3:
>> - fixed Kconfig dependency on FIXED_PHY
>>
>> Changes since v2:
>> - dropped the patch that adds an "internal" phy-mode
>
> Series applied, thanks.
Great, thanks David!
--
Florian
^ permalink raw reply
* Re: [PATCH v2 2/2] ARM: sunxi: dt: Convert to the new i2c compatibles
From: Wolfram Sang @ 2014-02-14 7:44 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140213225248.GB15350@lukather>
[-- Attachment #1: Type: text/plain, Size: 670 bytes --]
> > For non-a10, That should be at least
> >
> > compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-a10-i2c";
> >
> > or
> >
> > compatible = "allwinner,sun4i-a13-i2c", "allwinner,sun4i-i2c";
> >
> > depending on the outcome above.
> >
> > Or is my knowledge outdated already?
> >
>
> Since they are strictly compatible, we don't need to introduce any
> different compatible string here.
You never know all errata in advance. From what I know, one should
always use the specfic naming first, and then the generic fallback. So,
in case a distinction is needed later (think errata), then one doesn't
need to change the devicetrees.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2] can: xilinx CAN controller support.
From: Michal Simek @ 2014-02-14 8:55 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
Kedareswara rao Appana
In-Reply-To: <52FD1B16.6020600@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]
Hi Marc,
>> + int waiting_ech_skb_num;
>> + int xcan_echo_skb_max_tx;
>> + int xcan_echo_skb_max_rx;
>> + struct napi_struct napi;
>> + spinlock_t ech_skb_lock;
>> + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>> + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>
> Please remove read_reg, write_reg, as long as there isn't any BE support
> in the driver, call them directly.
That's not entirely truth. If you look at Microblaze then you will see
that Microblaze can be BE and LE.
There is just missing endian detection which we will add to the next version.
But because MB io helper functions are broken for a while you should be
able to use this driver on both endianess.
btw: I would prefer to use ioread32 and ioread32be instead of readl.
Is it OK for you?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v2] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-14 9:04 UTC (permalink / raw)
To: monstr
Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
Kedareswara rao Appana
In-Reply-To: <52FDD9FE.8040908@monstr.eu>
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
On 02/14/2014 09:55 AM, Michal Simek wrote:
> Hi Marc,
>
>>> + int waiting_ech_skb_num;
>>> + int xcan_echo_skb_max_tx;
>>> + int xcan_echo_skb_max_rx;
>>> + struct napi_struct napi;
>>> + spinlock_t ech_skb_lock;
>>> + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>>> + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>>
>> Please remove read_reg, write_reg, as long as there isn't any BE support
>> in the driver, call them directly.
>
> That's not entirely truth. If you look at Microblaze then you will see
> that Microblaze can be BE and LE.
> There is just missing endian detection which we will add to the next version.
As far as I know the endianess of the kernel is fixed and known during
compile time. Correct me if I'm wrong. So there is no need for a runtime
detection of the endianess and so no need for {read,write}_reg function
pointers.
> But because MB io helper functions are broken for a while you should be
> able to use this driver on both endianess.
>
> btw: I would prefer to use ioread32 and ioread32be instead of readl.
> Is it OK for you?
Make it so. :)
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/3] dp83640: Get pin and master/slave configuration from DT
From: Richard Cochran @ 2014-02-14 9:06 UTC (permalink / raw)
To: David Miller
Cc: stefan.sorensen-usnHOLptxrsHrNJx0XZkJA,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140213.173957.2126489785330496380.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Thu, Feb 13, 2014 at 05:39:57PM -0500, David Miller wrote:
> You'll have to code this in a way such that people who currently specify
> this parameter keep working.
So, do module parameters count as user land ABI?
[ In any case, for this module, there must be a way to retain the
parameters and still offer DT, and letting DT have precedence. ]
People want to be able to configure the auxiliary functions on the
pins of their PTP devices. My preference for supporting this is:
1. additional ioctl on the PTP character device
2. ethtool ioctl
3. DT and/or ACPI
The first option seems best because that is how you activate the
auxiliary functions. I am working on something in this direction.
Can't we just drop the DT idea altogther?
> If you don't like it, be angry at whoever added this module parameter
> in the first place, not me. Module parameters are 99 times out of 100
> not the right way to configure something, really.
Yeah, I am the one, be angry with me.
Sorry,
Richard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] can: xilinx CAN controller support.
From: Michal Simek @ 2014-02-14 9:13 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: monstr, Kedareswara rao Appana, wg, michal.simek, grant.likely,
robh+dt, linux-can, netdev, linux-arm-kernel, linux-kernel,
devicetree, Kedareswara rao Appana
In-Reply-To: <52FDDC24.8070805@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]
On 02/14/2014 10:04 AM, Marc Kleine-Budde wrote:
> On 02/14/2014 09:55 AM, Michal Simek wrote:
>> Hi Marc,
>>
>>>> + int waiting_ech_skb_num;
>>>> + int xcan_echo_skb_max_tx;
>>>> + int xcan_echo_skb_max_rx;
>>>> + struct napi_struct napi;
>>>> + spinlock_t ech_skb_lock;
>>>> + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>>>> + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>>>
>>> Please remove read_reg, write_reg, as long as there isn't any BE support
>>> in the driver, call them directly.
>>
>> That's not entirely truth. If you look at Microblaze then you will see
>> that Microblaze can be BE and LE.
>> There is just missing endian detection which we will add to the next version.
>
> As far as I know the endianess of the kernel is fixed and known during
> compile time. Correct me if I'm wrong. So there is no need for a runtime
> detection of the endianess and so no need for {read,write}_reg function
> pointers.
Endianess of the kernel is fixed and know during compile time
but what it is not fixed is endianess of that IP at compile time.
On fpga you can use bridges, partial reconfiguration, etc where
the only solution which is run-time endian detection via registers.
For example: drivers/block/xsysace.c, drivers/spi/spi-xilinx.c, etc
>> But because MB io helper functions are broken for a while you should be
>> able to use this driver on both endianess.
>>
>> btw: I would prefer to use ioread32 and ioread32be instead of readl.
>> Is it OK for you?
>
> Make it so. :)
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v2] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-14 9:16 UTC (permalink / raw)
To: monstr
Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
Kedareswara rao Appana
In-Reply-To: <52FDDE30.8020405@monstr.eu>
[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]
On 02/14/2014 10:13 AM, Michal Simek wrote:
>>> That's not entirely truth. If you look at Microblaze then you will see
>>> that Microblaze can be BE and LE.
>>> There is just missing endian detection which we will add to the next version.
>>
>> As far as I know the endianess of the kernel is fixed and known during
>> compile time. Correct me if I'm wrong. So there is no need for a runtime
>> detection of the endianess and so no need for {read,write}_reg function
>> pointers.
>
> Endianess of the kernel is fixed and know during compile time
> but what it is not fixed is endianess of that IP at compile time.
>
> On fpga you can use bridges, partial reconfiguration, etc where
> the only solution which is run-time endian detection via registers.
>
> For example: drivers/block/xsysace.c, drivers/spi/spi-xilinx.c, etc
Okay, now I get it. You can make it more complex then it used to be :D
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* [PATCH v3] [media] of: move graph helpers from drivers/media/v4l2-core to drivers/media
From: Philipp Zabel @ 2014-02-14 9:18 UTC (permalink / raw)
To: Russell King - ARM Linux, Mauro Carvalho Chehab
Cc: Grant Likely, Rob Herring, Sylwester Nawrocki, Laurent Pinchart,
Tomi Valkeinen, Kyungmin Park,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Guennadi Liakhovetski,
Philipp Zabel, Philipp Zabel
From: Philipp Zabel <philipp.zabel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This patch moves the parsing helpers used to parse connected graphs
in the device tree, like the video interface bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt, from
drivers/media/v4l2-core to drivers/media.
This allows to reuse the same parser code from outside the V4L2
framework, most importantly from display drivers.
The functions v4l2_of_get_next_endpoint, v4l2_of_get_remote_port,
and v4l2_of_get_remote_port_parent are moved. They are renamed to
of_graph_get_next_endpoint, of_graph_get_remote_port, and
of_graph_get_remote_port_parent, respectively.
Since there are not that many current users yet, switch all of
them to the new functions right away.
Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Mauro Carvalho Chehab <m.chehab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
---
Changes since v2:
- Removed CONFIG_OF_GRAPH option, helpers are compiled in when
CONFIG_OF is enabled.
---
drivers/media/Makefile | 2 +
drivers/media/i2c/adv7343.c | 4 +-
drivers/media/i2c/mt9p031.c | 4 +-
drivers/media/i2c/s5k5baf.c | 3 +-
drivers/media/i2c/tvp514x.c | 3 +-
drivers/media/i2c/tvp7002.c | 3 +-
drivers/media/of_graph.c | 133 ++++++++++++++++++++++++++
drivers/media/platform/exynos4-is/fimc-is.c | 6 +-
drivers/media/platform/exynos4-is/media-dev.c | 3 +-
drivers/media/platform/exynos4-is/mipi-csis.c | 3 +-
drivers/media/v4l2-core/v4l2-of.c | 117 ----------------------
include/media/of_graph.h | 46 +++++++++
include/media/v4l2-of.h | 24 -----
13 files changed, 198 insertions(+), 153 deletions(-)
create mode 100644 drivers/media/of_graph.c
create mode 100644 include/media/of_graph.h
diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 620f275..0341472 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -18,6 +18,8 @@ ifeq ($(CONFIG_MEDIA_CONTROLLER),y)
obj-$(CONFIG_MEDIA_SUPPORT) += media.o
endif
+obj-$(CONFIG_OF) += of_graph.o
+
obj-$(CONFIG_VIDEO_DEV) += v4l2-core/
obj-$(CONFIG_DVB_CORE) += dvb-core/
diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
index d4e15a6..74a1507 100644
--- a/drivers/media/i2c/adv7343.c
+++ b/drivers/media/i2c/adv7343.c
@@ -28,10 +28,10 @@
#include <linux/of.h>
#include <media/adv7343.h>
+#include <media/of_graph.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
-#include <media/v4l2-of.h>
#include "adv7343_regs.h"
@@ -410,7 +410,7 @@ adv7343_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index e5ddf47..60f36dc 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -27,9 +27,9 @@
#include <linux/videodev2.h>
#include <media/mt9p031.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
-#include <media/v4l2-of.h>
#include <media/v4l2-subdev.h>
#include "aptina-pll.h"
@@ -943,7 +943,7 @@ mt9p031_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- np = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ np = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!np)
return NULL;
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 77e10e0..06261ee 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <media/media-entity.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
#include <media/v4l2-subdev.h>
@@ -1855,7 +1856,7 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
if (ret < 0)
return ret;
- node_ep = v4l2_of_get_next_endpoint(node, NULL);
+ node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
dev_err(dev, "no endpoint defined at node %s\n",
node->full_name);
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index 83d85df..50062d2 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -37,6 +37,7 @@
#include <linux/v4l2-mediabus.h>
#include <linux/of.h>
+#include <media/of_graph.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
#include <media/v4l2-common.h>
@@ -1068,7 +1069,7 @@ tvp514x_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
index 912e1cc..7b3201b 100644
--- a/drivers/media/i2c/tvp7002.c
+++ b/drivers/media/i2c/tvp7002.c
@@ -31,6 +31,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/v4l2-dv-timings.h>
+#include <media/of_graph.h>
#include <media/tvp7002.h>
#include <media/v4l2-async.h>
#include <media/v4l2-device.h>
@@ -957,7 +958,7 @@ tvp7002_get_pdata(struct i2c_client *client)
if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
return client->dev.platform_data;
- endpoint = v4l2_of_get_next_endpoint(client->dev.of_node, NULL);
+ endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
if (!endpoint)
return NULL;
diff --git a/drivers/media/of_graph.c b/drivers/media/of_graph.c
new file mode 100644
index 0000000..aa526d7
--- /dev/null
+++ b/drivers/media/of_graph.c
@@ -0,0 +1,133 @@
+/*
+ * OF graph binding parsing library
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/types.h>
+
+/**
+ * of_graph_get_next_endpoint() - get next endpoint node
+ * @parent: pointer to the parent device node
+ * @prev: previous endpoint node, or NULL to get first
+ *
+ * Return: An 'endpoint' node pointer with refcount incremented. Refcount
+ * of the passed @prev node is not decremented, the caller have to use
+ * of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *prev)
+{
+ struct device_node *endpoint;
+ struct device_node *port = NULL;
+
+ if (!parent)
+ return NULL;
+
+ if (!prev) {
+ struct device_node *node;
+ /*
+ * It's the first call, we have to find a port subnode
+ * within this node or within an optional 'ports' node.
+ */
+ node = of_get_child_by_name(parent, "ports");
+ if (node)
+ parent = node;
+
+ port = of_get_child_by_name(parent, "port");
+
+ if (port) {
+ /* Found a port, get an endpoint. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ } else {
+ endpoint = NULL;
+ }
+
+ if (!endpoint)
+ pr_err("%s(): no endpoint nodes specified for %s\n",
+ __func__, parent->full_name);
+ of_node_put(node);
+ } else {
+ port = of_get_parent(prev);
+ if (!port)
+ /* Hm, has someone given us the root node ?... */
+ return NULL;
+
+ /* Avoid dropping prev node refcount to 0. */
+ of_node_get(prev);
+ endpoint = of_get_next_child(port, prev);
+ if (endpoint) {
+ of_node_put(port);
+ return endpoint;
+ }
+
+ /* No more endpoints under this port, try the next one. */
+ do {
+ port = of_get_next_child(parent, port);
+ if (!port)
+ return NULL;
+ } while (of_node_cmp(port->name, "port"));
+
+ /* Pick up the first endpoint in this port. */
+ endpoint = of_get_next_child(port, NULL);
+ of_node_put(port);
+ }
+
+ return endpoint;
+}
+EXPORT_SYMBOL(of_graph_get_next_endpoint);
+
+/**
+ * of_graph_get_remote_port_parent() - get remote port's parent node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote device node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ struct device_node *np;
+ unsigned int depth;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+
+ /* Walk 3 levels up only if there is 'ports' node. */
+ for (depth = 3; depth && np; depth--) {
+ np = of_get_next_parent(np);
+ if (depth == 2 && of_node_cmp(np->name, "ports"))
+ break;
+ }
+ return np;
+}
+EXPORT_SYMBOL(of_graph_get_remote_port_parent);
+
+/**
+ * of_graph_get_remote_port() - get remote port node
+ * @node: pointer to a local endpoint device_node
+ *
+ * Return: Remote port node associated with remote endpoint node linked
+ * to @node. Use of_node_put() on it when done.
+ */
+struct device_node *of_graph_get_remote_port(const struct device_node *node)
+{
+ struct device_node *np;
+
+ /* Get remote endpoint node. */
+ np = of_parse_phandle(node, "remote-endpoint", 0);
+ if (!np)
+ return NULL;
+ return of_get_next_parent(np);
+}
+EXPORT_SYMBOL(of_graph_get_remote_port);
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 13a4228..6405268 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -30,7 +30,7 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/videodev2.h>
-#include <media/v4l2-of.h>
+#include <media/of_graph.h>
#include <media/videobuf2-dma-contig.h>
#include "media-dev.h"
@@ -167,10 +167,10 @@ static int fimc_is_parse_sensor_config(struct fimc_is_sensor *sensor,
u32 tmp = 0;
int ret;
- np = v4l2_of_get_next_endpoint(np, NULL);
+ np = of_graph_get_next_endpoint(np, NULL);
if (!np)
return -ENXIO;
- np = v4l2_of_get_remote_port(np);
+ np = of_graph_get_remote_port(np);
if (!np)
return -ENXIO;
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index c1bce17..11219e2 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -24,6 +24,7 @@
#include <linux/pm_runtime.h>
#include <linux/types.h>
#include <linux/slab.h>
+#include <media/of_graph.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-of.h>
#include <media/media-device.h>
@@ -473,7 +474,7 @@ static int fimc_md_parse_port_node(struct fimc_md *fmd,
pd->mux_id = (endpoint.port - 1) & 0x1;
- rem = v4l2_of_get_remote_port_parent(ep);
+ rem = of_graph_get_remote_port_parent(ep);
of_node_put(ep);
if (rem == NULL) {
v4l2_info(&fmd->v4l2_dev, "Remote device at %s not found\n",
diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
index f3c3591..7211523 100644
--- a/drivers/media/platform/exynos4-is/mipi-csis.c
+++ b/drivers/media/platform/exynos4-is/mipi-csis.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/videodev2.h>
+#include <media/of_graph.h>
#include <media/s5p_fimc.h>
#include <media/v4l2-of.h>
#include <media/v4l2-subdev.h>
@@ -762,7 +763,7 @@ static int s5pcsis_parse_dt(struct platform_device *pdev,
&state->max_num_lanes))
return -EINVAL;
- node = v4l2_of_get_next_endpoint(node, NULL);
+ node = of_graph_get_next_endpoint(node, NULL);
if (!node) {
dev_err(&pdev->dev, "No port node at %s\n",
pdev->dev.of_node->full_name);
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index 42e3e8a..f919db3 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -152,120 +152,3 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
return 0;
}
EXPORT_SYMBOL(v4l2_of_parse_endpoint);
-
-/**
- * v4l2_of_get_next_endpoint() - get next endpoint node
- * @parent: pointer to the parent device node
- * @prev: previous endpoint node, or NULL to get first
- *
- * Return: An 'endpoint' node pointer with refcount incremented. Refcount
- * of the passed @prev node is not decremented, the caller have to use
- * of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
- struct device_node *prev)
-{
- struct device_node *endpoint;
- struct device_node *port = NULL;
-
- if (!parent)
- return NULL;
-
- if (!prev) {
- struct device_node *node;
- /*
- * It's the first call, we have to find a port subnode
- * within this node or within an optional 'ports' node.
- */
- node = of_get_child_by_name(parent, "ports");
- if (node)
- parent = node;
-
- port = of_get_child_by_name(parent, "port");
-
- if (port) {
- /* Found a port, get an endpoint. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- } else {
- endpoint = NULL;
- }
-
- if (!endpoint)
- pr_err("%s(): no endpoint nodes specified for %s\n",
- __func__, parent->full_name);
- of_node_put(node);
- } else {
- port = of_get_parent(prev);
- if (!port)
- /* Hm, has someone given us the root node ?... */
- return NULL;
-
- /* Avoid dropping prev node refcount to 0. */
- of_node_get(prev);
- endpoint = of_get_next_child(port, prev);
- if (endpoint) {
- of_node_put(port);
- return endpoint;
- }
-
- /* No more endpoints under this port, try the next one. */
- do {
- port = of_get_next_child(parent, port);
- if (!port)
- return NULL;
- } while (of_node_cmp(port->name, "port"));
-
- /* Pick up the first endpoint in this port. */
- endpoint = of_get_next_child(port, NULL);
- of_node_put(port);
- }
-
- return endpoint;
-}
-EXPORT_SYMBOL(v4l2_of_get_next_endpoint);
-
-/**
- * v4l2_of_get_remote_port_parent() - get remote port's parent node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote device node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- struct device_node *np;
- unsigned int depth;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
-
- /* Walk 3 levels up only if there is 'ports' node. */
- for (depth = 3; depth && np; depth--) {
- np = of_get_next_parent(np);
- if (depth == 2 && of_node_cmp(np->name, "ports"))
- break;
- }
- return np;
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port_parent);
-
-/**
- * v4l2_of_get_remote_port() - get remote port node
- * @node: pointer to a local endpoint device_node
- *
- * Return: Remote port node associated with remote endpoint node linked
- * to @node. Use of_node_put() on it when done.
- */
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node)
-{
- struct device_node *np;
-
- /* Get remote endpoint node. */
- np = of_parse_phandle(node, "remote-endpoint", 0);
- if (!np)
- return NULL;
- return of_get_next_parent(np);
-}
-EXPORT_SYMBOL(v4l2_of_get_remote_port);
diff --git a/include/media/of_graph.h b/include/media/of_graph.h
new file mode 100644
index 0000000..3bbeb60
--- /dev/null
+++ b/include/media/of_graph.h
@@ -0,0 +1,46 @@
+/*
+ * OF graph binding parsing helpers
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Author: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski <g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#ifndef __LINUX_OF_GRAPH_H
+#define __LINUX_OF_GRAPH_H
+
+#ifdef CONFIG_OF
+struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
+ struct device_node *previous);
+struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node);
+struct device_node *of_graph_get_remote_port(const struct device_node *node);
+#else
+
+static inline struct device_node *of_graph_get_next_endpoint(
+ const struct device_node *parent,
+ struct device_node *previous)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port_parent(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+static inline struct device_node *of_graph_get_remote_port(
+ const struct device_node *node)
+{
+ return NULL;
+}
+
+#endif /* CONFIG_OF */
+
+#endif /* __LINUX_OF_GRAPH_H */
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 541cea4..8174282 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -72,11 +72,6 @@ struct v4l2_of_endpoint {
#ifdef CONFIG_OF
int v4l2_of_parse_endpoint(const struct device_node *node,
struct v4l2_of_endpoint *endpoint);
-struct device_node *v4l2_of_get_next_endpoint(const struct device_node *parent,
- struct device_node *previous);
-struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node);
-struct device_node *v4l2_of_get_remote_port(const struct device_node *node);
#else /* CONFIG_OF */
static inline int v4l2_of_parse_endpoint(const struct device_node *node,
@@ -85,25 +80,6 @@ static inline int v4l2_of_parse_endpoint(const struct device_node *node,
return -ENOSYS;
}
-static inline struct device_node *v4l2_of_get_next_endpoint(
- const struct device_node *parent,
- struct device_node *previous)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port_parent(
- const struct device_node *node)
-{
- return NULL;
-}
-
-static inline struct device_node *v4l2_of_get_remote_port(
- const struct device_node *node)
-{
- return NULL;
-}
-
#endif /* CONFIG_OF */
#endif /* _V4L2_OF_H */
--
1.9.0.rc3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net-next v5 00/10] Support for the Broadcom GENET driver
From: Richard Cochran @ 2014-02-14 9:19 UTC (permalink / raw)
To: Florian Fainelli
Cc: David Miller, netdev, devicetree, cernekee, mark.rutland, romieu
In-Reply-To: <52FDC094.1020104@gmail.com>
On Thu, Feb 13, 2014 at 11:07:00PM -0800, Florian Fainelli wrote:
> Le 13/02/2014 21:29, David Miller a écrit :
> >Series applied, thanks.
>
> Great, thanks David!
Sorry for the late comment, but how about adding a call to
skb_tx_timestamp(), in order to support so_timestamping?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 4/6] net: cpsw: Use cpsw-ctrl-macid driver
From: Markus Pargmann @ 2014-02-14 9:23 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: David S. Miller, Benoît Cousson, linux-omap, devicetree,
linux-arm-kernel, kernel
In-Reply-To: <20140213193702.GE17650@pengutronix.de>
Hi Uwe,
On Thu, Feb 13, 2014 at 08:37:02PM +0100, Uwe Kleine-König wrote:
> Hello Markus,
>
> On Wed, Dec 18, 2013 at 05:47:20PM +0100, Markus Pargmann wrote:
> > Use ctrl-macid driver to obtain the macids stored in the processor. This
> > is only done when defined in DT.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> > Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++
> > drivers/net/ethernet/ti/cpsw.c | 18 ++++++++++++++----
> > drivers/net/ethernet/ti/cpsw.h | 2 ++
> > 3 files changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> > index c39f077..b95c38b 100644
> > --- a/Documentation/devicetree/bindings/net/cpsw.txt
> > +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> > @@ -34,6 +34,11 @@ Required properties:
> > Optional properties:
> > - dual_emac_res_vlan : Specifies VID to be used to segregate the ports
> > - mac-address : Specifies slave MAC address
> > +- ti,mac-address-ctrl : When cpsw-ctrl-macid support is compiledin, this can
> > + be set to a phandle with one argument, see
> > + cpsw-ctrl-macid.txt. If this method fails, cpsw falls
> > + back to mac-address or random mac-address.
> > +
> >
> > Note: "ti,hwmods" field is used to fetch the base address and irq
> > resources from TI, omap hwmod data base during device registration.
> > diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> > index 5120d9c..382d793 100644
> > --- a/drivers/net/ethernet/ti/cpsw.c
> > +++ b/drivers/net/ethernet/ti/cpsw.c
> > @@ -1804,9 +1804,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
> > snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
> > PHY_ID_FMT, mdio->name, phyid);
> >
> > - mac_addr = of_get_mac_address(slave_node);
> > - if (mac_addr)
> > - memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
> > + ret = cpsw_ctrl_macid_read(slave_node, slave_data->mac_addr);
> > + if (ret) {
> > + if (ret == -EPROBE_DEFER)
> > + return ret;
> > +
> > + mac_addr = of_get_mac_address(slave_node);
> > + if (mac_addr)
> > + memcpy(slave_data->mac_addr, mac_addr,
> > + ETH_ALEN);
> > + }
> I'd do it the other way round: Use the contents from an explicit
> "mac-address" or "local-mac-address" property (i.e. of_get_mac_address)
> and if that doesn't return anything use the mac-address-ctrl as
> fallback.
Yes you are right. In this case this wouldn't even influence any boots
with u-boot which already set the correct mac-address property.
I will fix this.
Thanks,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/6] net: cpsw: Add control-module macid driver
From: Markus Pargmann @ 2014-02-14 9:32 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: David S. Miller, Benoît Cousson, linux-omap, devicetree,
linux-arm-kernel, kernel
In-Reply-To: <20140213194431.GF17650@pengutronix.de>
Hi,
On Thu, Feb 13, 2014 at 08:44:31PM +0100, Uwe Kleine-König wrote:
> On Wed, Dec 18, 2013 at 05:47:19PM +0100, Markus Pargmann wrote:
> > This driver extracts the hardware macid from the control module of
> > am335x processors. It exports a function cpsw_ctrl_macid_read for cpsw
> > to get the macid from within the processor.
> >
> > This driver is not used, unless it is defined in DT and referenced by a
> > cpsw slave with a phandle.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> > .../devicetree/bindings/net/cpsw-ctrl-macid.txt | 31 +++++
> > drivers/net/ethernet/ti/Kconfig | 8 ++
> > drivers/net/ethernet/ti/Makefile | 1 +
> > drivers/net/ethernet/ti/cpsw-ctrl-macid.c | 138 +++++++++++++++++++++
> > 4 files changed, 178 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > create mode 100644 drivers/net/ethernet/ti/cpsw-ctrl-macid.c
> >
> > diff --git a/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > new file mode 100644
> > index 0000000..abff2af
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/cpsw-ctrl-macid.txt
> > @@ -0,0 +1,31 @@
> > +TI CPSW ctrl macid Devicetree bindings
> > +--------------------------------------
> > +
> > +Required properties:
> > + - compatible : Should be "ti,am3352-cpsw-ctrl-macid"
> > + - reg : physical base address and size of the cpsw
> > + registers map
> > + - reg-names : names of the register map given in "reg" node
> > + - #ti,cpsw-ctrl-macid : Should be <1>
> #ti,mac-address-ctrl-cells?
Sounds better, will fix.
>
> > +
> > +When used from cpsw, "ti,mac-address-ctrl" should be a phandle to this device
> > +node with one argument, 0 or 1 to select the macid 0 or 1.
> > +
> > +Examples:
> > +
> > + cpsw_ctrl_macid: cpsw-ctrl-macid@44e10630 {
> > + compatible = "ti,am3352-cpsw-ctrl-macid";
> > + #ti,mac-address-ctrl-cells = <1>;
> > + reg = <0x44e10630 0x16>;
> s/0x16/0x10/
Thanks, that's a bug, obviously we only have 4, not 5.5 registers.
>
> > + reg-names = "ctrl-macid";
> > + };
> > +
> > +Used in cpsw slave nodes like this:
> > +
> > + cpsw_emac0: slave@4a100200 {
> > + ti,mac-address-ctrl = <&cpsw_ctrl_macid 0>;
> > + };
> > +
> > + cpsw_emac1: slave@4a100300 {
> > + ti,mac-address-ctrl = <&cpsw_ctrl_macid 1>;
> > + };
> > diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> > index 53150c2..24819ef 100644
> > --- a/drivers/net/ethernet/ti/Kconfig
> > +++ b/drivers/net/ethernet/ti/Kconfig
> > @@ -56,12 +56,20 @@ config TI_CPSW_PHY_SEL
> > This driver supports configuring of the phy mode connected to
> > the CPSW.
> >
> > +config TI_CPSW_CTRL_MACID
> > + boolean "TI CPSW internal MACID support"
> > + depends on TI_CPSW
> > + ---help---
> > + This driver supports reading the hardcoded MACID from am33xx
> > + processors control module.
> > +
> Would it be nicer to put this after the TI_CPSW definition. (Think
> $(make config).)
I inserted TI_CPSW_CTRL_MACID here because the other TI_CPSW specific
subdriver (TI_CPSW_PHY_SEL) was above TI_CPSW. But I could change this.
>
> > config TI_CPSW
> > tristate "TI CPSW Switch Support"
> > depends on ARM && (ARCH_DAVINCI || SOC_AM33XX)
> > select TI_DAVINCI_CPDMA
> > select TI_DAVINCI_MDIO
> > select TI_CPSW_PHY_SEL
> > + select TI_CPSW_CTRL_MACID
> If TI_CPSW selects TI_CPSW_CTRL_MACID the latter doesn't need to depend
> on the former. So this optin is user visible but never
> user-(de)selectable. I'd say drop the Kconfig symbol and just add
> cpsw-ctrl-macid.o to ti_cpsw-y in the Makefile (or really make it
> optional).
As this is closely related to the cpsw driver, I think it's better to
make it non-optional and include it in the Makefile.
Thanks,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH v2] can: xilinx CAN controller support.
From: Appana Durga Kedareswara Rao @ 2014-02-14 9:36 UTC (permalink / raw)
To: Marc Kleine-Budde, wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org,
Michal Simek,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <52FD1B16.6020600-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Friday, February 14, 2014 12:51 AM
> To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek;
> grant.likely@linaro.org; robh+dt@kernel.org; linux-can@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; Appana Durga
> Kedareswara Rao
> Subject: Re: [PATCH v2] can: xilinx CAN controller support.
>
> On 02/12/2014 08:10 AM, Kedareswara rao Appana wrote:
> > This patch adds xilinx CAN controller support.
> > This driver supports both ZYNQ CANPS IP and Soft IP AXI CAN
> > controller.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> > This patch is rebased on the 3.14 rc2 kernel.
> > Changes for v2:
> > - Updated with the review comments.
> > - Removed unnecessary debug prints.
> > - included tx,rx fifo depths in ZYNQ CANPS case also.
> > ---
> > .../devicetree/bindings/net/can/xilinx_can.txt | 45 +
> > drivers/net/can/Kconfig | 7 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/xilinx_can.c | 1153 ++++++++++++++++++++
> > 4 files changed, 1206 insertions(+), 0 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > create mode 100644 drivers/net/can/xilinx_can.c
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > new file mode 100644
> > index 0000000..0e57103
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > @@ -0,0 +1,45 @@
> > +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings
> > +---------------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : Should be "xlnx,zynq-can-1.00.a" for Zynq
> CAN
> > + controllers and "xlnx,axi-can-1.00.a" for Axi CAN
> > + controllers.
> > +- reg : Physical base address and size of the Axi CAN/Zynq
> > + CANPS registers map.
> > +- interrupts : Property with a value describing the interrupt
> > + number.
> > +- interrupt-parent : Must be core interrupt controller
> > +- clock-names : List of input clock names - "ref_clk",
> "aper_clk"
> > + (See clock bindings for details. Two clocks are
> > + required for Zynq CAN. For Axi CAN
> > + case it is one(ref_clk)).
> > +- clocks : Clock phandles (see clock bindings for details).
> > +- tx-fifo-depth : Can Tx fifo depth.
> > +- rx-fifo-depth : Can Rx fifo depth.
> > +
> > +
> > +Example:
> > +
> > +For Zynq CANPS Dts file:
> > + zynq_can_0: zynq-can@e0008000 {
> > + compatible = "xlnx,zynq-can-1.00.a";
> > + clocks = <&clkc 19>, <&clkc 36>;
> > + clock-names = "ref_clk", "aper_clk";
> > + reg = <0xe0008000 0x1000>;
> > + interrupts = <0 28 4>;
> > + interrupt-parent = <&intc>;
> > + tx-fifo-depth = <0x40>;
> > + rx-fifo-depth = <0x40>;
> > + };
> > +For Axi CAN Dts file:
> > + axi_can_0: axi-can@40000000 {
> > + compatible = "xlnx,axi-can-1.00.a";
> > + clocks = <&clkc 0>;
> > + clock-names = "ref_clk" ;
> > + reg = <0x40000000 0x10000>;
> > + interrupt-parent = <&intc>;
> > + interrupts = <0 59 1>;
> > + tx-fifo-depth = <0x40>;
> > + rx-fifo-depth = <0x40>;
> > + };
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > 9e7d95d..b180239 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -125,6 +125,13 @@ config CAN_GRCAN
> > endian syntheses of the cores would need some modifications on
> > the hardware level to work.
> >
> > +config CAN_XILINXCAN
> > + tristate "Xilinx CAN"
> > + depends on ARCH_ZYNQ || MICROBLAZE
> > + ---help---
> > + Xilinx CAN driver. This driver supports both soft AXI CAN IP and
> > + Zynq CANPS IP.
> > +
> > source "drivers/net/can/mscan/Kconfig"
> >
> > source "drivers/net/can/sja1000/Kconfig"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index
> > c744039..0b8e11e 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-
> ican3.o
> > obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
> > obj-$(CONFIG_PCH_CAN) += pch_can.o
> > obj-$(CONFIG_CAN_GRCAN) += grcan.o
> > +obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
> >
> > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git
> > a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c new file
> > mode 100644 index 0000000..642e6b4
> > --- /dev/null
> > +++ b/drivers/net/can/xilinx_can.c
> > @@ -0,0 +1,1153 @@
> > +/* Xilinx CAN device driver
> > + *
> > + * Copyright (C) 2012 - 2014 Xilinx, Inc.
> > + * Copyright (C) 2009 PetaLogix. All rights reserved.
> > + *
> > + * Description:
> > + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
> > + * This program is free software: you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation, either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/skbuff.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +#include <linux/can/led.h>
> > +
> > +#define DRIVER_NAME "XILINX_CAN"
> > +
> > +/* CAN registers set */
> > +#define XCAN_SRR_OFFSET 0x00 /* Software reset */
> > +#define XCAN_MSR_OFFSET 0x04 /* Mode select */
> > +#define XCAN_BRPR_OFFSET 0x08 /* Baud rate prescaler */
> > +#define XCAN_BTR_OFFSET 0x0C /* Bit timing */
> > +#define XCAN_ECR_OFFSET 0x10 /* Error counter */
> > +#define XCAN_ESR_OFFSET 0x14 /* Error status */
> > +#define XCAN_SR_OFFSET 0x18 /* Status */
> > +#define XCAN_ISR_OFFSET 0x1C /* Interrupt status */
> > +#define XCAN_IER_OFFSET 0x20 /* Interrupt enable */
> > +#define XCAN_ICR_OFFSET 0x24 /* Interrupt clear */
> > +#define XCAN_TXFIFO_ID_OFFSET 0x30 /* TX FIFO ID */
> > +#define XCAN_TXFIFO_DLC_OFFSET 0x34 /* TX FIFO DLC */
> > +#define XCAN_TXFIFO_DW1_OFFSET 0x38 /* TX FIFO Data
> Word 1 */
> > +#define XCAN_TXFIFO_DW2_OFFSET 0x3C /* TX FIFO Data
> Word 2 */
> > +#define XCAN_RXFIFO_ID_OFFSET 0x50 /* RX FIFO ID */
> > +#define XCAN_RXFIFO_DLC_OFFSET 0x54 /* RX FIFO DLC */
> > +#define XCAN_RXFIFO_DW1_OFFSET 0x58 /* RX FIFO Data
> Word 1 */
> > +#define XCAN_RXFIFO_DW2_OFFSET 0x5C /* RX FIFO Data
> Word 2 */
>
> Can you define all register offsets via an enum please.
>
Ok
> > +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
> > +#define XCAN_SRR_CEN_MASK 0x00000002 /* CAN enable */
> > +#define XCAN_SRR_RESET_MASK 0x00000001 /* Soft Reset the
> CAN core */
> > +#define XCAN_MSR_LBACK_MASK 0x00000002 /* Loop back
> mode select */
> > +#define XCAN_MSR_SLEEP_MASK 0x00000001 /* Sleep mode
> select */
> > +#define XCAN_BRPR_BRP_MASK 0x000000FF /* Baud rate
> prescaler */
> > +#define XCAN_BTR_SJW_MASK 0x00000180 /* Synchronous
> jump width */
> > +#define XCAN_BTR_TS2_MASK 0x00000070 /* Time segment
> 2 */
> > +#define XCAN_BTR_TS1_MASK 0x0000000F /* Time segment
> 1 */
> > +#define XCAN_ECR_REC_MASK 0x0000FF00 /* Receive error
> counter */
> > +#define XCAN_ECR_TEC_MASK 0x000000FF /* Transmit error
> counter */
> > +#define XCAN_ESR_ACKER_MASK 0x00000010 /* ACK error */
> > +#define XCAN_ESR_BERR_MASK 0x00000008 /* Bit error */
> > +#define XCAN_ESR_STER_MASK 0x00000004 /* Stuff error */
> > +#define XCAN_ESR_FMER_MASK 0x00000002 /* Form error */
> > +#define XCAN_ESR_CRCER_MASK 0x00000001 /* CRC error */
> > +#define XCAN_SR_TXFLL_MASK 0x00000400 /* TX FIFO is full
> */
> > +#define XCAN_SR_ESTAT_MASK 0x00000180 /* Error status */
> > +#define XCAN_SR_ERRWRN_MASK 0x00000040 /* Error warning
> */
> > +#define XCAN_SR_NORMAL_MASK 0x00000008 /* Normal mode
> */
> > +#define XCAN_SR_LBACK_MASK 0x00000002 /* Loop back
> mode */
> > +#define XCAN_SR_CONFIG_MASK 0x00000001 /* Configuration
> mode */
> > +#define XCAN_IXR_TXFEMP_MASK 0x00004000 /* TX FIFO Empty
> */
> > +#define XCAN_IXR_WKUP_MASK 0x00000800 /* Wake up
> interrupt */
> > +#define XCAN_IXR_SLP_MASK 0x00000400 /* Sleep
> interrupt */
> > +#define XCAN_IXR_BSOFF_MASK 0x00000200 /* Bus off
> interrupt */
> > +#define XCAN_IXR_ERROR_MASK 0x00000100 /* Error interrupt
> */
> > +#define XCAN_IXR_RXNEMP_MASK 0x00000080 /* RX FIFO
> NotEmpty intr */
> > +#define XCAN_IXR_RXOFLW_MASK 0x00000040 /* RX FIFO
> Overflow intr */
> > +#define XCAN_IXR_RXOK_MASK 0x00000010 /* Message
> received intr */
> > +#define XCAN_IXR_TXOK_MASK 0x00000002 /* TX successful
> intr */
> > +#define XCAN_IXR_ARBLST_MASK 0x00000001 /* Arbitration
> lost intr */
> > +#define XCAN_IDR_ID1_MASK 0xFFE00000 /* Standard msg
> identifier */
> > +#define XCAN_IDR_SRR_MASK 0x00100000 /* Substitute
> remote TXreq */
> > +#define XCAN_IDR_IDE_MASK 0x00080000 /* Identifier
> extension */
> > +#define XCAN_IDR_ID2_MASK 0x0007FFFE /* Extended
> message ident */
> > +#define XCAN_IDR_RTR_MASK 0x00000001 /* Remote TX
> request */
> > +#define XCAN_DLCR_DLC_MASK 0xF0000000 /* Data length
> code */
> > +
Need to use BIT() Macro for the Masks?
> > +#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK |
> XCAN_IXR_BSOFF_MASK |\
> > + XCAN_IXR_WKUP_MASK |
> XCAN_IXR_SLP_MASK | \
> > + XCAN_IXR_RXNEMP_MASK |
> XCAN_IXR_ERROR_MASK | \
> > + XCAN_IXR_ARBLST_MASK |
> XCAN_IXR_RXOK_MASK)
> > +
> > +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
> > +#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width
> */
> > +#define XCAN_BTR_TS2_SHIFT 4 /* Time segment 2 */
> > +#define XCAN_IDR_ID1_SHIFT 21 /* Standard Messg
> Identifier */
> > +#define XCAN_IDR_ID2_SHIFT 1 /* Extended Message
> Identifier */
> > +#define XCAN_DLCR_DLC_SHIFT 28 /* Data length code */
> > +#define XCAN_ESR_REC_SHIFT 8 /* Rx Error Count */
> > +
> > +/* CAN frame length constants */
> > +#define XCAN_ECHO_SKB_MAX 64
> > +#define XCAN_FRAME_MAX_DATA_LEN 8
> > +#define XCAN_TIMEOUT (50 * HZ)
>
> This is 50 seconds, is this intentional?
>
Sorry will make it as 1*HZ.
> > +
> > +/**
> > + * struct xcan_priv - This definition define CAN driver instance
> > + * @can: CAN private data structure.
> > + * @open_time: For holding timeout values
>
> Please remove open_time completely from the driver.
Ok
>
> > + * @waiting_ech_skb_index: Pointer for skb
> > + * @ech_skb_next: This tell the next packet in the queue
> > + * @waiting_ech_skb_num: Gives the number of packets waiting
> > + * @xcan_echo_skb_max_tx: Maximum number packets the driver
> can send
> > + * @xcan_echo_skb_max_rx: Maximum number packets the driver
> can receive
> > + * @napi: NAPI structure
> > + * @ech_skb_lock: For spinlock purpose
> > + * @read_reg: For reading data from CAN registers
> > + * @write_reg: For writing data to CAN registers
> > + * @dev: Network device data structure
> > + * @reg_base: Ioremapped address to registers
> > + * @irq_flags: For request_irq()
> > + * @aperclk: Pointer to struct clk
> > + * @devclk: Pointer to struct clk
> > + */
> > +struct xcan_priv {
> > + struct can_priv can;
> > + int open_time;
> > + int waiting_ech_skb_index;
> > + int ech_skb_next;
>
> please make them:
>
> unsigned int tx_head;
> unsigned int tx_tail;
>
> I'll explain how to use them later. Have a look at the ti_hecc driver.
Ok
>
> > + int waiting_ech_skb_num;
> > + int xcan_echo_skb_max_tx;
> > + int xcan_echo_skb_max_rx;
> > + struct napi_struct napi;
> > + spinlock_t ech_skb_lock;
> > + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
> > + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>
> Please remove read_reg, write_reg, as long as there isn't any BE support in
> the driver, call them directly.
>
As per yours and Michal discussion I am keeping this here (endianess of the IP is not fixed at compile time).
> > + struct net_device *dev;
> > + void __iomem *reg_base;
> > + unsigned long irq_flags;
> > + struct clk *aperclk;
> > + struct clk *devclk;
> > +};
> > +
> > +/* CAN Bittiming constants as per Xilinx CAN specs */ static const
> > +struct can_bittiming_const xcan_bittiming_const = {
> > + .name = DRIVER_NAME,
> > + .tseg1_min = 1,
> > + .tseg1_max = 16,
> > + .tseg2_min = 1,
> > + .tseg2_max = 8,
> > + .sjw_max = 4,
> > + .brp_min = 1,
> > + .brp_max = 256,
> > + .brp_inc = 1,
> > +};
> > +
> > +/**
> > + * xcan_write_reg - Write a value to the device register
> > + * @priv: Driver private data structure
> > + * @reg: Register offset
> > + * @val: Value to write at the Register offset
> > + *
> > + * Write data to the paricular CAN register */ static void
> > +xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val)
>
> Please use the enum for instead of an int for the reg.
Ok
>
> > +{
> > + writel(val, priv->reg_base + reg);
> > +}
> > +
> > +/**
> > + * xcan_read_reg - Read a value from the device register
> > + * @priv: Driver private data structure
> > + * @reg: Register offset
> > + *
> > + * Read data from the particular CAN register
> > + * Return: value read from the CAN register */ static u32
> > +xcan_read_reg(const struct xcan_priv *priv, int reg) {
>
> same here
Ok
>
> > + return readl(priv->reg_base + reg);
> > +}
> > +
> > +/**
> > + * set_reset_mode - Resets the CAN device mode
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver reset mode routine.The driver
> > + * enters into configuration mode.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +set_reset_mode(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + unsigned long timeout;
> > +
> > + priv->can.state = CAN_STATE_STOPPED;
> > +
> > + timeout = jiffies + XCAN_TIMEOUT;
> > + while (!(priv->read_reg(priv, XCAN_SR_OFFSET) &
> XCAN_SR_CONFIG_MASK)) {
> > + if (time_after(jiffies, timeout)) {
> > + netdev_warn(ndev, "timedout waiting for config
> mode\n");
> > + return -ETIMEDOUT;
> > + }
> > + usleep_range(500, 10000);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_set_bittiming - CAN set bit timing routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver set bittiming routine.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_set_bittiming(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct can_bittiming *bt = &priv->can.bittiming;
> > + u32 btr0, btr1;
> > + u32 is_config_mode;
> > +
> > + /* Check whether Xilinx CAN is in configuration mode.
> > + * It cannot set bit timing if Xilinx CAN is not in configuration mode.
> > + */
> > + is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) &
> > + XCAN_SR_CONFIG_MASK;
> > + if (!is_config_mode) {
> > + netdev_alert(ndev,
> > + "Cannot set bittiming can is not in config mode\n");
> > + return -EPERM;
> > + }
> > +
> > + /* Setting Baud Rate prescalar value in BRPR Register */
> > + btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK;
> > +
> > + /* Setting Time Segment 1 in BTR Register */
> > + btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK;
> > +
> > + /* Setting Time Segment 2 in BTR Register */
> > + btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) &
> > + XCAN_BTR_TS2_MASK;
> > +
> > + /* Setting Synchronous jump width in BTR Register */
> > + btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) &
> XCAN_BTR_SJW_MASK;
>
> All the masking should not be needed, as the bit timing is calculated within
> the bounds you specified.
Ok
>
> > + priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0);
> > + priv->write_reg(priv, XCAN_BTR_OFFSET, btr1);
> > +
> > + netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
> > + priv->read_reg(priv, XCAN_BRPR_OFFSET),
> > + priv->read_reg(priv, XCAN_BTR_OFFSET));
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_start - This the drivers start routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the drivers start routine.
> > + * Based on the State of the CAN device it puts
> > + * the CAN device into a proper mode.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_start(struct net_device *ndev)
>
> Please name the function xcan_chip_start(), to for a common naming like
> the flexcan and at91 driver.
>
OK
> > +{
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 err;
> > + unsigned long timeout;
> > +
> > + /* Check if it is in reset mode */
> > + if (priv->can.state != CAN_STATE_STOPPED)
>
> Don't depend on any state here, I suggest to do a softreset (or
> equivalent) of you CAN core and configure everything.
>
Ok
> > + err = set_reset_mode(ndev);
> > + if (err < 0)
> > + return err;
> > +
> > + /* Enable interrupts */
> > + priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL);
> > +
> > + /* Check whether it is loopback mode or normal mode */
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> > + /* Put device into loopback mode */
> > + priv->write_reg(priv, XCAN_MSR_OFFSET,
> XCAN_MSR_LBACK_MASK);
> > + else
> > + /* The device is in normal mode */
> > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > +
> > + if (priv->can.state == CAN_STATE_STOPPED) {
> > + /* Enable Xilinx CAN */
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_CEN_MASK);
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + timeout = jiffies + XCAN_TIMEOUT;
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > + while ((priv->read_reg(priv, XCAN_SR_OFFSET)
> > + & XCAN_SR_LBACK_MASK) == 0) {
> > + if (time_after(jiffies, timeout)) {
> > + netdev_warn(ndev,
> > + "timedout for loopback
> mode\n");
> > + return -ETIMEDOUT;
> > + }
> > + usleep_range(500, 10000);
> > + }
> > + } else {
> > + while ((priv->read_reg(priv, XCAN_SR_OFFSET)
> > + & XCAN_SR_NORMAL_MASK) == 0) {
> > + if (time_after(jiffies, timeout)) {
> > + netdev_warn(ndev,
> > + "timedout for normal
> mode\n");
> > + return -ETIMEDOUT;
> > + }
> > + usleep_range(500, 10000);
> > + }
> > + }
> > + netdev_dbg(ndev, "status:#x%08x\n",
> > + priv->read_reg(priv, XCAN_SR_OFFSET));
> > + }
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_do_set_mode - This sets the mode of the driver
> > + * @ndev: Pointer to net_device structure
> > + * @mode: Tells the mode of the driver
> > + *
> > + * This check the drivers state and calls the
> > + * the corresponding modes to set.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_do_set_mode(struct net_device *ndev, enum can_mode mode) {
> > + int ret;
> > +
> > + switch (mode) {
> > + case CAN_MODE_START:
> > + ret = xcan_start(ndev);
> > + if (ret < 0)
> > + netdev_err(ndev, "xcan_start failed!\n");
> > + netif_wake_queue(ndev);
> > + break;
> > + default:
> > + ret = -EOPNOTSUPP;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * xcan_start_xmit - Starts the transmission
> > + * @skb: sk_buff pointer that contains data to be Txed
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This function is invoked from upper layers to initiate
> > +transmission. This
> > + * function uses the next available free txbuff and populates their
> > +fields to
> > + * start the transmission.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf = (struct can_frame *)skb->data;
> > + u32 id, dlc, data[2] = {0, 0}, rtr = 0;
>
> I think you can drop the rtr varibale and use cf->can_id & CAN_RTR_FLAG
> instead.
>
OK
> > + unsigned long flags;
> > +
> > + if (can_dropped_invalid_skb(ndev, skb))
> > + return NETDEV_TX_OK;
> > +
> > + /* Watch carefully on the bit sequence */
> > + if (cf->can_id & CAN_EFF_FLAG) {
> > + /* Extended CAN ID format */
> > + id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT)
> &
> > + XCAN_IDR_ID2_MASK;
> > + id |= (((cf->can_id & CAN_EFF_MASK) >>
> > + (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
> > + XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
> > +
> > + /* The substibute remote TX request bit should be "1"
> > + * for extended frames as in the Xilinx CAN datasheet
> > + */
> > + id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
> > +
> > + if (cf->can_id & CAN_RTR_FLAG) {
> > + /* Extended frames remote TX request */
> > + id |= XCAN_IDR_RTR_MASK;
> > + rtr = 1;
> > + }
> > + } else {
> > + /* Standard CAN ID format */
> > + id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT)
> &
> > + XCAN_IDR_ID1_MASK;
> > +
> > + if (cf->can_id & CAN_RTR_FLAG) {
> > + /* Extended frames remote TX request */
> > + id |= XCAN_IDR_SRR_MASK;
> > + rtr = 1;
> > + }
> > + }
> > +
> > + dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;
>
> No need to mask dlc, it's valid.
>
OK
> > +
> > + if (dlc > 0)
>
> You've copied my speudo code :)
> But you have to use (cf->can_dlc > 0) here, as dlc is the shifted value.
Yes :) I missed it will change
>
> > + data[0] = be32_to_cpup((__be32 *)(cf->data + 0));
> > + if (dlc > 4)
> > + data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
> > +
> > + can_put_echo_skb(skb, ndev, priv->ech_skb_next);
>
> can_put_echo_skb(skb, ndev,
> priv->tx_head % priv->xcan_echo_skb_max_tx);
>
> priv->tx_head++;
>
Ok
> > +
> > + /* Write the Frame to Xilinx CAN TX FIFO */
> > + priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
> > + priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
> > + if (!rtr) {
> > + priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data[0]);
> > + priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data[1]);
> > + stats->tx_bytes += cf->can_dlc;
>
> Please add a comment which write triggers the tx. What in case of the rtr?
> Which write triggers the tx then?
>
Ok Will Add
In RTR Case the below write triggers the trasmission
priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
In Normal case this write
priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data[1]);
Triggers the transmission.
In Btw: Due to the limitations in the IP( Tx DLC register is a write only Register)
I can't put this stats->tx_bytes += cf->can_dlc; in the tx interrupt routine.
> > + }
> > +
> > + priv->ech_skb_next = (priv->ech_skb_next + 1) %
> > + priv->xcan_echo_skb_max_tx;
>
> Please remove, it's not needed.
>
Ok
> > +
> > + spin_lock_irqsave(&priv->ech_skb_lock, flags);
> > + priv->waiting_ech_skb_num++;
> > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>
> All 3 not needed.
>
Ok
> > +
> > + /* Check if the TX buffer is full */
> > + if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK)
> {
> > + netif_stop_queue(ndev);
> > + netdev_err(ndev, "TX register is still full!\n");
> > + return NETDEV_TX_BUSY;
>
> If this is true, there is a Bug in the flow control. It should be moved to the
> beginning of the function, see at91_can's xmit function.
Yes this Condition Becomes true.
Will put the below check in the Beginning of this function
>
> /* Check if the TX buffer is full */
> if (unlikely(priv->read_reg(priv, XCAN_SR_OFFSET) &
> XCAN_SR_TXFLL_MASK)) {
> netif_stop_queue(ndev);
> netdev_err(ndev,
> "BUG!, TX FIFO full when queue awake!\n");
> return NETDEV_TX_BUSY;
> }
>
> > + } else if (priv->waiting_ech_skb_num == priv-
> >xcan_echo_skb_max_tx) {
> > + netif_stop_queue(ndev);
> > + netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n",
> > + priv->waiting_ech_skb_num, priv-
> >xcan_echo_skb_max_tx);
> > + return NETDEV_TX_BUSY;
> > + }
>
> This is a the regular flow control function and must be called before a TX
> complete interrupt can trigger. Your tx-complete interrupt is probably
> always enabled?
>
Yes. The tx-completed interrupt always enabled.
> So here you check the fill level of the FIFO:
>
> if ((priv->tx_head - priv->tx_tail) ==
> priv->xcan_echo_skb_max_tx)
> netif_stop_queue(ndev);
>
> If it's full, stop the queue. The you trigger the tx, the tx complete interrupt
> gets called and the queue will be restarted.
>
Ok
> > +
> > + return NETDEV_TX_OK;
> > +}
> > +
> > +/**
> > + * xcan_rx - Is called from CAN isr to complete the received
> > + * frame processing
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This function is invoked from the CAN isr(poll) to process the Rx
> > +frames. It
> > + * does minimal processing and invokes "netif_receive_skb" to
> > +complete further
> > + * processing.
> > + * Return: 0 on success and negative error value on error */ static
> > +int xcan_rx(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + u32 id_xcan, dlc, data[2] = {0, 0}, rtr = 0;
> > +
> > + skb = alloc_can_skb(ndev, &cf);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + /* Read a frame from Xilinx zynq CANPS */
> > + id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET);
> > + dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) &
> > +XCAN_DLCR_DLC_MASK;
>
> Better do the shift to dlc.
>
Ok
> > +
> > + /* Change Xilinx CAN data length format to socketCAN data format
> */
> > + cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
> > + XCAN_DLCR_DLC_SHIFT);
>
> Then it's just: get_can_dlc(dlc);
Ok
>
> > +
> > + /* Change Xilinx CAN ID format to socketCAN ID format */
> > + if (id_xcan & XCAN_IDR_IDE_MASK) {
> > + /* The received frame is an Extended format frame */
> > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
> > + cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
> > + XCAN_IDR_ID2_SHIFT;
> > + cf->can_id |= CAN_EFF_FLAG;
> > + if (id_xcan & XCAN_IDR_RTR_MASK) {
> > + cf->can_id |= CAN_RTR_FLAG;
> > + rtr = 1;
> > + }
> > + } else {
> > + /* The received frame is a standard format frame */
> > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
> > + XCAN_IDR_ID1_SHIFT;
> > + if (id_xcan & XCAN_IDR_RTR_MASK) {
> > + cf->can_id |= CAN_RTR_FLAG;
> > + rtr = 1;
> > + }
> > + }
> > +
> > + if (!rtr) {
> > + data[0] = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET);
> > + data[1] = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET);
> > +
> > + /* Change Xilinx CAN data format to socketCAN data format
> */
> > + *(__be32 *)(cf->data) = cpu_to_be32(data[0]);
> > + if (cf->can_dlc > 4)
> > + *(__be32 *)(cf->data + 4) = cpu_to_be32(data[1]);
> > + }
> > + can_led_event(ndev, CAN_LED_EVENT_RX);
> > +
> > + netif_receive_skb(skb);
> > +
> > + stats->rx_bytes += cf->can_dlc;
> > + stats->rx_packets++;
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_err_interrupt - error frame Isr
> > + * @ndev: net_device pointer
> > + * @isr: interrupt status register value
> > + *
> > + * This is the CAN error interrupt and it will
> > + * check the the type of error and forward the error
> > + * frame to upper layers.
> > + */
> > +static void xcan_err_interrupt(struct net_device *ndev, u32 isr) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + u32 err_status, status;
> > +
> > + skb = alloc_can_err_skb(ndev, &cf);
> > + if (!skb) {
> > + netdev_err(ndev, "alloc_can_err_skb() failed!\n");
> > + return;
> > + }
> > +
> > + err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
> > + priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);
> > + status = priv->read_reg(priv, XCAN_SR_OFFSET);
> > +
> > + if (isr & XCAN_IXR_BSOFF_MASK) {
> > + priv->can.state = CAN_STATE_BUS_OFF;
> > + cf->can_id |= CAN_ERR_BUSOFF;
> > + priv->can.can_stats.bus_off++;
> > + /* Leave device in Config Mode in bus-off state */
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_RESET_MASK);
> > + can_bus_off(ndev);
> > + } else if ((status & XCAN_SR_ESTAT_MASK) ==
> XCAN_SR_ESTAT_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> > + priv->can.can_stats.error_passive++;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE |
> > + CAN_ERR_CRTL_TX_PASSIVE;
> > + } else if (status & XCAN_SR_ERRWRN_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + priv->can.state = CAN_STATE_ERROR_WARNING;
> > + priv->can.can_stats.error_warning++;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING |
> > + CAN_ERR_CRTL_TX_WARNING;
> > + }
> > +
> > + /* Check for Arbitration lost interrupt */
> > + if (isr & XCAN_IXR_ARBLST_MASK) {
> > + cf->can_id |= CAN_ERR_LOSTARB;
> > + cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
> > + priv->can.can_stats.arbitration_lost++;
> > + }
> > +
> > + /* Check for RX FIFO Overflow interrupt */
> > + if (isr & XCAN_IXR_RXOFLW_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> > + stats->rx_over_errors++;
> > + stats->rx_errors++;
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_RESET_MASK);
> > + }
> > +
> > + /* Check for error interrupt */
> > + if (isr & XCAN_IXR_ERROR_MASK) {
> > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > +
> > + /* Check for Ack error interrupt */
> > + if (err_status & XCAN_ESR_ACKER_MASK) {
> > + cf->can_id |= CAN_ERR_ACK;
> > + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> > + stats->tx_errors++;
> > + }
> > +
> > + /* Check for Bit error interrupt */
> > + if (err_status & XCAN_ESR_BERR_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_BIT;
> > + stats->tx_errors++;
> > + }
> > +
> > + /* Check for Stuff error interrupt */
> > + if (err_status & XCAN_ESR_STER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_STUFF;
> > + stats->rx_errors++;
> > + }
> > +
> > + /* Check for Form error interrupt */
> > + if (err_status & XCAN_ESR_FMER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_FORM;
> > + stats->rx_errors++;
> > + }
> > +
> > + /* Check for CRC error interrupt */
> > + if (err_status & XCAN_ESR_CRCER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
> > + CAN_ERR_PROT_LOC_CRC_DEL;
> > + stats->rx_errors++;
> > + }
> > + priv->can.can_stats.bus_error++;
> > + }
> > +
> > + netif_rx(skb);
> > + stats->rx_packets++;
> > + stats->rx_bytes += cf->can_dlc;
> > +
> > + netdev_dbg(ndev, "%s: error status register:0x%x\n",
> > + __func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
> }
> > +
> > +/**
> > + * xcan_state_interrupt - It will check the state of the CAN device
> > + * @ndev: net_device pointer
> > + * @isr: interrupt status register value
> > + *
> > + * This will checks the state of the CAN device
> > + * and puts the device into appropriate state.
> > + */
> > +static void xcan_state_interrupt(struct net_device *ndev, u32 isr) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + /* Check for Sleep interrupt if set put CAN device in sleep state */
> > + if (isr & XCAN_IXR_SLP_MASK)
> > + priv->can.state = CAN_STATE_SLEEPING;
> > +
> > + /* Check for Wake up interrupt if set put CAN device in Active state
> */
> > + if (isr & XCAN_IXR_WKUP_MASK)
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE; }
> > +
> > +/**
> > + * xcan_rx_poll - Poll routine for rx packets (NAPI)
> > + * @napi: napi structure pointer
> > + * @quota: Max number of rx packets to be processed.
> > + *
> > + * This is the poll routine for rx part.
> > + * It will process the packets maximux quota value.
> > + *
> > + * Return: number of packets received */ static int
> > +xcan_rx_poll(struct napi_struct *napi, int quota) {
> > + struct net_device *ndev = napi->dev;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 isr, ier;
> > + int work_done = 0;
> > +
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
> > + if (isr & XCAN_IXR_RXOK_MASK) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> > + XCAN_IXR_RXOK_MASK);
> > + if (xcan_rx(ndev) < 0)
> > + return work_done;
> > + work_done++;
> > + } else {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> > + XCAN_IXR_RXNEMP_MASK);
> > + break;
> > + }
>
> What does the XCAN_IXR_RXOK_MASK mean if it's send and undset?
XCAN_IXR_RXOK_MASK Means it is successfully received one packet
>
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> XCAN_IXR_RXNEMP_MASK);
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + }
> > +
> > + if (work_done < quota) {
> > + napi_complete(napi);
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier |= (XCAN_IXR_RXOK_MASK |
> XCAN_IXR_RXNEMP_MASK);
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>
> Is this a read-modify-write register? I mean will an interrupt get disabled, if
> you write a 0-bit in the IER register? What does the ICR register?
ISR- Interrupt Status Register (Read only register)
IER- Interrupt Enable Register(R/w register)
ICR- Interrupt Clear Register.(write only register)
The Interrupt Status Register (ISR) contains bits that are set when a particular interrupt condition occurs. If
the corresponding mask bit in the Interrupt Enable Register is set, an interrupt is generated.
Interrupt bits in the ISR can be cleared by writing to the Interrupt Clear Register
>
> > + }
> > + return work_done;
> > +}
> > +
> > +/**
> > + * xcan_tx_interrupt - Tx Done Isr
> > + * @ndev: net_device pointer
> > + */
> > +static void xcan_tx_interrupt(struct net_device *ndev) {
> > + unsigned long flags;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + u32 processed = 0, txpackets;
> > +
> > + stats->tx_packets++;
> > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> > + priv->waiting_ech_skb_num, priv-
> >waiting_ech_skb_index);
> > +
> > + txpackets = priv->waiting_ech_skb_num;
> > +
> > + if (txpackets) {
> > + can_get_echo_skb(ndev, priv->waiting_ech_skb_index);
> > + priv->waiting_ech_skb_index =
> > + (priv->waiting_ech_skb_index + 1) %
> > + priv->xcan_echo_skb_max_tx;
> > + processed++;
> > + txpackets--;
> > + }
> > +
> > + spin_lock_irqsave(&priv->ech_skb_lock, flags);
> > + priv->waiting_ech_skb_num -= processed;
> > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>
> This all simplyfies to a:
> can_get_echo_skb(ndev, priv->tx_tail %
> priv->xcan_echo_skb_max_tx);
> priv->tx_tail++;
>
Ok
> I think you should add some kind of loop here, it there is more than one tx-
> complete per IRQ.
>
Ok
> > +
> > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> > + priv->waiting_ech_skb_num, priv-
> >waiting_ech_skb_index);
> > +
> > + netif_wake_queue(ndev);
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_TX); }
> > +
> > +/**
> > + * xcan_interrupt - CAN Isr
> > + * @irq: irq number
> > + * @dev_id: device id poniter
> > + *
> > + * This is the xilinx CAN Isr. It checks for the type of interrupt
> > + * and invokes the corresponding ISR.
> > + *
> > + * Return:
> > + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise
> > +*/ static irqreturn_t xcan_interrupt(int irq, void *dev_id) {
> > + struct net_device *ndev = (struct net_device *)dev_id;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 isr, ier;
> > +
> > + /* Get the interrupt status from Xilinx CAN */
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + if (!isr)
> > + return IRQ_NONE;
> > +
> > + netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__,
> > + isr, priv->read_reg(priv, XCAN_ESR_OFFSET));
> > +
> > + /* Check for the type of interrupt and Processing it */
> > + if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> (XCAN_IXR_SLP_MASK |
> > + XCAN_IXR_WKUP_MASK));
> > + xcan_state_interrupt(ndev, isr);
> > + }
> > +
> > + /* Check for Tx interrupt and Processing it */
> > + if (isr & XCAN_IXR_TXOK_MASK) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> XCAN_IXR_TXOK_MASK);
> > + xcan_tx_interrupt(ndev);
> > + }
> > +
> > + /* Check for the type of error interrupt and Processing it */
> > + if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
> > + XCAN_IXR_BSOFF_MASK |
> XCAN_IXR_ARBLST_MASK)) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> (XCAN_IXR_ERROR_MASK |
> > + XCAN_IXR_RXOFLW_MASK |
> XCAN_IXR_BSOFF_MASK |
> > + XCAN_IXR_ARBLST_MASK));
> > + xcan_err_interrupt(ndev, isr);
> > + }
> > +
> > + /* Check for the type of receive interrupt and Processing it */
> > + if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier &= ~(XCAN_IXR_RXNEMP_MASK |
> XCAN_IXR_RXOK_MASK);
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> > + napi_schedule(&priv->napi);
> > + }
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/**
> > + * xcan_stop - Driver stop routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the drivers stop routine. It will disable the
> > + * interrupts and put the device into configuration mode.
> > + */
> > +static void xcan_stop(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 ier;
> > +
> > + /* Disable interrupts and leave the can in configuration mode */
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier &= ~XCAN_INTR_ALL;
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
> > + priv->can.state = CAN_STATE_STOPPED; }
> > +
> > +/**
> > + * xcan_open - Driver open routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver open routine.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_open(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + int ret;
> > +
> > + ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
> > + ndev->name, (void *)ndev);
> > + if (ret < 0) {
> > + netdev_err(ndev, "Irq allocation for CAN failed\n");
> > + return ret;
> > + }
> > +
> > + /* Set chip into reset mode */
> > + ret = set_reset_mode(ndev);
> > + if (ret < 0)
> > + netdev_err(ndev, "mode resetting failed failed!\n");
>
> Is this critical?
This condition usually won't fail.
But if the controller has a problems at the h/w level in that case putted this err print.
If you want me to change it as a warning will do
>
> > +
> > + /* Common open */
> > + ret = open_candev(ndev);
> > + if (ret)
> > + return ret;
>
> You should free the interrupt handler if this fails.
Ok
>
> > +
> > + ret = xcan_start(ndev);
> > + if (ret < 0)
> > + netdev_err(ndev, "xcan_start failed!\n");
> > +
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_OPEN);
> > + napi_enable(&priv->napi);
> > + netif_start_queue(ndev);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_close - Driver close routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * Return: 0 always
> > + */
> > +static int xcan_close(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + netif_stop_queue(ndev);
> > + napi_disable(&priv->napi);
> > + xcan_stop(ndev);
> > + free_irq(ndev->irq, ndev);
> > + close_candev(ndev);
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_STOP);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_get_berr_counter - error counter routine
> > + * @ndev: Pointer to net_device structure
> > + * @bec: Pointer to can_berr_counter structure
> > + *
> > + * This is the driver error counter routine.
> > + * Return: 0 always
> > + */
> > +static int xcan_get_berr_counter(const struct net_device *ndev,
> > + struct can_berr_counter *bec)
> > +{
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) &
> XCAN_ECR_TEC_MASK;
> > + bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
> > + XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
> > + return 0;
> > +}
> > +
> > +static const struct net_device_ops xcan_netdev_ops = {
> > + .ndo_open = xcan_open,
> > + .ndo_stop = xcan_close,
> > + .ndo_start_xmit = xcan_start_xmit,
> > +};
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +/**
> > + * xcan_suspend - Suspend method for the driver
> > + * @_dev: Address of the platform_device structure
> > + *
> > + * Put the driver into low power mode.
> > + * Return: 0 always
> > + */
> > +static int xcan_suspend(struct device *_dev) {
> > + struct platform_device *pdev = container_of(_dev,
> > + struct platform_device, dev);
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + if (netif_running(ndev)) {
> > + netif_stop_queue(ndev);
> > + netif_device_detach(ndev);
> > + }
> > +
> > + priv->write_reg(priv, XCAN_MSR_OFFSET,
> XCAN_MSR_SLEEP_MASK);
> > + priv->can.state = CAN_STATE_SLEEPING;
> > +
> > + clk_disable(priv->aperclk);
> > + clk_disable(priv->devclk);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_resume - Resume from suspend
> > + * @dev: Address of the platformdevice structure
> > + *
> > + * Resume operation after suspend.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_resume(struct device *dev) {
> > + struct platform_device *pdev = container_of(dev,
> > + struct platform_device, dev);
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + int ret;
> > +
> > + ret = clk_enable(priv->aperclk);
> > + if (ret) {
> > + dev_err(dev, "Cannot enable clock.\n");
> > + return ret;
> > + }
> > + ret = clk_enable(priv->devclk);
> > + if (ret) {
> > + dev_err(dev, "Cannot enable clock.\n");
> > + return ret;
> > + }
> > +
> > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > + if (netif_running(ndev)) {
> > + netif_device_attach(ndev);
> > + netif_start_queue(ndev);
> > + }
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
> xcan_resume);
> > +
> > +/**
> > + * xcan_probe - Platform registration call
> > + * @pdev: Handle to the platform device structure
> > + *
> > + * This function does all the memory allocation and registration for
> > +the CAN
> > + * device.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_probe(struct platform_device *pdev) {
> > + struct resource *res; /* IO mem resources */
> > + struct net_device *ndev;
> > + struct xcan_priv *priv;
> > + int ret, fifodep;
> > +
> > + /* Create a CAN device instance */
> > + ndev = alloc_candev(sizeof(struct xcan_priv),
> XCAN_ECHO_SKB_MAX);
> > + if (!ndev)
> > + return -ENOMEM;
> > +
> > + priv = netdev_priv(ndev);
> > + priv->dev = ndev;
> > + priv->can.bittiming_const = &xcan_bittiming_const;
> > + priv->can.do_set_bittiming = xcan_set_bittiming;
> > + priv->can.do_set_mode = xcan_do_set_mode;
> > + priv->can.do_get_berr_counter = xcan_get_berr_counter;
> > + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> > + CAN_CTRLMODE_BERR_REPORTING;
> > +
> > + /* Get IRQ for the device */
> > + ndev->irq = platform_get_irq(pdev, 0);
> > +
> > + spin_lock_init(&priv->ech_skb_lock);
> > + ndev->flags |= IFF_ECHO; /* We support local echo */
> > +
> > + platform_set_drvdata(pdev, ndev);
> > + SET_NETDEV_DEV(ndev, &pdev->dev);
> > + ndev->netdev_ops = &xcan_netdev_ops;
> > +
> > + /* Get the virtual base address for the device */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + priv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(priv->reg_base)) {
> > + ret = PTR_ERR(priv->reg_base);
> > + goto err_free;
> > + }
> > + ndev->mem_start = res->start;
> > + ndev->mem_end = res->end;
> > +
> > + priv->write_reg = xcan_write_reg;
> > + priv->read_reg = xcan_read_reg;
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth",
> > + &fifodep);
> > + if (ret < 0)
> > + goto err_free;
> > + priv->xcan_echo_skb_max_tx = fifodep;
> > +
> > + ret = of_property_read_u32(pdev->dev.of_node, "rx-fifo-depth",
> > + &fifodep);
> > + if (ret < 0)
> > + goto err_free;
> > + priv->xcan_echo_skb_max_rx = fifodep;
> > +
> > + /* Getting the CAN devclk info */
> > + priv->devclk = devm_clk_get(&pdev->dev, "ref_clk");
> > + if (IS_ERR(priv->devclk)) {
> > + dev_err(&pdev->dev, "Device clock not found.\n");
> > + ret = PTR_ERR(priv->devclk);
> > + goto err_free;
> > + }
> > +
> > + /* Check for type of CAN device */
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "xlnx,zynq-can-1.00.a")) {
> > + priv->aperclk = devm_clk_get(&pdev->dev, "aper_clk");
> > + if (IS_ERR(priv->aperclk)) {
> > + dev_err(&pdev->dev, "aper clock not found\n");
> > + ret = PTR_ERR(priv->aperclk);
> > + goto err_free;
> > + }
> > + } else {
> > + priv->aperclk = priv->devclk;
> > + }
> > +
> > + ret = clk_prepare_enable(priv->devclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "unable to enable device clock\n");
> > + goto err_free;
> > + }
> > +
> > + ret = clk_prepare_enable(priv->aperclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "unable to enable aper clock\n");
> > + goto err_unprepar_disabledev;
> > + }
>
> Can you keep your clocks disaled if the interface is not up?
I didn't get it will you please explain?
Regards,
Kedar.
>
> > +
> > + priv->can.clock.freq = clk_get_rate(priv->devclk);
> > +
> > + netif_napi_add(ndev, &priv->napi, xcan_rx_poll,
> > + priv->xcan_echo_skb_max_rx);
> > + ret = register_candev(ndev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "fail to register failed (err=%d)\n",
> ret);
> > + goto err_unprepar_disableaper;
> > + }
> > +
> > + devm_can_led_init(ndev);
> > + dev_info(&pdev->dev,
> > + "reg_base=0x%p irq=%d clock=%d, tx fifo
> depth:%d\n",
> > + priv->reg_base, ndev->irq, priv->can.clock.freq,
> > + priv->xcan_echo_skb_max_tx);
> > +
> > + return 0;
> > +
> > +err_unprepar_disableaper:
> > + clk_disable_unprepare(priv->aperclk);
> > +err_unprepar_disabledev:
> > + clk_disable_unprepare(priv->devclk);
> > +err_free:
> > + free_candev(ndev);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * xcan_remove - Unregister the device after releasing the resources
> > + * @pdev: Handle to the platform device structure
> > + *
> > + * This function frees all the resources allocated to the device.
> > + * Return: 0 always
> > + */
> > +static int xcan_remove(struct platform_device *pdev) {
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + if (set_reset_mode(ndev) < 0)
> > + netdev_err(ndev, "mode resetting failed!\n");
> > +
> > + unregister_candev(ndev);
> > + netif_napi_del(&priv->napi);
> > + clk_disable_unprepare(priv->aperclk);
> > + clk_disable_unprepare(priv->devclk);
> > +
> > + free_candev(ndev);
> > +
> > + return 0;
> > +}
> > +
> > +/* Match table for OF platform binding */ static struct of_device_id
> > +xcan_of_match[] = {
> > + { .compatible = "xlnx,zynq-can-1.00.a", },
> > + { .compatible = "xlnx,axi-can-1.00.a", },
> > + { /* end of list */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, xcan_of_match);
> > +
> > +static struct platform_driver xcan_driver = {
> > + .probe = xcan_probe,
> > + .remove = xcan_remove,
> > + .driver = {
> > + .owner = THIS_MODULE,
> > + .name = DRIVER_NAME,
> > + .pm = &xcan_dev_pm_ops,
> > + .of_match_table = xcan_of_match,
> > + },
> > +};
> > +
> > +module_platform_driver(xcan_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Xilinx Inc");
> > +MODULE_DESCRIPTION("Xilinx CAN interface");
> >
>
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH v7 01/12] mfd: omap-usb-host: Use resource managed clk_get()
From: Lee Jones @ 2014-02-14 9:57 UTC (permalink / raw)
To: Roger Quadros
Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, bcousson-rdvid1DuHRBWk0Htik3J/w,
balbi-l0cyMroinI0, nm-l0cyMroinI0, khilman-QSEj5FYQhm4dnm+yROfE0A,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz
In-Reply-To: <1392200333-28397-2-git-send-email-rogerq-l0cyMroinI0@public.gmane.org>
> Use devm_clk_get() instead of clk_get().
>
> CC: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/mfd/omap-usb-host.c | 81 +++++++++------------------------------------
> 1 file changed, 16 insertions(+), 65 deletions(-)
Applied, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-14 9:59 UTC (permalink / raw)
To: Appana Durga Kedareswara Rao, wg@grandegger.com, Michal Simek,
grant.likely@linaro.org, robh+dt@kernel.org,
linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <ad62e4f0-fe91-4dac-9a74-ffe9c9128cb2@CH1EHSMHS024.ehs.local>
[-- Attachment #1: Type: text/plain, Size: 14479 bytes --]
On 02/14/2014 10:36 AM, Appana Durga Kedareswara Rao wrote:
>>> +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
>>> +#define XCAN_SRR_CEN_MASK 0x00000002 /* CAN enable */
>>> +#define XCAN_SRR_RESET_MASK 0x00000001 /* Soft Reset the
>> CAN core */
>>> +#define XCAN_MSR_LBACK_MASK 0x00000002 /* Loop back
>> mode select */
>>> +#define XCAN_MSR_SLEEP_MASK 0x00000001 /* Sleep mode
>> select */
>>> +#define XCAN_BRPR_BRP_MASK 0x000000FF /* Baud rate
>> prescaler */
>>> +#define XCAN_BTR_SJW_MASK 0x00000180 /* Synchronous
>> jump width */
>>> +#define XCAN_BTR_TS2_MASK 0x00000070 /* Time segment
>> 2 */
>>> +#define XCAN_BTR_TS1_MASK 0x0000000F /* Time segment
>> 1 */
>>> +#define XCAN_ECR_REC_MASK 0x0000FF00 /* Receive error
>> counter */
>>> +#define XCAN_ECR_TEC_MASK 0x000000FF /* Transmit error
>> counter */
>>> +#define XCAN_ESR_ACKER_MASK 0x00000010 /* ACK error */
>>> +#define XCAN_ESR_BERR_MASK 0x00000008 /* Bit error */
>>> +#define XCAN_ESR_STER_MASK 0x00000004 /* Stuff error */
>>> +#define XCAN_ESR_FMER_MASK 0x00000002 /* Form error */
>>> +#define XCAN_ESR_CRCER_MASK 0x00000001 /* CRC error */
>>> +#define XCAN_SR_TXFLL_MASK 0x00000400 /* TX FIFO is full
>> */
>>> +#define XCAN_SR_ESTAT_MASK 0x00000180 /* Error status */
>>> +#define XCAN_SR_ERRWRN_MASK 0x00000040 /* Error warning
>> */
>>> +#define XCAN_SR_NORMAL_MASK 0x00000008 /* Normal mode
>> */
>>> +#define XCAN_SR_LBACK_MASK 0x00000002 /* Loop back
>> mode */
>>> +#define XCAN_SR_CONFIG_MASK 0x00000001 /* Configuration
>> mode */
>>> +#define XCAN_IXR_TXFEMP_MASK 0x00004000 /* TX FIFO Empty
>> */
>>> +#define XCAN_IXR_WKUP_MASK 0x00000800 /* Wake up
>> interrupt */
>>> +#define XCAN_IXR_SLP_MASK 0x00000400 /* Sleep
>> interrupt */
>>> +#define XCAN_IXR_BSOFF_MASK 0x00000200 /* Bus off
>> interrupt */
>>> +#define XCAN_IXR_ERROR_MASK 0x00000100 /* Error interrupt
>> */
>>> +#define XCAN_IXR_RXNEMP_MASK 0x00000080 /* RX FIFO
>> NotEmpty intr */
>>> +#define XCAN_IXR_RXOFLW_MASK 0x00000040 /* RX FIFO
>> Overflow intr */
>>> +#define XCAN_IXR_RXOK_MASK 0x00000010 /* Message
>> received intr */
>>> +#define XCAN_IXR_TXOK_MASK 0x00000002 /* TX successful
>> intr */
>>> +#define XCAN_IXR_ARBLST_MASK 0x00000001 /* Arbitration
>> lost intr */
>>> +#define XCAN_IDR_ID1_MASK 0xFFE00000 /* Standard msg
>> identifier */
>>> +#define XCAN_IDR_SRR_MASK 0x00100000 /* Substitute
>> remote TXreq */
>>> +#define XCAN_IDR_IDE_MASK 0x00080000 /* Identifier
>> extension */
>>> +#define XCAN_IDR_ID2_MASK 0x0007FFFE /* Extended
>> message ident */
>>> +#define XCAN_IDR_RTR_MASK 0x00000001 /* Remote TX
>> request */
>>> +#define XCAN_DLCR_DLC_MASK 0xF0000000 /* Data length
>> code */
>>> +
>
> Need to use BIT() Macro for the Masks?
You can, but it IMHO only makes sense where only a single bit is set.
[...]
>>> + int waiting_ech_skb_num;
>>> + int xcan_echo_skb_max_tx;
>>> + int xcan_echo_skb_max_rx;
>>> + struct napi_struct napi;
>>> + spinlock_t ech_skb_lock;
>>> + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>>> + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>>
>> Please remove read_reg, write_reg, as long as there isn't any BE support in
>> the driver, call them directly.
>>
>
>
> As per yours and Michal discussion I am keeping this here (endianess
> of the IP is not fixed at compile time).
Ok.
[...]
>>> +/**
>>> + * xcan_start_xmit - Starts the transmission
>>> + * @skb: sk_buff pointer that contains data to be Txed
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This function is invoked from upper layers to initiate
>>> +transmission. This
>>> + * function uses the next available free txbuff and populates their
>>> +fields to
>>> + * start the transmission.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &ndev->stats;
>>> + struct can_frame *cf = (struct can_frame *)skb->data;
>>> + u32 id, dlc, data[2] = {0, 0}, rtr = 0;
>>
>> I think you can drop the rtr varibale and use cf->can_id & CAN_RTR_FLAG
>> instead.
>>
>
> OK
>>> + unsigned long flags;
>>> +
>>> + if (can_dropped_invalid_skb(ndev, skb))
>>> + return NETDEV_TX_OK;
>>> +
>>> + /* Watch carefully on the bit sequence */
>>> + if (cf->can_id & CAN_EFF_FLAG) {
>>> + /* Extended CAN ID format */
>>> + id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT)
>> &
>>> + XCAN_IDR_ID2_MASK;
>>> + id |= (((cf->can_id & CAN_EFF_MASK) >>
>>> + (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
>>> + XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
>>> +
>>> + /* The substibute remote TX request bit should be "1"
>>> + * for extended frames as in the Xilinx CAN datasheet
>>> + */
>>> + id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
>>> +
>>> + if (cf->can_id & CAN_RTR_FLAG) {
>>> + /* Extended frames remote TX request */
>>> + id |= XCAN_IDR_RTR_MASK;
>>> + rtr = 1;
>>> + }
>>> + } else {
>>> + /* Standard CAN ID format */
>>> + id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT)
>> &
>>> + XCAN_IDR_ID1_MASK;
>>> +
>>> + if (cf->can_id & CAN_RTR_FLAG) {
>>> + /* Extended frames remote TX request */
>>> + id |= XCAN_IDR_SRR_MASK;
>>> + rtr = 1;
>>> + }
>>> + }
>>> +
>>> + dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;
>>
>> No need to mask dlc, it's valid.
>>
> OK
>
>>> +
>>> + if (dlc > 0)
>>
>> You've copied my speudo code :)
>> But you have to use (cf->can_dlc > 0) here, as dlc is the shifted value.
>
> Yes :) I missed it will change
>
>>
>>> + data[0] = be32_to_cpup((__be32 *)(cf->data + 0));
>>> + if (dlc > 4)
>>> + data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
>>> +
>>> + can_put_echo_skb(skb, ndev, priv->ech_skb_next);
>>
>> can_put_echo_skb(skb, ndev,
>> priv->tx_head % priv->xcan_echo_skb_max_tx);
>>
>> priv->tx_head++;
>>
>
> Ok
>>> +
>>> + /* Write the Frame to Xilinx CAN TX FIFO */
>>> + priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
>>> + priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
>>> + if (!rtr) {
>>> + priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data[0]);
>>> + priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data[1]);
>>> + stats->tx_bytes += cf->can_dlc;
>>
>> Please add a comment which write triggers the tx. What in case of the rtr?
>> Which write triggers the tx then?
>>
>
> Ok Will Add
>
> In RTR Case the below write triggers the trasmission
> priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
> In Normal case this write
> priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data[1]);
> Triggers the transmission.
>
> In Btw: Due to the limitations in the IP( Tx DLC register is a write only Register)
> I can't put this stats->tx_bytes += cf->can_dlc; in the tx interrupt routine.
No problem.
[...]
>>> + if (work_done < quota) {
>>> + napi_complete(napi);
>>> + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> + ier |= (XCAN_IXR_RXOK_MASK |
>> XCAN_IXR_RXNEMP_MASK);
>>> + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>
>> Is this a read-modify-write register? I mean will an interrupt get disabled, if
>> you write a 0-bit in the IER register? What does the ICR register?
>
> ISR- Interrupt Status Register (Read only register)
> IER- Interrupt Enable Register(R/w register)
> ICR- Interrupt Clear Register.(write only register)
>
>
> The Interrupt Status Register (ISR) contains bits that are set when a particular interrupt condition occurs. If
> the corresponding mask bit in the Interrupt Enable Register is set, an interrupt is generated.
> Interrupt bits in the ISR can be cleared by writing to the Interrupt Clear Register
Thanks.
[...]
>>> +/**
>>> + * xcan_open - Driver open routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the driver open routine.
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_open(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + int ret;
>>> +
>>> + ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
>>> + ndev->name, (void *)ndev);
>>> + if (ret < 0) {
>>> + netdev_err(ndev, "Irq allocation for CAN failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + /* Set chip into reset mode */
>>> + ret = set_reset_mode(ndev);
>>> + if (ret < 0)
>>> + netdev_err(ndev, "mode resetting failed failed!\n");
>>
>> Is this critical?
>
> This condition usually won't fail.
> But if the controller has a problems at the h/w level in that case putted this err print.
> If you want me to change it as a warning will do
If there is a hardware level problem, is it better to return here with
an error (and free the IRQ)?
[...]
>>> +/**
>>> + * xcan_probe - Platform registration call
>>> + * @pdev: Handle to the platform device structure
>>> + *
>>> + * This function does all the memory allocation and registration for
>>> +the CAN
>>> + * device.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_probe(struct platform_device *pdev) {
>>> + struct resource *res; /* IO mem resources */
>>> + struct net_device *ndev;
>>> + struct xcan_priv *priv;
>>> + int ret, fifodep;
>>> +
>>> + /* Create a CAN device instance */
>>> + ndev = alloc_candev(sizeof(struct xcan_priv),
>> XCAN_ECHO_SKB_MAX);
>>> + if (!ndev)
>>> + return -ENOMEM;
>>> +
>>> + priv = netdev_priv(ndev);
>>> + priv->dev = ndev;
>>> + priv->can.bittiming_const = &xcan_bittiming_const;
>>> + priv->can.do_set_bittiming = xcan_set_bittiming;
>>> + priv->can.do_set_mode = xcan_do_set_mode;
>>> + priv->can.do_get_berr_counter = xcan_get_berr_counter;
>>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>>> + CAN_CTRLMODE_BERR_REPORTING;
>>> +
>>> + /* Get IRQ for the device */
>>> + ndev->irq = platform_get_irq(pdev, 0);
>>> +
>>> + spin_lock_init(&priv->ech_skb_lock);
>>> + ndev->flags |= IFF_ECHO; /* We support local echo */
>>> +
>>> + platform_set_drvdata(pdev, ndev);
>>> + SET_NETDEV_DEV(ndev, &pdev->dev);
>>> + ndev->netdev_ops = &xcan_netdev_ops;
>>> +
>>> + /* Get the virtual base address for the device */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + priv->reg_base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(priv->reg_base)) {
>>> + ret = PTR_ERR(priv->reg_base);
>>> + goto err_free;
>>> + }
>>> + ndev->mem_start = res->start;
>>> + ndev->mem_end = res->end;
>>> +
>>> + priv->write_reg = xcan_write_reg;
>>> + priv->read_reg = xcan_read_reg;
>>> +
>>> + ret = of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth",
>>> + &fifodep);
>>> + if (ret < 0)
>>> + goto err_free;
>>> + priv->xcan_echo_skb_max_tx = fifodep;
>>> +
>>> + ret = of_property_read_u32(pdev->dev.of_node, "rx-fifo-depth",
>>> + &fifodep);
>>> + if (ret < 0)
>>> + goto err_free;
>>> + priv->xcan_echo_skb_max_rx = fifodep;
>>> +
>>> + /* Getting the CAN devclk info */
>>> + priv->devclk = devm_clk_get(&pdev->dev, "ref_clk");
>>> + if (IS_ERR(priv->devclk)) {
>>> + dev_err(&pdev->dev, "Device clock not found.\n");
>>> + ret = PTR_ERR(priv->devclk);
>>> + goto err_free;
>>> + }
>>> +
>>> + /* Check for type of CAN device */
>>> + if (of_device_is_compatible(pdev->dev.of_node,
>>> + "xlnx,zynq-can-1.00.a")) {
>>> + priv->aperclk = devm_clk_get(&pdev->dev, "aper_clk");
>>> + if (IS_ERR(priv->aperclk)) {
>>> + dev_err(&pdev->dev, "aper clock not found\n");
>>> + ret = PTR_ERR(priv->aperclk);
>>> + goto err_free;
>>> + }
>>> + } else {
>>> + priv->aperclk = priv->devclk;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->devclk);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "unable to enable device clock\n");
>>> + goto err_free;
>>> + }
>>> +
>>> + ret = clk_prepare_enable(priv->aperclk);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "unable to enable aper clock\n");
>>> + goto err_unprepar_disabledev;
>>> + }
>>
>> Can you keep your clocks disaled if the interface is not up?
>
> I didn't get it will you please explain?
This feature s optional, but a a good practice.
This function gets called when the driver is loaded, i.e. during boot.
So the complete CAN core will be enabled and powered, even if the
interface is down. To reduce power consumption it's better to enable the
clocks in the open() function and disable in close(). If you access some
CAN registers during probe you have to enable the clock and you probably
have to enable it in the do_get_berr_counter callback, as it may be
called if the interface is still down.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* Re: [PATCH 10/10] Documentation: Add device tree bindings for TI LMU devices
From: Mark Rutland @ 2014-02-14 10:06 UTC (permalink / raw)
To: Milo Kim
Cc: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Samuel Ortiz
In-Reply-To: <1392359564-7205-1-git-send-email-milo.kim-l0cyMroinI0@public.gmane.org>
On Fri, Feb 14, 2014 at 06:32:44AM +0000, Milo Kim wrote:
> Bindings for TI LMU, backlight, LM3631 regulator and LM3633 LED are added.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Milo Kim <milo.kim-l0cyMroinI0@public.gmane.org>
> ---
> .../devicetree/bindings/leds/leds-lm3633.txt | 39 +++++
> Documentation/devicetree/bindings/mfd/ti-lmu.txt | 182 ++++++++++++++++++++
> .../bindings/regulator/lm3631-regulator.txt | 49 ++++++
> .../bindings/video/backlight/ti-lmu-backlight.txt | 127 ++++++++++++++
> 4 files changed, 397 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/ti-lmu.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
> create mode 100644 Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
> new file mode 100644
> index 0000000..4adeb62
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
> @@ -0,0 +1,39 @@
> +TI LMU LM3633 LED device tree bindings
> +
> +Required properties:
> + - compatible: "ti,lm3633-leds"
> + - lvled1-used, lvled2-used, lvled3-used, lvled4-used, lvled5-used, lvled6-used
> + : LED string configuration. Each child node should include this information
> + about which LED string is used.
Which child nodes? They weren't mentioned until this point.
If properties need to be in a child node, mention the child node first,
and make it clear where the properties are expected to be.
> +
> +Optional properties:
> + - chan-name: LED channel name
Any reason to abbreviate "channel" to "chan"?
What's this for?
> + - max-current-milliamp: Max current setting. Unit is mA.
The code and examples treat this as an 8-bit value, but this fact isn't
mentioned here.
> +
> +Example:
> +
> +lm3633@36 {
> + compatible = "ti,lm3633";
It wasn't mentioned that this had to be a sub-node of a "ti,lm3633"
node. Please describe above and refer to the document for the
"ti,lm3633" binding.
[...]
> diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> new file mode 100644
> index 0000000..2b3ecca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> @@ -0,0 +1,182 @@
> +TI LMU(Lighting Management Unit) device tree bindings
> +
> +TI LMU driver supports lighting devices belows.
> +
> + Name Device tree properties
> + ------ ------------------------
> + LM3532 Backlight
> + LM3631 Backlight and regulator
> + LM3633 Backlight and LED
> + LM3695 Backlight
> + LM3697 Backlight
> +
> +Those have shared device tree properties.
> +
> +Required properties:
> + - compatible: "ti,lm3532", "ti,lm3631", "ti,lm3633", "ti,lm3695", "ti,lm3697"
Should be one of, rather than all at once?
> + - reg: I2C slave address.
> + 0x38 is LM3532
> + 0x29 is LM3631
> + 0x36 is LM3633, LM3697
> + 0x63 is LM3695
> + - ti,enable-gpio: GPIO number of hardware enable pin
We refer to GPIOs with more than a number...
> +
> +For the TI LMU backlight properties, please refer to:
> +Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> +
> +For the LM3631 regulator properties, please refer to:
> +Documentation/devicetree/bindings/regulator/lm3631-regulator.txt
> +
> +For the LM3633 LED properties, please refer to:
> +Documentation/devicetree/bindings/leds/leds-lm3633.txt
Are these expected as subnodes?
> +
> +Examples:
> +
> +lm3532@38 {
> + compatible = "ti,lm3532";
> + reg = <0x38>;
> +
> + /* GPIO134 for HWEN pin */
> + ti,enable-gpio = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> +
> + backlight {
> + compatible = "ti,lmu-backlight", "ti,lm3532-backlight";
This looks backwards. The most general string should be later in the
list. This applies elsewhere too.
[...]
> diff --git a/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> new file mode 100644
> index 0000000..554ddca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/backlight/ti-lmu-backlight.txt
> @@ -0,0 +1,127 @@
> +TI LMU backlight device tree bindings
> +
> +Required properties:
> + - compatible: One of lists below with "ti,lmu-backlight" should be set.
> + "ti,lm3532-backlight"
> + "ti,lm3631-backlight"
> + "ti,lm3633-backlight"
> + "ti,lm3695-backlight"
> + "ti,lm3697-backlight"
Do you mean that "ti,lmu-backlight" should be a fallback entry in the
compatible list?
> + - hvled1-used, hvled2-used, hvled3-used: Backlight string configuration.
> + Each backlight child node should include this information about
> + which backlight string is used.
> +
> +Optional properties
> + - bl-name: Backlight device name
Why bother abbreviating backlight to bl?
What's this for anyway? Surely something else has to link to the
backlight node, and a meaningful name should be implied.
> + - max-current-milliamp: Max current setting. Unit is mA.
Type?
> + - initial-brightness: Backlight initial brightness
Type? Units?
> + - ramp-up: Light effect for ramp up rate. Unit is msec.
> + - ramp-down: Light effect for ramp down rate. Unit is msec.
> + - pwm-period: PWM period. Only valid for PWM brightness control mode.
Type? Units?
> + - pwms, pwm-names: For the PWM user nodes, please refer to
How many do you expect?
What are each of them for?
What are they named?
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 02/12] mfd: omap-usb-host: Get clocks based on hardware revision
From: Lee Jones @ 2014-02-14 10:07 UTC (permalink / raw)
To: Roger Quadros
Cc: tony, bcousson, balbi, nm, khilman, linux-omap, linux-arm-kernel,
linux-kernel, devicetree, linux-usb, Samuel Ortiz
In-Reply-To: <1392200333-28397-3-git-send-email-rogerq@ti.com>
> Not all revisions have all the clocks so get the necessary clocks
> based on hardware revision.
>
> This should avoid un-necessary clk_get failure messages that were
> observed earlier.
>
> Be more strict and always fail on clk_get() error.
It might have been clearer if you'd broken these two pieces of
functionality changes into two different patches. In future it would
be preferred.
> CC: Lee Jones <lee.jones@linaro.org>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/mfd/omap-usb-host.c | 92 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 64 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 0c3c9a0..60a3bed 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -665,22 +665,41 @@ static int usbhs_omap_probe(struct platform_device *pdev)
> goto err_mem;
> }
>
> - need_logic_fck = false;
> + /* Set all clocks as invalid to begin with */
> + omap->ehci_logic_fck = omap->init_60m_fclk = ERR_PTR(-ENODEV);
> + omap->utmi_p1_gfclk = omap->utmi_p2_gfclk = ERR_PTR(-ENODEV);
> + omap->xclk60mhsp1_ck = omap->xclk60mhsp2_ck = ERR_PTR(-ENODEV);
> +
For readability you should probably do these one per line.
> for (i = 0; i < omap->nports; i++) {
> - if (is_ehci_phy_mode(i) || is_ehci_tll_mode(i) ||
> - is_ehci_hsic_mode(i))
> - need_logic_fck |= true;
> + omap->utmi_clk[i] = ERR_PTR(-ENODEV);
> + omap->hsic480m_clk[i] = ERR_PTR(-ENODEV);
> + omap->hsic60m_clk[i] = ERR_PTR(-ENODEV);
> }
>
> - omap->ehci_logic_fck = ERR_PTR(-EINVAL);
> - if (need_logic_fck) {
> - omap->ehci_logic_fck = devm_clk_get(dev, "ehci_logic_fck");
Has this clock been renamed, or is it no longer required?
Perhaps you should be explicit in the commit log as to which clocks
you're removing.
> - if (IS_ERR(omap->ehci_logic_fck)) {
> - ret = PTR_ERR(omap->ehci_logic_fck);
> - dev_dbg(dev, "ehci_logic_fck failed:%d\n", ret);
> + /* for OMAP3 i.e. USBHS REV1 */
> + if (omap->usbhs_rev == OMAP_USBHS_REV1) {
> + need_logic_fck = false;
> + for (i = 0; i < omap->nports; i++) {
> + if (is_ehci_phy_mode(pdata->port_mode[i]) ||
> + is_ehci_tll_mode(pdata->port_mode[i]) ||
> + is_ehci_hsic_mode(pdata->port_mode[i]))
> +
> + need_logic_fck |= true;
> + }
> +
> + if (need_logic_fck) {
> + omap->ehci_logic_fck = clk_get(dev, "usbhost_120m_fck");
devm_clk_get()?
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 03/12] mfd: omap-usb-host: Use clock names as per function for reference clocks
From: Lee Jones @ 2014-02-14 10:09 UTC (permalink / raw)
To: Roger Quadros
Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, bcousson-rdvid1DuHRBWk0Htik3J/w,
balbi-l0cyMroinI0, nm-l0cyMroinI0, khilman-QSEj5FYQhm4dnm+yROfE0A,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz
In-Reply-To: <1392200333-28397-4-git-send-email-rogerq-l0cyMroinI0@public.gmane.org>
> Use a meaningful name for the reference clocks so that it indicates the function.
>
> CC: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
> ---
> drivers/mfd/omap-usb-host.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 60a3bed..ce620a8 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -714,21 +714,21 @@ static int usbhs_omap_probe(struct platform_device *pdev)
> goto err_mem;
> }
>
> - omap->xclk60mhsp1_ck = devm_clk_get(dev, "xclk60mhsp1_ck");
> + omap->xclk60mhsp1_ck = devm_clk_get(dev, "refclk_60m_ext_p1");
You can't do that. These changes will have to be in the same patch as
the core change i.e. where they are initialised.
> if (IS_ERR(omap->xclk60mhsp1_ck)) {
> ret = PTR_ERR(omap->xclk60mhsp1_ck);
> dev_err(dev, "xclk60mhsp1_ck failed error:%d\n", ret);
> goto err_mem;
> }
>
> - omap->xclk60mhsp2_ck = devm_clk_get(dev, "xclk60mhsp2_ck");
> + omap->xclk60mhsp2_ck = devm_clk_get(dev, "refclk_60m_ext_p2");
> if (IS_ERR(omap->xclk60mhsp2_ck)) {
> ret = PTR_ERR(omap->xclk60mhsp2_ck);
> dev_err(dev, "xclk60mhsp2_ck failed error:%d\n", ret);
> goto err_mem;
> }
>
> - omap->init_60m_fclk = devm_clk_get(dev, "init_60m_fclk");
> + omap->init_60m_fclk = devm_clk_get(dev, "refclk_60m_int");
> if (IS_ERR(omap->init_60m_fclk)) {
> ret = PTR_ERR(omap->init_60m_fclk);
> dev_err(dev, "init_60m_fclk failed error:%d\n", ret);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 08/10] leds: Add LM3633 driver
From: Mark Rutland @ 2014-02-14 10:09 UTC (permalink / raw)
To: Milo Kim
Cc: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
Samuel Ortiz
In-Reply-To: <1392359540-7135-1-git-send-email-milo.kim@ti.com>
On Fri, Feb 14, 2014 at 06:32:20AM +0000, Milo Kim wrote:
> LM3633 LED driver supports generic LED functions and pattern generation.
> Pattern is generated by using LMU effect driver APIs.
> Sysfs documentation is added.
>
> Cc: Bryan Wu <cooloney@gmail.com>
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> ---
> Documentation/leds/leds-lm3633.txt | 38 +++
> drivers/leds/Kconfig | 10 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm3633.c | 661 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 710 insertions(+)
> create mode 100644 Documentation/leds/leds-lm3633.txt
> create mode 100644 drivers/leds/leds-lm3633.c
[...]
> +static int lm3633_led_parse_dt(struct device *dev, struct ti_lmu *lmu)
> +{
> + struct ti_lmu_led_platform_data *pdata;
> + struct device_node *node = dev->of_node;
> + struct device_node *child;
> + int num_leds;
> + int i = 0;
> + u8 imax_mA;
> +
> + if (!node) {
> + dev_err(dev, "No device node exists\n");
> + return -ENODEV;
> + }
> +
> + num_leds = of_get_child_count(node);
> + if (num_leds == 0) {
> + dev_err(dev, "No LED channels\n");
> + return -EINVAL;
> + }
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata) * num_leds, GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + for_each_child_of_node(node, child) {
> + of_property_read_string(child, "chan-name", &pdata[i].name);
What if this is missing from a node.?
> +
> + /* Make LED strings */
> + pdata[i].led_string = 0;
> + if (of_find_property(child, "lvled1-used", NULL))
> + pdata[i].led_string |= LMU_LVLED1;
> + if (of_find_property(child, "lvled2-used", NULL))
> + pdata[i].led_string |= LMU_LVLED2;
> + if (of_find_property(child, "lvled3-used", NULL))
> + pdata[i].led_string |= LMU_LVLED3;
> + if (of_find_property(child, "lvled4-used", NULL))
> + pdata[i].led_string |= LMU_LVLED4;
> + if (of_find_property(child, "lvled5-used", NULL))
> + pdata[i].led_string |= LMU_LVLED5;
> + if (of_find_property(child, "lvled6-used", NULL))
> + pdata[i].led_string |= LMU_LVLED6;
You can use of_property_read_bool for these.
> +
> + of_property_read_u8(child, "max-current-milliamp", &imax_mA);
> + pdata[i].imax = ti_lmu_get_current_code(imax_mA);
What happens if this is missing from a node?
Cheers,
Mark.
^ permalink raw reply
* Re: [PATCH 02/10] backlight: Add TI LMU backlight common driver
From: Mark Rutland @ 2014-02-14 10:13 UTC (permalink / raw)
To: Milo Kim
Cc: Lee Jones, Jingoo Han, Bryan Wu, Mark Brown,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Samuel Ortiz
In-Reply-To: <1392359468-6925-1-git-send-email-milo.kim-l0cyMroinI0@public.gmane.org>
On Fri, Feb 14, 2014 at 06:31:08AM +0000, Milo Kim wrote:
> TI LMU backlight driver provides common driver features.
> Chip specific configuration is handled by each backlight driver such like
> LM3532, LM3631, LM3633, LM3695 and LM3697.
>
> It supports common features as below.
> - Consistent device control flow
> - Control bank assignment from the platform data
> - Backlight subsystem control
> - PWM brightness control
> - Shared device tree node
>
> Cc: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Milo Kim <milo.kim-l0cyMroinI0@public.gmane.org>
> ---
> drivers/video/backlight/Kconfig | 7 +
> drivers/video/backlight/Makefile | 1 +
> drivers/video/backlight/ti-lmu-backlight.c | 369 ++++++++++++++++++++++++++++
> drivers/video/backlight/ti-lmu-backlight.h | 78 ++++++
> 4 files changed, 455 insertions(+)
> create mode 100644 drivers/video/backlight/ti-lmu-backlight.c
> create mode 100644 drivers/video/backlight/ti-lmu-backlight.h
[...]
> +static int ti_lmu_backlight_parse_dt(struct device *dev, struct ti_lmu *lmu)
> +{
> + struct ti_lmu_backlight_platform_data *pdata;
> + struct device_node *node = dev->of_node;
> + struct device_node *child;
> + int num_backlights;
> + int i = 0;
> + u8 imax_mA;
> +
> + if (!node) {
> + dev_err(dev, "No device node exists\n");
> + return -ENODEV;
> + }
> +
> + num_backlights = of_get_child_count(node);
> + if (num_backlights == 0) {
> + dev_err(dev, "No backlight strings\n");
> + return -EINVAL;
> + }
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata) * num_backlights, GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> +
> + for_each_child_of_node(node, child) {
> + of_property_read_string(child, "bl-name", &pdata[i].name);
> +
> + /* Make backlight strings */
> + pdata[i].bl_string = 0;
> + if (of_find_property(child, "hvled1-used", NULL))
> + pdata[i].bl_string |= LMU_HVLED1;
> + if (of_find_property(child, "hvled2-used", NULL))
> + pdata[i].bl_string |= LMU_HVLED2;
> + if (of_find_property(child, "hvled3-used", NULL))
> + pdata[i].bl_string |= LMU_HVLED3;
Use of_property_read_bool here.
> +
> + of_property_read_u8(child, "max-current-milliamp", &imax_mA);
> + pdata[i].imax = ti_lmu_get_current_code(imax_mA);
If this is missing from a node, imax_mA will have the value from the
last iteration (or an uninitialised value).
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox