Hi Shawn,
On Thu, Dec 10, 2020 at 09:51:37AM +0800, Shawn Guo wrote:
On Mon, Dec 07, 2020 at 10:40:22PM -0600, Bjorn Andersson wrote:
The SN65DSI86 provides the ability to supply a PWM signal on GPIO 4, with the primary purpose of controlling the backlight of the attached panel. Add an implementation that exposes this using the standard PWM framework, to allow e.g. pwm-backlight to expose this to the user.
Special thanks to Doug Anderson for suggestions related to the involved math.
Signed-off-by: Bjorn Andersson bjorn.andersson@linaro.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 202 ++++++++++++++++++++++++++ 1 file changed, 202 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index f27306c51e4d..43c0acba57ab 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -4,6 +4,7 @@
*/
+#include <linux/atomic.h> #include <linux/bits.h> #include <linux/clk.h> #include <linux/debugfs.h> @@ -14,6 +15,7 @@ #include <linux/module.h> #include <linux/of_graph.h> #include <linux/pm_runtime.h> +#include <linux/pwm.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h>
@@ -89,6 +91,11 @@ #define SN_ML_TX_MODE_REG 0x96 #define ML_TX_MAIN_LINK_OFF 0 #define ML_TX_NORMAL_MODE BIT(0) +#define SN_PWM_PRE_DIV_REG 0xA0 +#define SN_BACKLIGHT_SCALE_REG 0xA1 +#define BACKLIGHT_SCALE_MAX 0xFFFF +#define SN_BACKLIGHT_REG 0xA3 +#define SN_PWM_EN_INV_REG 0xA5 #define SN_AUX_CMD_STATUS_REG 0xF4 #define AUX_IRQ_STATUS_AUX_RPLY_TOUT BIT(3) #define AUX_IRQ_STATUS_AUX_SHORT BIT(5) @@ -111,6 +118,8 @@
#define SN_LINK_TRAINING_TRIES 10
+#define SN_PWM_GPIO 3
So this maps to the GPIO4 described in sn65dsi86 datasheet. I'm wondering if it's more readable to define the following SHIFT constants (your code), and use GPIO_MUX_GPIO4_SHIFT >> 2 where you need GPIO offset?
#define GPIO_MUX_GPIO1_SHIFT 0 #define GPIO_MUX_GPIO2_SHIFT 2 #define GPIO_MUX_GPIO3_SHIFT 4 #define GPIO_MUX_GPIO4_SHIFT 6
If you agree, you may consider to integrate this patch beforehand:
https://github.com/shawnguo2/linux/commit/7cde887ffb3b27a36e77a08bee3666d149...
My preferred way here would be to add a prefix for the other constants. It (IMHO) looks nicer and
GPIO_INPUT_SHIFT
looks like a quite generic name for a hardware specific definition. (Even if up to now there is no other code location using this name.)
Best regards Uwe