diff mbox series

[v4,13/13] video: backlight: mt6370: Add Mediatek MT6370 support

Message ID 20220704053901.728-14-peterwu.pub@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add Mediatek MT6370 PMIC support | expand

Commit Message

ChiaEn Wu July 4, 2022, 5:39 a.m. UTC
From: ChiaEn Wu <chiaen_wu@richtek.com>

Add Mediatek MT6370 Backlight support.

Signed-off-by: ChiaEn Wu <chiaen_wu@richtek.com>
---

v4:
- Replace text "Mediatek" with "MediaTek" in Kconfig
- Add Copyright in the source code
- Revise the comment of "PWM HYS STEPS"
---
 drivers/video/backlight/Kconfig            |  12 +
 drivers/video/backlight/Makefile           |   1 +
 drivers/video/backlight/mt6370-backlight.c | 352 +++++++++++++++++++++++++++++
 3 files changed, 365 insertions(+)
 create mode 100644 drivers/video/backlight/mt6370-backlight.c

Comments

Andy Shevchenko July 4, 2022, 9:14 p.m. UTC | #1
On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
>
> From: ChiaEn Wu <chiaen_wu@richtek.com>
>
> Add Mediatek MT6370 Backlight support.

...

> +         This driver can also be built as a module. If so the module

If so,

> +         will be called "mt6370-backlight.ko".

No ".ko" part.

...

> +#include <linux/gpio/driver.h>

Can you elaborate on this?

> +#include <linux/kernel.h>
> +#include <linux/log2.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>

> +#include <linux/of.h>

Can you elaborate on this?

> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>

Missed mod_devicetable.h.

...

> +               brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
> +               brightness_val[1] = (brightness - 1)
> +                                   >> fls(MT6370_BL_DIM2_MASK);

Bad indentation. One line?

...

> +       if (priv->enable_gpio)

Dup check.

> +               gpiod_set_value(priv->enable_gpio, brightness ? 1 : 0);

...

> +       brightness = brightness_val[1] << fls(MT6370_BL_DIM2_MASK);
> +       brightness += (brightness_val[0] & MT6370_BL_DIM2_MASK);

Too many parentheses.

...

> +               /*
> +                * prop_val =  1      -->  1 steps --> 0x00
> +                * prop_val =  2 ~  4 -->  4 steps --> 0x01
> +                * prop_val =  5 ~ 16 --> 16 steps --> 0x10
> +                * prop_val = 17 ~ 64 --> 64 steps --> 0x11
> +                */
> +               prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;

Isn't something closer to get_order() or fls()?

...

> +       props->max_brightness = min_t(u32, brightness,
> +                                     MT6370_BL_MAX_BRIGHTNESS);

One line?

...

> +       val = 0;

Do you need this here?

> +       prop_val = 0;

Useless.

> +       ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
> +                                     &prop_val);
> +       if (ret) {
> +               dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
> +               return ret;
> +       }
> +
> +       if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
> +               dev_err(dev,
> +                       "No channel specified or over than upper bound (%d)\n",
> +                       prop_val);
> +               return -EINVAL;
> +       }

...

