Re: [PATCH 5/6] ARM: s3c64xx: hmt: Use PWM lookup table

[Krzysztof Kozlowski]

On 08.10.2015 00:37, Thierry Reding wrote:
> On Wed, Oct 07, 2015 at 10:37:42AM +0900, Krzysztof Kozlowski wrote:
>> On 05.10.2015 21:47, Thierry Reding wrote:
>>> Use a PWM lookup table to provide the PWM to the pwm-backlight device.
>>> The driver has a legacy code path that is required only because boards
>>> still use the legacy method of requesting PWMs by global ID. Replacing
>>> these usages allows that legacy fallback to be removed.
>>>
>>> Cc: Kukjin Kim 
>>> Cc: Krzysztof Kozlowski 
>>> Signed-off-by: Thierry Reding 
>>> ---
>>>  arch/arm/mach-s3c64xx/mach-hmt.c | 9 +++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c 
>>> b/arch/arm/mach-s3c64xx/mach-hmt.c
>>> index e4b087c58ee6..816b39d1e6d1 100644
>>> --- a/arch/arm/mach-s3c64xx/mach-hmt.c
>>> +++ b/arch/arm/mach-s3c64xx/mach-hmt.c
>>> @@ -19,6 +19,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -73,6 +74,11 @@ static struct s3c2410_uartcfg hmt_uartcfgs[] __initdata 
>>> = {
>>> },
>>>  };
>>>  
>>> +static struct pwm_lookup hmt_pwm_lookup[] = {
>>> +   PWM_LOOKUP("samsung-pwm", 1, "pwm-backlight.0", NULL,
>>
>> Same questions as in patch 2 - why suffix ".0" for "pwm-backlight"?
> 
> For the same reason that I explained in patch 2. Not sure if the .id = 0
> was really supposed to be. For most devices it would probably make sense
> to initialize it to -1 because they typically only have a single
> backlight. But given that userspace might be using the name to control
> the backlight via sysfs it's probably not a good idea to go and change
> that.

Thanks for explanation.

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] ARM: s3c64xx: hmt: Use PWM lookup table

[Thierry Reding]

On Wed, Oct 07, 2015 at 10:37:42AM +0900, Krzysztof Kozlowski wrote:
> On 05.10.2015 21:47, Thierry Reding wrote:
> > Use a PWM lookup table to provide the PWM to the pwm-backlight device.
> > The driver has a legacy code path that is required only because boards
> > still use the legacy method of requesting PWMs by global ID. Replacing
> > these usages allows that legacy fallback to be removed.
> > 
> > Cc: Kukjin Kim 
> > Cc: Krzysztof Kozlowski 
> > Signed-off-by: Thierry Reding 
> > ---
> >  arch/arm/mach-s3c64xx/mach-hmt.c | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c 
> > b/arch/arm/mach-s3c64xx/mach-hmt.c
> > index e4b087c58ee6..816b39d1e6d1 100644
> > --- a/arch/arm/mach-s3c64xx/mach-hmt.c
> > +++ b/arch/arm/mach-s3c64xx/mach-hmt.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -73,6 +74,11 @@ static struct s3c2410_uartcfg hmt_uartcfgs[] __initdata 
> > = {
> > },
> >  };
> >  
> > +static struct pwm_lookup hmt_pwm_lookup[] = {
> > +   PWM_LOOKUP("samsung-pwm", 1, "pwm-backlight.0", NULL,
> 
> Same questions as in patch 2 - why suffix ".0" for "pwm-backlight"?

For the same reason that I explained in patch 2. Not sure if the .id = 0
was really supposed to be. For most devices it would probably make sense
to initialize it to -1 because they typically only have a single
backlight. But given that userspace might be using the name to control
the backlight via sysfs it's probably not a good idea to go and change
that.

Thierry


signature.asc
Description: PGP signature

Re: [PATCH 5/6] ARM: s3c64xx: hmt: Use PWM lookup table

[Krzysztof Kozlowski]

On 05.10.2015 21:47, Thierry Reding wrote:
> Use a PWM lookup table to provide the PWM to the pwm-backlight device.
> The driver has a legacy code path that is required only because boards
> still use the legacy method of requesting PWMs by global ID. Replacing
> these usages allows that legacy fallback to be removed.
> 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Signed-off-by: Thierry Reding 
> ---
>  arch/arm/mach-s3c64xx/mach-hmt.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c 
> b/arch/arm/mach-s3c64xx/mach-hmt.c
> index e4b087c58ee6..816b39d1e6d1 100644
> --- a/arch/arm/mach-s3c64xx/mach-hmt.c
> +++ b/arch/arm/mach-s3c64xx/mach-hmt.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -73,6 +74,11 @@ static struct s3c2410_uartcfg hmt_uartcfgs[] __initdata = {
>   },
>  };
>  
> +static struct pwm_lookup hmt_pwm_lookup[] = {
> + PWM_LOOKUP("samsung-pwm", 1, "pwm-backlight.0", NULL,

Same questions as in patch 2 - why suffix ".0" for "pwm-backlight"?

Best regards,
Krzysztof

> +10 / (100 * 256 * 20), PWM_POLARITY_NORMAL),
> +};
> +
>  static int hmt_bl_init(struct device *dev)
>  {
>   int ret;
> @@ -110,10 +116,8 @@ static void hmt_bl_exit(struct device *dev)
>  }
>  
>  static struct platform_pwm_backlight_data hmt_backlight_data = {
> - .pwm_id = 1,
>   .max_brightness = 100 * 256,
>   .dft_brightness = 40 * 256,
> - .pwm_period_ns  = 10 / (100 * 256 * 20),
>   .enable_gpio= -1,
>   .init   = hmt_bl_init,
>   .notify = hmt_bl_notify,
> @@ -268,6 +272,7 @@ static void __init hmt_machine_init(void)
>   gpio_request(S3C64XX_GPF(13), "usb power");
>   gpio_direction_output(S3C64XX_GPF(13), 1);
>  
> + pwm_add_table(hmt_pwm_lookup, ARRAY_SIZE(hmt_pwm_lookup));
>   platform_add_devices(hmt_devices, ARRAY_SIZE(hmt_devices));
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] ARM: s3c64xx: hmt: Use PWM lookup table

[Thierry Reding]

Use a PWM lookup table to provide the PWM to the pwm-backlight device.
The driver has a legacy code path that is required only because boards
still use the legacy method of requesting PWMs by global ID. Replacing
these usages allows that legacy fallback to be removed.

Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Signed-off-by: Thierry Reding 
---
 arch/arm/mach-s3c64xx/mach-hmt.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c b/arch/arm/mach-s3c64xx/mach-hmt.c
index e4b087c58ee6..816b39d1e6d1 100644
--- a/arch/arm/mach-s3c64xx/mach-hmt.c
+++ b/arch/arm/mach-s3c64xx/mach-hmt.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -73,6 +74,11 @@ static struct s3c2410_uartcfg hmt_uartcfgs[] __initdata = {
},
 };
 
+static struct pwm_lookup hmt_pwm_lookup[] = {
+   PWM_LOOKUP("samsung-pwm", 1, "pwm-backlight.0", NULL,
+  10 / (100 * 256 * 20), PWM_POLARITY_NORMAL),
+};
+
 static int hmt_bl_init(struct device *dev)
 {
int ret;
@@ -110,10 +116,8 @@ static void hmt_bl_exit(struct device *dev)
 }
 
 static struct platform_pwm_backlight_data hmt_backlight_data = {
-   .pwm_id = 1,
.max_brightness = 100 * 256,
.dft_brightness = 40 * 256,
-   .pwm_period_ns  = 10 / (100 * 256 * 20),
.enable_gpio= -1,
.init   = hmt_bl_init,
.notify = hmt_bl_notify,
@@ -268,6 +272,7 @@ static void __init hmt_machine_init(void)
gpio_request(S3C64XX_GPF(13), "usb power");
gpio_direction_output(S3C64XX_GPF(13), 1);
 
+   pwm_add_table(hmt_pwm_lookup, ARRAY_SIZE(hmt_pwm_lookup));
platform_add_devices(hmt_devices, ARRAY_SIZE(hmt_devices));
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html