> +static int mt6370_bl_probe(struct platform_device *pdev)
> +{
> +       struct mt6370_priv *priv;
> +       struct backlight_properties props = {
> +               .type = BACKLIGHT_RAW,
> +               .scale = BACKLIGHT_SCALE_LINEAR,
> +       };
> +       int ret;

  struct device *dev = &pdev->dev;

will save you a few LoCs.
ChiaEn Wu July 13, 2022, 10:53 a.m. UTC | #2
Hi Andy,
Thanks for your reply! I have some questions want to ask you below.

Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月5日 週二 清晨5:14寫道:
>
> On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> >
> > From: ChiaEn Wu <chiaen_wu@richtek.com>
> >
> > Add Mediatek MT6370 Backlight support.
>
> ...
>
> > +         This driver can also be built as a module. If so the module
>
> If so,
>
> > +         will be called "mt6370-backlight.ko".
>
> No ".ko" part.
>
> ...
>
> > +#include <linux/gpio/driver.h>
>
> Can you elaborate on this?
>
> > +#include <linux/kernel.h>
> > +#include <linux/log2.h>
> > +#include <linux/minmax.h>
> > +#include <linux/module.h>
>
> > +#include <linux/of.h>
>
> Can you elaborate on this?
>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
>
> Missed mod_devicetable.h.
>
> ...
>
> > +               brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
> > +               brightness_val[1] = (brightness - 1)
> > +                                   >> fls(MT6370_BL_DIM2_MASK);
>
> Bad indentation. One line?

Well... if indent to one line, it will be over 80 characters(or called columns?)
From my understanding, it is not allowed, right??

>
> ...
>
> > +       if (priv->enable_gpio)
>
> Dup check.
>
> > +               gpiod_set_value(priv->enable_gpio, brightness ? 1 : 0);
>
> ...
>
> > +       brightness = brightness_val[1] << fls(MT6370_BL_DIM2_MASK);
> > +       brightness += (brightness_val[0] & MT6370_BL_DIM2_MASK);
>
> Too many parentheses.
>
> ...
>
> > +               /*
> > +                * prop_val =  1      -->  1 steps --> 0x00
> > +                * prop_val =  2 ~  4 -->  4 steps --> 0x01
> > +                * prop_val =  5 ~ 16 --> 16 steps --> 0x10
> > +                * prop_val = 17 ~ 64 --> 64 steps --> 0x11
> > +                */
> > +               prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
>
> Isn't something closer to get_order() or fls()?

I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and
this change is meet your expectations??

>
> ...
>
> > +       props->max_brightness = min_t(u32, brightness,
> > +                                     MT6370_BL_MAX_BRIGHTNESS);
>
> One line?

 Ditto, it will be over 80 characters...

>
> ...
>
> > +       val = 0;
>
> Do you need this here?
>
> > +       prop_val = 0;
>
> Useless.
>
> > +       ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
> > +                                     &prop_val);
> > +       if (ret) {
> > +               dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
> > +               return ret;
> > +       }
> > +
> > +       if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
> > +               dev_err(dev,
> > +                       "No channel specified or over than upper bound (%d)\n",
> > +                       prop_val);
> > +               return -EINVAL;
> > +       }
>
> ...
>
> > +static int mt6370_bl_probe(struct platform_device *pdev)
> > +{
> > +       struct mt6370_priv *priv;
> > +       struct backlight_properties props = {
> > +               .type = BACKLIGHT_RAW,
> > +               .scale = BACKLIGHT_SCALE_LINEAR,
> > +       };
> > +       int ret;
>
>   struct device *dev = &pdev->dev;
>
> will save you a few LoCs.
>
> --
> With Best Regards,
> Andy Shevchenko

Best regards,
ChiaEn Wu
Andy Shevchenko July 13, 2022, 12:07 p.m. UTC | #3
On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月5日 週二 清晨5:14寫道:
> > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:

Please, remove unneeded context when replying!

...

> > > +               brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
> > > +               brightness_val[1] = (brightness - 1)
> > > +                                   >> fls(MT6370_BL_DIM2_MASK);
> >
> > Bad indentation. One line?
>
> Well... if indent to one line, it will be over 80 characters(or called columns?)
> From my understanding, it is not allowed, right??

It's allowed to some extent.Use your common sense.
Here it's obviously broken indentation.

...

> > > +               prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
> >
> > Isn't something closer to get_order() or fls()?
>
> I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and
> this change is meet your expectations??

Nope. Try again. What about fls()?

...

> > > +       props->max_brightness = min_t(u32, brightness,
> > > +                                     MT6370_BL_MAX_BRIGHTNESS);
> >
> > One line?
>
>  Ditto, it will be over 80 characters...

As per above.
Lee Jones July 13, 2022, 1:07 p.m. UTC | #4
On Wed, 13 Jul 2022, Andy Shevchenko wrote:

> On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月5日 週二 清晨5:14寫道:
> > > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> 
> Please, remove unneeded context when replying!
> 
> ...
> 
> > > > +               brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
> > > > +               brightness_val[1] = (brightness - 1)
> > > > +                                   >> fls(MT6370_BL_DIM2_MASK);
> > >
> > > Bad indentation. One line?
> >
> > Well... if indent to one line, it will be over 80 characters(or called columns?)
> > From my understanding, it is not allowed, right??
> 
> It's allowed to some extent.Use your common sense.
> Here it's obviously broken indentation.

Refrain from going crazy though - hard limit is 100 chars.
ChiaEn Wu July 14, 2022, 7:13 a.m. UTC | #5
Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月13日 週三 晚上8:07寫道:
>
> On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月5日 週二 清晨5:14寫道:
> > > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
>
> Please, remove unneeded context when replying!
>
> ...
>
> > > > +               brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
> > > > +               brightness_val[1] = (brightness - 1)
> > > > +                                   >> fls(MT6370_BL_DIM2_MASK);
> > >
> > > Bad indentation. One line?
> >
> > Well... if indent to one line, it will be over 80 characters(or called columns?)
> > From my understanding, it is not allowed, right??
>
> It's allowed to some extent.Use your common sense.
> Here it's obviously broken indentation.
>
> ...
>
> > > > +               prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
> > >
> > > Isn't something closer to get_order() or fls()?
> >
> > I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and
> > this change is meet your expectations??
>
> Nope. Try again. What about fls()?

I have tried two methods so far, as follows
-------------------------------------------------------------
/*
 * prop_val =  1      -->  1 steps --> b'00
 * prop_val =  2 ~  4 -->  4 steps --> b'01
 * prop_val =  5 ~ 16 --> 16 steps --> b'10
 * prop_val = 17 ~ 64 --> 64 steps --> b'11
*/

// 1. use fls() and ffs() combination
prop_val = ffs(prop_val) == fls(prop_val) ? fls(prop_val) >> 1 :
(fls(prop_val) + 1) >> 1;

// 2. use one line for-loop, but without fls()
for (i = --prop_val, prop_val = 0; i >> 2 * prop_val != 0; prop_val++);
-------------------------------------------------------------
Do these changes meet your expectations??

>
> ...
>
> > > > +       props->max_brightness = min_t(u32, brightness,
> > > > +                                     MT6370_BL_MAX_BRIGHTNESS);
> > >
> > > One line?
> >
> >  Ditto, it will be over 80 characters...
>
> As per above.
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko July 14, 2022, 9:27 a.m. UTC | #6
On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月13日 週三 晚上8:07寫道:
> > On Wed, Jul 13, 2022 at 12:53 PM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月5日 週二 清晨5:14寫道:
> > > > On Mon, Jul 4, 2022 at 7:43 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:

Please, once again, remove unneeded context when replying!
^^^^^^^

...

> > > > > +               prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
> > > >
> > > > Isn't something closer to get_order() or fls()?
> > >
> > > I will revise it to "(get_order(prop_va * PAGE_SIZE) + 1) / 2" and
> > > this change is meet your expectations??
> >
> > Nope. Try again. What about fls()?
>
> I have tried two methods so far, as follows
> -------------------------------------------------------------
> /*
>  * prop_val =  1      -->  1 steps --> b'00
>  * prop_val =  2 ~  4 -->  4 steps --> b'01
>  * prop_val =  5 ~ 16 --> 16 steps --> b'10
>  * prop_val = 17 ~ 64 --> 64 steps --> b'11
> */

So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3.
Now, consider x - 1:
0  ( 0 ) --> 0
1  (2^0) --> 1
4  (2^2) --> 2
16 (2^4) --> 3
64 (2^6) --> ? (but let's consider that the range has been checked already)

Since we take the lower limit, it means ffs():

  y = (ffs(x - 1) + 1) / 2;

Does it work for you?

> // 1. use fls() and ffs() combination
> prop_val = ffs(prop_val) == fls(prop_val) ? fls(prop_val) >> 1 :
> (fls(prop_val) + 1) >> 1;
>
> // 2. use one line for-loop, but without fls()
> for (i = --prop_val, prop_val = 0; i >> 2 * prop_val != 0; prop_val++);
> -------------------------------------------------------------
> Do these changes meet your expectations??

No, this is ugly. Yes, I understand that a bit arithmetics is hard...
Andy Shevchenko July 14, 2022, 9:43 a.m. UTC | #7
On Thu, Jul 14, 2022 at 11:27 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月13日 週三 晚上8:07寫道:

...

> > I have tried two methods so far, as follows
> > -------------------------------------------------------------
> > /*
> >  * prop_val =  1      -->  1 steps --> b'00
> >  * prop_val =  2 ~  4 -->  4 steps --> b'01
> >  * prop_val =  5 ~ 16 --> 16 steps --> b'10
> >  * prop_val = 17 ~ 64 --> 64 steps --> b'11
> > */
>
> So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3.
> Now, consider x - 1:
> 0  ( 0 ) --> 0
> 1  (2^0) --> 1
> 4  (2^2) --> 2
> 16 (2^4) --> 3
> 64 (2^6) --> ? (but let's consider that the range has been checked already)
>
> Since we take the lower limit, it means ffs():
>
>   y = (ffs(x - 1) + 1) / 2;
>
> Does it work for you?

It wouldn't, because we need to use fls() against it actually.

So,
0..1   (-1..0)   --> 0
2..4   (1..3)   --> 1
5..16  (4..15)  --> 2
17..64 (16..63) --> 3

y = x ? ((fls(x - 1) + 1) / 2 : 0;
Daniel Thompson July 14, 2022, 9:47 a.m. UTC | #8
On Thu, Jul 14, 2022 at 11:27:07AM +0200, Andy Shevchenko wrote:
> On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > I have tried two methods so far, as follows
> > -------------------------------------------------------------
> > /*
> >  * prop_val =  1      -->  1 steps --> b'00
> >  * prop_val =  2 ~  4 -->  4 steps --> b'01
> >  * prop_val =  5 ~ 16 --> 16 steps --> b'10
> >  * prop_val = 17 ~ 64 --> 64 steps --> b'11
> > */
>
> So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3.
> Now, consider x - 1:
> 0  ( 0 ) --> 0
> 1  (2^0) --> 1
> 4  (2^2) --> 2
> 16 (2^4) --> 3
> 64 (2^6) --> ? (but let's consider that the range has been checked already)
>
> Since we take the lower limit, it means ffs():
>
>   y = (ffs(x - 1) + 1) / 2;
>
> Does it work for you?

To be honest, for this tiny table, writing code that *doesn't* require intricate
deciphering together with a huge comment saying what is does would probably be
better:

		prop_val = (prop_val <=  1 ? 0 :
		            prop_val <=  4 ? 1 :
			    prop_val <= 16 ? 2 :
			                     3);

This would be "obviously correct" and require no comment.


Daniel.
Andy Shevchenko July 14, 2022, 9:49 a.m. UTC | #9
On Thu, Jul 14, 2022 at 11:47 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Thu, Jul 14, 2022 at 11:27:07AM +0200, Andy Shevchenko wrote:
> > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > > I have tried two methods so far, as follows
> > > -------------------------------------------------------------
> > > /*
> > >  * prop_val =  1      -->  1 steps --> b'00
> > >  * prop_val =  2 ~  4 -->  4 steps --> b'01
> > >  * prop_val =  5 ~ 16 --> 16 steps --> b'10
> > >  * prop_val = 17 ~ 64 --> 64 steps --> b'11
> > > */
> >
> > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3.
> > Now, consider x - 1:
> > 0  ( 0 ) --> 0
> > 1  (2^0) --> 1
> > 4  (2^2) --> 2
> > 16 (2^4) --> 3
> > 64 (2^6) --> ? (but let's consider that the range has been checked already)
> >
> > Since we take the lower limit, it means ffs():
> >
> >   y = (ffs(x - 1) + 1) / 2;
> >
> > Does it work for you?
>
> To be honest, for this tiny table, writing code that *doesn't* require intricate
> deciphering together with a huge comment saying what is does would probably be
> better:
>
>                 prop_val = (prop_val <=  1 ? 0 :
>                             prop_val <=  4 ? 1 :
>                             prop_val <= 16 ? 2 :
>                                              3);
>
> This would be "obviously correct" and require no comment.

Agree. It will also limit checking (and whatever needed for that).
Andy Shevchenko July 14, 2022, 10:15 a.m. UTC | #10
On Thu, Jul 14, 2022 at 11:43 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jul 14, 2022 at 11:27 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jul 14, 2022 at 9:13 AM ChiaEn Wu <peterwu.pub@gmail.com> wrote:
> > > Andy Shevchenko <andy.shevchenko@gmail.com> 於 2022年7月13日 週三 晚上8:07寫道:

...

> > >  * prop_val =  1      -->  1 steps --> b'00
> > >  * prop_val =  2 ~  4 -->  4 steps --> b'01
> > >  * prop_val =  5 ~ 16 --> 16 steps --> b'10
> > >  * prop_val = 17 ~ 64 --> 64 steps --> b'11
> >
> > So, for 1 --> 0, for 2 --> 1, for 5 --> 2, and for 17 --> 3.
> > Now, consider x - 1:
> > 0  ( 0 ) --> 0
> > 1  (2^0) --> 1
> > 4  (2^2) --> 2
> > 16 (2^4) --> 3
> > 64 (2^6) --> ? (but let's consider that the range has been checked already)
> >
> > Since we take the lower limit, it means ffs():
> >
> >   y = (ffs(x - 1) + 1) / 2;
> >
> > Does it work for you?
>
> It wouldn't, because we need to use fls() against it actually.
>
> So,
> 0..1   (-1..0)   --> 0
> 2..4   (1..3)   --> 1
> 5..16  (4..15)  --> 2
> 17..64 (16..63) --> 3
>
> y = x ? ((fls(x - 1) + 1) / 2 : 0;

Okay, I nailed it down, but Daniel is right, it's simpler to have just
conditionals.

y = x >=2 ? __fls(x - 1) / 2 + 1 : 0;


--
With Best Regards,
Andy Shevchenko
diff mbox series

Patch

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index a003e02..099b0d0 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -268,6 +268,18 @@  config BACKLIGHT_MAX8925
 	  If you have a LCD backlight connected to the WLED output of MAX8925
 	  WLED output, say Y here to enable this driver.
 
+config BACKLIGHT_MT6370
+	tristate "MediaTek MT6370 Backlight Driver"
+	depends on MFD_MT6370
+	help
+	  This enables support for Mediatek MT6370 Backlight driver.
+	  It's commonly used to drive the display WLED. There are 4 channels
+	  inside, and each channel supports up to 30mA of current capability
+	  with 2048 current steps in exponential or linear mapping curves.
+
+	  This driver can also be built as a module. If so the module
+	  will be called "mt6370-backlight.ko".
+
 config BACKLIGHT_APPLE
 	tristate "Apple Backlight Driver"
 	depends on X86 && ACPI
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index cae2c83..e815f3f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -44,6 +44,7 @@  obj-$(CONFIG_BACKLIGHT_LP855X)		+= lp855x_bl.o
 obj-$(CONFIG_BACKLIGHT_LP8788)		+= lp8788_bl.o
 obj-$(CONFIG_BACKLIGHT_LV5207LP)	+= lv5207lp.o
 obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
+obj-$(CONFIG_BACKLIGHT_MT6370)		+= mt6370-backlight.o
 obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
 obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
 obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
diff --git a/drivers/video/backlight/mt6370-backlight.c b/drivers/video/backlight/mt6370-backlight.c
new file mode 100644
index 0000000..655f585
--- /dev/null
+++ b/drivers/video/backlight/mt6370-backlight.c
@@ -0,0 +1,352 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Richtek Technology Corp.
+ *
+ * Author: ChiaEn Wu <chiaen_wu@richtek.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/kernel.h>
+#include <linux/log2.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MT6370_REG_DEV_INFO		0x100
+#define MT6370_REG_BL_EN		0x1A0
+#define MT6370_REG_BL_BSTCTRL		0x1A1
+#define MT6370_REG_BL_PWM		0x1A2
+#define MT6370_REG_BL_DIM2		0x1A4
+
+#define MT6370_VENID_MASK		GENMASK(7, 4)
+#define MT6370_BL_EXT_EN_MASK		BIT(7)
+#define MT6370_BL_EN_MASK		BIT(6)
+#define MT6370_BL_CONFIG_MASK		BIT(0)
+#define MT6370_BL_CH_MASK		GENMASK(5, 2)
+#define MT6370_BL_DIM2_MASK		GENMASK(2, 0)
+#define MT6370_BL_DUMMY_6372_MASK	GENMASK(2, 0)
+#define MT6370_BL_DIM2_6372_SHIFT	3
+#define MT6370_BL_PWM_EN_MASK		BIT(7)
+#define MT6370_BL_PWM_HYS_EN_MASK	BIT(2)
+#define MT6370_BL_PWM_HYS_SEL_MASK	GENMASK(1, 0)
+#define MT6370_BL_OVP_EN_MASK		BIT(7)
+#define MT6370_BL_OVP_SEL_MASK		GENMASK(6, 5)
+#define MT6370_BL_OC_EN_MASK		BIT(3)
+#define MT6370_BL_OC_SEL_MASK		GENMASK(2, 1)
+
+#define MT6370_BL_PWM_HYS_TH_MIN_STEP	1
+#define MT6370_BL_PWM_HYS_TH_MAX_STEP	64
+#define MT6370_BL_OVP_MIN_UV		17000000
+#define MT6370_BL_OVP_MAX_UV		29000000
+#define MT6370_BL_OVP_STEP_UV		4000000
+#define MT6370_BL_OCP_MIN_UA		900000
+#define MT6370_BL_OCP_MAX_UA		1800000
+#define MT6370_BL_OCP_STEP_UA		300000
+#define MT6370_BL_MAX_BRIGHTNESS	2048
+#define MT6370_BL_MAX_CH		15
+
+enum {
+	MT6370_VID_COMMON = 0,
+	MT6370_VID_6372,
+};
+
+struct mt6370_priv {
+	int vid_type;
+	struct backlight_device *bl;
+	struct device *dev;
+	struct gpio_desc *enable_gpio;
+	struct regmap *regmap;
+};
+
+static int mt6370_bl_update_status(struct backlight_device *bl_dev)
+{
+	struct mt6370_priv *priv = bl_get_data(bl_dev);
+	int brightness = backlight_get_brightness(bl_dev);
+	unsigned int enable_val;
+	u8 brightness_val[2];
+	int ret;
+
+	if (brightness) {
+		brightness_val[0] = (brightness - 1) & MT6370_BL_DIM2_MASK;
+		brightness_val[1] = (brightness - 1)
+				    >> fls(MT6370_BL_DIM2_MASK);
+
+		/*
+		 * To make MT6372 using 14 bits to control the brightness
+		 * backward compatible with 11 bits brightness control
+		 * (like MT6370 and MT6371 do), we left shift the value
+		 * and pad with 1 to remaining bits. Hence, the MT6372's
+		 * backlight brightness will be almost the same as MT6370's
+		 * and MT6371's.
+		 */
+		if (priv->vid_type == MT6370_VID_6372) {
+			brightness_val[0] <<= MT6370_BL_DIM2_6372_SHIFT;
+			brightness_val[0] |= MT6370_BL_DUMMY_6372_MASK;
+		}
+
+		ret = regmap_raw_write(priv->regmap, MT6370_REG_BL_DIM2,
+				       brightness_val, sizeof(brightness_val));
+		if (ret)
+			return ret;
+	}
+
+	if (priv->enable_gpio)
+		gpiod_set_value(priv->enable_gpio, brightness ? 1 : 0);
+
+	enable_val = brightness ? MT6370_BL_EN_MASK : 0;
+	return regmap_update_bits(priv->regmap, MT6370_REG_BL_EN,
+				  MT6370_BL_EN_MASK, enable_val);
+}
+
+static int mt6370_bl_get_brightness(struct backlight_device *bl_dev)
+{
+	struct mt6370_priv *priv = bl_get_data(bl_dev);
+	unsigned int enable;
+	u8 brightness_val[2];
+	int brightness, ret;
+
+	ret = regmap_read(priv->regmap, MT6370_REG_BL_EN, &enable);
+	if (ret)
+		return ret;
+
+	if (!(enable & MT6370_BL_EN_MASK))
+		return 0;
+
+	ret = regmap_raw_read(priv->regmap, MT6370_REG_BL_DIM2,
+			      brightness_val, sizeof(brightness_val));
+	if (ret)
+		return ret;
+
+	if (priv->vid_type == MT6370_VID_6372)
+		brightness_val[0] >>= MT6370_BL_DIM2_6372_SHIFT;
+
+	brightness = brightness_val[1] << fls(MT6370_BL_DIM2_MASK);
+	brightness += (brightness_val[0] & MT6370_BL_DIM2_MASK);
+
+	return brightness + 1;
+}
+
+static const struct backlight_ops mt6370_bl_ops = {
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = mt6370_bl_update_status,
+	.get_brightness = mt6370_bl_get_brightness,
+};
+
+static int mt6370_init_backlight_properties(struct mt6370_priv *priv,
+					    struct backlight_properties *props)
+{
+	struct device *dev = priv->dev;
+	u8 prop_val;
+	u32 brightness, ovp_uV, ocp_uA;
+	unsigned int mask, val;
+	int ret;
+
+	/* Vendor optional properties */
+	val = 0;
+	if (device_property_read_bool(dev, "mediatek,bled-pwm-enable"))
+		val |= MT6370_BL_PWM_EN_MASK;
+
+	if (device_property_read_bool(dev, "mediatek,bled-pwm-hys-enable"))
+		val |= MT6370_BL_PWM_HYS_EN_MASK;
+
+	ret = device_property_read_u8(dev,
+				      "mediatek,bled-pwm-hys-input-th-steps",
+				      &prop_val);
+	if (!ret) {
+		prop_val = clamp_val(prop_val,
+				     MT6370_BL_PWM_HYS_TH_MIN_STEP,
+				     MT6370_BL_PWM_HYS_TH_MAX_STEP);
+		/*
+		 * prop_val =  1      -->  1 steps --> 0x00
+		 * prop_val =  2 ~  4 -->  4 steps --> 0x01
+		 * prop_val =  5 ~ 16 --> 16 steps --> 0x10
+		 * prop_val = 17 ~ 64 --> 64 steps --> 0x11
+		 */
+		prop_val = (ilog2(roundup_pow_of_two(prop_val)) + 1) >> 1;
+		val |= prop_val << (ffs(MT6370_BL_PWM_HYS_SEL_MASK) - 1);
+	}
+
+	ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_PWM,
+				 val, val);
+	if (ret)
+		return ret;
+
+	val = 0;
+	if (device_property_read_bool(dev, "mediatek,bled-ovp-shutdown"))
+		val |= MT6370_BL_OVP_EN_MASK;
+
+	ret = device_property_read_u32(dev, "mediatek,bled-ovp-microvolt",
+				       &ovp_uV);
+	if (!ret) {
+		ovp_uV = clamp_val(ovp_uV, MT6370_BL_OVP_MIN_UV,
+				   MT6370_BL_OVP_MAX_UV);
+		ovp_uV = DIV_ROUND_UP(ovp_uV - MT6370_BL_OVP_MIN_UV,
+				      MT6370_BL_OVP_STEP_UV);
+		val |= ovp_uV << (ffs(MT6370_BL_OVP_SEL_MASK) - 1);
+	}
+
+	if (device_property_read_bool(dev, "mediatek,bled-ocp-shutdown"))
+		val |= MT6370_BL_OC_EN_MASK;
+
+	ret = device_property_read_u32(dev, "mediatek,bled-ocp-microamp",
+				       &ocp_uA);
+	if (!ret) {
+		ocp_uA = clamp_val(ocp_uA, MT6370_BL_OCP_MIN_UA,
+				   MT6370_BL_OCP_MAX_UA);
+		ocp_uA = DIV_ROUND_UP(ocp_uA - MT6370_BL_OCP_MIN_UA,
+				      MT6370_BL_OCP_STEP_UA);
+		val |= ocp_uA << (ffs(MT6370_BL_OC_SEL_MASK) - 1);
+	}
+
+	ret = regmap_update_bits(priv->regmap, MT6370_REG_BL_BSTCTRL,
+				 val, val);
+	if (ret)
+		return ret;
+
+	/* Common properties */
+	ret = device_property_read_u32(dev, "max-brightness", &brightness);
+	if (ret)
+		brightness = MT6370_BL_MAX_BRIGHTNESS;
+
+	props->max_brightness = min_t(u32, brightness,
+				      MT6370_BL_MAX_BRIGHTNESS);
+
+	ret = device_property_read_u32(dev, "default-brightness", &brightness);
+	if (ret)
+		brightness = props->max_brightness;
+
+	props->brightness = min_t(u32, brightness, props->max_brightness);
+
+	val = 0;
+	prop_val = 0;
+	ret = device_property_read_u8(dev, "mediatek,bled-channel-use",
+				      &prop_val);
+	if (ret) {
+		dev_err(dev, "mediatek,bled-channel-use DT property missing\n");
+		return ret;
+	}
+
+	if (!prop_val || prop_val > MT6370_BL_MAX_CH) {
+		dev_err(dev,
+			"No channel specified or over than upper bound (%d)\n",
+			prop_val);
+		return -EINVAL;
+	}
+
+	mask = MT6370_BL_EXT_EN_MASK | MT6370_BL_CH_MASK;
+	val = prop_val << (ffs(MT6370_BL_CH_MASK) - 1);
+
+	if (priv->enable_gpio)
+		val |= MT6370_BL_EXT_EN_MASK;
+
+	return regmap_update_bits(priv->regmap, MT6370_REG_BL_EN, mask, val);
+}
+
+static int mt6370_check_vendor_info(struct mt6370_priv *priv)
+{
+	/*
+	 * MT6372 uses 14 bits to control the brightness but MT6370 and MT6371
+	 * use 11 bits. They are different so we have to use this function to
+	 * check the vendor ID and use different methods to calculate the
+	 * brightness.
+	 */
+	unsigned int dev_info, vid;
+	int ret;
+
+	ret = regmap_read(priv->regmap, MT6370_REG_DEV_INFO, &dev_info);
+	if (ret)
+		return ret;
+
+	vid = FIELD_GET(MT6370_VENID_MASK, dev_info);
+	if (vid == 0x9 || vid == 0xb)
+		priv->vid_type = MT6370_VID_6372;
+	else
+		priv->vid_type = MT6370_VID_COMMON;
+
+	return 0;
+}
+
+static int mt6370_bl_probe(struct platform_device *pdev)
+{
+	struct mt6370_priv *priv;
+	struct backlight_properties props = {
+		.type = BACKLIGHT_RAW,
+		.scale = BACKLIGHT_SCALE_LINEAR,
+	};
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	priv->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (!priv->regmap)
+		return dev_err_probe(&pdev->dev, -ENODEV,
+				     "Failed to get regmap\n");
+
+	ret = mt6370_check_vendor_info(priv);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to check vendor info\n");
+
+	priv->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->enable_gpio))
+		dev_err(&pdev->dev, "Failed to get 'enable' gpio\n");
+
+	ret = mt6370_init_backlight_properties(priv, &props);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to init backlight properties\n");
+
+	priv->bl = devm_backlight_device_register(&pdev->dev, pdev->name,
+						  &pdev->dev, priv,
+						  &mt6370_bl_ops, &props);
+	if (IS_ERR(priv->bl))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->bl),
+				     "Failed to register backlight\n");
+
+	backlight_update_status(priv->bl);
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int mt6370_bl_remove(struct platform_device *pdev)
+{
+	struct mt6370_priv *priv = platform_get_drvdata(pdev);
+	struct backlight_device *bl_dev = priv->bl;
+
+	bl_dev->props.brightness = 0;
+	backlight_update_status(priv->bl);
+
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused mt6370_bl_of_match[] = {
+	{ .compatible = "mediatek,mt6370-backlight", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mt6370_bl_of_match);
+
+static struct platform_driver mt6370_bl_driver = {
+	.driver = {
+		.name = "mt6370-backlight",
+		.of_match_table = mt6370_bl_of_match,
+	},
+	.probe = mt6370_bl_probe,
+	.remove = mt6370_bl_remove,
+};
+module_platform_driver(mt6370_bl_driver);
+
+MODULE_AUTHOR("ChiaEn Wu <chiaen_wu@richtek.com>");
+MODULE_DESCRIPTION("MediaTek MT6370 Backlight Driver");
+MODULE_LICENSE("GPL v2");