Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v4 04/24] hwmon: pwm-fan: use pwm_get_xxx() helpers where appropriate
From: Guenter Roeck @ 2015-11-16 15:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1447664207-24370-5-git-send-email-boris.brezillon@free-electrons.com>

On 11/16/2015 12:56 AM, Boris Brezillon wrote:
> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> Doing that will ease adaptation of the PWM framework to support atomic
> update.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Patch generated with the following coccinelle script:
>
> --->8---
> virtual patch
>
> @@
> struct pwm_device *p;
> expression e;
> @@
> (
> -(p)->polarity = e;
> +pwm_set_polarity((p), e);
> |
> -(p)->polarity
> +pwm_get_polarity((p))

s/((p))/(p)/

> |
> -(p)->period = e;
> +pwm_set_period((p), e);
> |
> -(p)->period
> +pwm_get_period((p))

s/((p))/(p)/

> |
> -(p)->duty_cycle = e;
> +pwm_set_duty_cycle((p), e);

The (p) seems unnecessary here.

> |
> -(p)->duty_cycle
> +pwm_get_duty_cycle((p))

s/((p))/(p)/

> )
> --->8---
> ---
>   drivers/hwmon/pwm-fan.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 3e23003..105b964 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -47,8 +47,8 @@ static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
>   	if (ctx->pwm_value = pwm)
>   		goto exit_set_pwm_err;
>
> -	duty = DIV_ROUND_UP(pwm * (ctx->pwm->period - 1), MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, ctx->pwm->period);
> +	duty = DIV_ROUND_UP(pwm * (pwm_get_period((ctx->pwm)) - 1), MAX_PWM);
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));

Please drop all the unnecessary ( ).

Thanks,
Guenter

>   	if (ret)
>   		goto exit_set_pwm_err;
>
> @@ -234,10 +234,10 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, ctx);
>
>   	/* Set duty cycle to maximum allowed */
> -	duty_cycle = ctx->pwm->period - 1;
> +	duty_cycle = pwm_get_period((ctx->pwm)) - 1;
>   	ctx->pwm_value = MAX_PWM;
>
> -	ret = pwm_config(ctx->pwm, duty_cycle, ctx->pwm->period);
> +	ret = pwm_config(ctx->pwm, duty_cycle, pwm_get_period((ctx->pwm)));
>   	if (ret) {
>   		dev_err(&pdev->dev, "Failed to configure PWM\n");
>   		return ret;
> @@ -309,8 +309,9 @@ static int pwm_fan_resume(struct device *dev)
>   	if (ctx->pwm_value = 0)
>   		return 0;
>
> -	duty = DIV_ROUND_UP(ctx->pwm_value * (ctx->pwm->period - 1), MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, ctx->pwm->period);
> +	duty = DIV_ROUND_UP(ctx->pwm_value * (pwm_get_period((ctx->pwm)) - 1),
> +			    MAX_PWM);
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
>   	if (ret)
>   		return ret;
>   	return pwm_enable(ctx->pwm);
>


^ permalink raw reply

* Re: [PATCH v4 05/24] misc: max77693-haptic: use pwm_get_xxx() helpers where appropriate
From: Boris Brezillon @ 2015-11-16 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5649FC75.8020203@roeck-us.net>

On Mon, 16 Nov 2015 07:55:33 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On 11/16/2015 05:55 AM, Boris Brezillon wrote:
> > Hi Krzysztof,
> >
> > On Mon, 16 Nov 2015 22:10:40 +0900
> > Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> >
> >> W dniu 16.11.2015 o 17:56, Boris Brezillon pisze:
> >>> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> >>> Doing that will ease adaptation of the PWM framework to support atomic
> >>> update.
> >>>
> >>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> >>> ---
> >>> Patch generated with the following coccinelle script:
> >>>
> >>> --->8---
> >>> virtual patch
> >>>
> >>> @@
> >>> struct pwm_device *p;
> >>> expression e;
> >>> @@
> >>> (
> >>> -(p)->polarity = e;
> >>> +pwm_set_polarity((p), e);
> >>> |
> >>> -(p)->polarity
> >>> +pwm_get_polarity((p))
> >>> |
> >>> -(p)->period = e;
> >>> +pwm_set_period((p), e);
> >>> |
> >>> -(p)->period
> >>> +pwm_get_period((p))
> >>> |
> >>> -(p)->duty_cycle = e;
> >>> +pwm_set_duty_cycle((p), e);
> >>> |
> >>> -(p)->duty_cycle
> >>> +pwm_get_duty_cycle((p))
> >>> )
> >>> --->8---
> >>> ---
> >>>   drivers/input/misc/max77693-haptic.c | 7 ++++---
> >>>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
> >>> index 6d96bff..a038fb3 100644
> >>> --- a/drivers/input/misc/max77693-haptic.c
> >>> +++ b/drivers/input/misc/max77693-haptic.c
> >>> @@ -70,10 +70,11 @@ struct max77693_haptic {
> >>>
> >>>   static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
> >>>   {
> >>> -	int delta = (haptic->pwm_dev->period + haptic->pwm_duty) / 2;
> >>> +	int delta = (pwm_get_period((haptic->pwm_dev)) + haptic->pwm_duty) / 2;
> >>
> >> Double parentheses over argument are not needed so just:
> >> pwm_get_period(haptic->pwm_dev) + ...
> >
> > Actually it was generated with coccinelle, hence I didn't fix existing
> > coding style issues, but I have no problem fixing them.
> >
> There was no existing coding style issue. Your coccinelle script introduces it.
> You might want to consider updating your script and remove the unnecessary (( ))
> from it.

My bad, you are right: my script is buggy. I'll fix that.

Thanks,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v4 13/24] hwmon: pwm-fan: use pwm_get/set_default_xxx() helpers where appropriate
From: Guenter Roeck @ 2015-11-16 16:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1447664207-24370-14-git-send-email-boris.brezillon@free-electrons.com>

On 11/16/2015 12:56 AM, Boris Brezillon wrote:
> pwm_set/get_default_xxx() helpers have been introduced to differentiate
> the default PWM states (those retrieved through DT, PWM lookup table or
> statically assigned by the driver) and the current ones.
> Make use of those helpers where appropriate.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>   drivers/hwmon/pwm-fan.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 105b964..989d7b4 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -47,8 +47,9 @@ static int  __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
>   	if (ctx->pwm_value = pwm)
>   		goto exit_set_pwm_err;
>
> -	duty = DIV_ROUND_UP(pwm * (pwm_get_period((ctx->pwm)) - 1), MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
> +	duty = DIV_ROUND_UP(pwm * (pwm_get_default_period((ctx->pwm)) - 1),
> +			    MAX_PWM);
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));

Same here - please no unnecessary ( ) [ which were first introduced by
the other patch ].

Guenter

>   	if (ret)
>   		goto exit_set_pwm_err;
>
> @@ -234,10 +235,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, ctx);
>
>   	/* Set duty cycle to maximum allowed */
> -	duty_cycle = pwm_get_period((ctx->pwm)) - 1;
> +	duty_cycle = pwm_get_default_period((ctx->pwm)) - 1;
>   	ctx->pwm_value = MAX_PWM;
>
> -	ret = pwm_config(ctx->pwm, duty_cycle, pwm_get_period((ctx->pwm)));
> +	ret = pwm_config(ctx->pwm, duty_cycle,
> +			 pwm_get_default_period((ctx->pwm)));
>   	if (ret) {
>   		dev_err(&pdev->dev, "Failed to configure PWM\n");
>   		return ret;
> @@ -309,9 +311,10 @@ static int pwm_fan_resume(struct device *dev)
>   	if (ctx->pwm_value = 0)
>   		return 0;
>
> -	duty = DIV_ROUND_UP(ctx->pwm_value * (pwm_get_period((ctx->pwm)) - 1),
> +	duty = DIV_ROUND_UP(ctx->pwm_value *
> +			    (pwm_get_default_period((ctx->pwm)) - 1),
>   			    MAX_PWM);
> -	ret = pwm_config(ctx->pwm, duty, pwm_get_period((ctx->pwm)));
> +	ret = pwm_config(ctx->pwm, duty, pwm_get_default_period((ctx->pwm)));
>   	if (ret)
>   		return ret;
>   	return pwm_enable(ctx->pwm);
>


^ permalink raw reply

* Re: [PATCH] [media] hdmi: added functions for MPEG InfoFrames
From: Enric Balletbo Serra @ 2015-11-16 16:28 UTC (permalink / raw)
  To: Thierry Reding
  Cc: linux-fbdev, Mauro Carvalho Chehab, linux-kernel, dri-devel,
	Tomi Valkeinen, Hans Verkuil, Martin Bugge,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <20151116115037.GF31033@ulmo.nvidia.com>

Hello Thierry,

Many thanks for your comments.

2015-11-16 12:50 GMT+01:00 Thierry Reding <treding@nvidia.com>:
> On Sat, Nov 14, 2015 at 07:38:19PM +0100, Enric Balletbo i Serra wrote:
>> The MPEG Source (MS) InfoFrame is in EIA/CEA-861B. It describes aspects of
>> the compressed video stream that were used to produce the uncompressed
>> video.
>>
>> The patch adds functions to work with MPEG InfoFrames.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>  drivers/video/hdmi.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/hdmi.h |  24 ++++++++
>>  2 files changed, 180 insertions(+)
>
> According to the CEA specification a source is expected to send this
> type of infoframe once per video frame. I'm curious how you envision
> this to be ensured. Would hardware provide a mechanism to store this
> data and send the infoframe automatically? How would you ensure that
> updates sent to the hardware match the upcoming frame?
>

To be honest I'm not sure if I have the full picture. In the use case
I'm trying there is a hardware mechanism to store the data and send
the infoframe through a "Packet Send Control Register".

>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> [...]
>> @@ -435,6 +510,8 @@ hdmi_infoframe_pack(union hdmi_infoframe *frame, void *buffer, size_t size)
>>               length = hdmi_vendor_any_infoframe_pack(&frame->vendor,
>>                                                       buffer, size);
>>               break;
>> +     case HDMI_INFOFRAME_TYPE_MPEG:
>> +             length = hdmi_mpeg_infoframe_pack(&frame->mpeg, buffer, size);
>
> Missing "break;"?
>

Ack.

>> @@ -899,6 +978,41 @@ static void hdmi_audio_infoframe_log(const char *level,
>>                       frame->downmix_inhibit ? "Yes" : "No");
>>  }
>>
>> +static const char *hdmi_mpeg_picture_get_name(enum hdmi_mpeg_picture_type type)
>> +{
>> +     switch (type) {
>> +     case HDMI_MPEG_PICTURE_TYPE_UNKNOWN:
>> +             return "Unknown";
>> +     case HDMI_MPEG_PICTURE_TYPE_I:
>> +             return "Intra-coded picture";
>> +     case HDMI_MPEG_PICTURE_TYPE_B:
>> +             return "Bi-predictive picture";
>> +     case HDMI_MPEG_PICTURE_TYPE_P:
>> +             return "Predicted picture";
>> +     }
>
> I'd have chosen the slightly more canonical "I-Frame", "P-Frame",
> "B-Frame" here, but that's not a strong objection.
>

I don't have any inconvenient to change, are the following names more
canonical ?

       HDMI_MPEG_UNKNOWN_FRAME = 0x00,
       HDMI_MPEG_I_FRAME = 0x01,
       HDMI_MPEG_B_FRAME = 0x02,
       HDMI_MPEG_P_FRAME = 0x03,



>> +     return "Reserved";
>
> There really can't be another value here, so I think this should either
> return NULL or even go as far as let it crash. There should be no way
> for the invalid value to make it this far.
>

Ok.

>> +static int hdmi_mpeg_infoframe_unpack(struct hdmi_mpeg_infoframe *frame,
>> +                                  void *buffer)
>> +{
>> +     u8 *ptr = buffer;
>> +
>> +     if (ptr[0] != HDMI_INFOFRAME_TYPE_MPEG ||
>> +         ptr[1] != 1 ||
>> +         ptr[2] != HDMI_MPEG_INFOFRAME_SIZE) {
>> +             return -EINVAL;
>> +     }
>
> There's no need for the braces here.
>
>> @@ -314,6 +336,7 @@ union hdmi_vendor_any_infoframe {
>>   * @spd: spd infoframe
>>   * @vendor: union of all vendor infoframes
>>   * @audio: audio infoframe
>> + * @mpeg: mpeg infoframe
>
> s/mpeg/MPEG/
>

Ok.

> Thierry

I will send v2 after receiving your feedback again.

Best regards,
Enric

^ permalink raw reply

* Re: [PATCH v4 04/24] hwmon: pwm-fan: use pwm_get_xxx() helpers where appropriate
From: Boris Brezillon @ 2015-11-16 16:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5649FD5B.3000208@roeck-us.net>

On Mon, 16 Nov 2015 07:59:23 -0800
Guenter Roeck <linux@roeck-us.net> wrote:

> On 11/16/2015 12:56 AM, Boris Brezillon wrote:
> > Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> > Doing that will ease adaptation of the PWM framework to support atomic
> > update.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Patch generated with the following coccinelle script:
> >
> > --->8---
> > virtual patch
> >
> > @@
> > struct pwm_device *p;
> > expression e;
> > @@
> > (
> > -(p)->polarity = e;
> > +pwm_set_polarity((p), e);
> > |
> > -(p)->polarity
> > +pwm_get_polarity((p))
> 
> s/((p))/(p)/
> 
> > |
> > -(p)->period = e;
> > +pwm_set_period((p), e);
> > |
> > -(p)->period
> > +pwm_get_period((p))
> 
> s/((p))/(p)/
> 
> > |
> > -(p)->duty_cycle = e;
> > +pwm_set_duty_cycle((p), e);
> 
> The (p) seems unnecessary here.

I don't get this one. You mean I should drop one the parenthesis around
p, right?




-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v4 04/24] hwmon: pwm-fan: use pwm_get_xxx() helpers where appropriate
From: Guenter Roeck @ 2015-11-16 17:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20151116175322.48dba6f9@bbrezillon>

On 11/16/2015 08:53 AM, Boris Brezillon wrote:
> On Mon, 16 Nov 2015 07:59:23 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
>> On 11/16/2015 12:56 AM, Boris Brezillon wrote:
>>> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
>>> Doing that will ease adaptation of the PWM framework to support atomic
>>> update.
>>>
>>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>>> ---
>>> Patch generated with the following coccinelle script:
>>>
>>> --->8---
>>> virtual patch
>>>
>>> @@
>>> struct pwm_device *p;
>>> expression e;
>>> @@
>>> (
>>> -(p)->polarity = e;
>>> +pwm_set_polarity((p), e);
>>> |
>>> -(p)->polarity
>>> +pwm_get_polarity((p))
>>
>> s/((p))/(p)/
>>
>>> |
>>> -(p)->period = e;
>>> +pwm_set_period((p), e);
>>> |
>>> -(p)->period
>>> +pwm_get_period((p))
>>
>> s/((p))/(p)/
>>
>>> |
>>> -(p)->duty_cycle = e;
>>> +pwm_set_duty_cycle((p), e);
>>
>> The (p) seems unnecessary here.
>
> I don't get this one. You mean I should drop one the parenthesis around
> p, right?
>

Same as above - s/(p)/p/. It should never be necessary to write
	pwm_set_duty_cycle((p), e)
since
	pwm_set_duty_cycle(p, e)
should be the same.

On the other side, I did not see this expression used in any of the patches,
though maybe I missed it.

Thanks,
Guenter


^ permalink raw reply

* Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate
From: Joachim Eastwood @ 2015-11-16 17:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1447664207-24370-3-git-send-email-boris.brezillon@free-electrons.com>

Hi Boris

(Adding Ariel for pwm-lpc18xx-sct)

On 16 November 2015 at 09:56, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> Doing that will ease adaptation of the PWM framework to support atomic
> update.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Patch generated with the following coccinelle script:
>
> --->8---
> virtual patch
>
> @@
> struct pwm_device *p;
> expression e;
> @@
> (
> -(p)->polarity = e;
> +pwm_set_polarity((p), e);
> |
> -(p)->polarity
> +pwm_get_polarity((p))
> |
> -(p)->period = e;
> +pwm_set_period((p), e);
> |
> -(p)->period
> +pwm_get_period((p))
> |
> -(p)->duty_cycle = e;
> +pwm_set_duty_cycle((p), e);
> |
> -(p)->duty_cycle
> +pwm_get_duty_cycle((p))
> )
> --->8---
> ---
>  drivers/pwm/pwm-crc.c         | 2 +-
>  drivers/pwm/pwm-lpc18xx-sct.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
> index 7101c70..2f88543 100644
> --- a/drivers/pwm/pwm-crc.c
> +++ b/drivers/pwm/pwm-crc.c
> @@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
>                 return -EINVAL;
>         }
>
> -       if (pwm->period != period_ns) {
> +       if (pwm_get_period((pwm)) != period_ns) {
>                 int clk_div;
>
>                 /* changing the clk divisor, need to disable fisrt */
> diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> index 9163085..091fa13 100644
> --- a/drivers/pwm/pwm-lpc18xx-sct.c
> +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> @@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>                            LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event),
>                            LPC18XX_PWM_EVSTATEMSK_ALL);
>
> -       if (pwm->polarity = PWM_POLARITY_NORMAL) {
> +       if (pwm_get_polarity((pwm)) = PWM_POLARITY_NORMAL) {

What is the deal with the double parentheses?

Think I saw that in some of the other patches as well.


regards,
Joachim Eastwood

^ permalink raw reply

* Re: [PATCH v4 02/24] pwm: use pwm_get_xxx() helpers where appropriate
From: Boris Brezillon @ 2015-11-16 18:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGhQ9VwEuwazF5Kdw=9R60EovVoiwen6fXiVjL+=EddXPYVGAg@mail.gmail.com>

Hi Joachim,

On Mon, 16 Nov 2015 18:46:44 +0100
Joachim  Eastwood <manabian@gmail.com> wrote:

> Hi Boris
> 
> (Adding Ariel for pwm-lpc18xx-sct)
> 
> On 16 November 2015 at 09:56, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> > Doing that will ease adaptation of the PWM framework to support atomic
> > update.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Patch generated with the following coccinelle script:
> >
> > --->8---
> > virtual patch
> >
> > @@
> > struct pwm_device *p;
> > expression e;
> > @@
> > (
> > -(p)->polarity = e;
> > +pwm_set_polarity((p), e);
> > |
> > -(p)->polarity
> > +pwm_get_polarity((p))
> > |
> > -(p)->period = e;
> > +pwm_set_period((p), e);
> > |
> > -(p)->period
> > +pwm_get_period((p))
> > |
> > -(p)->duty_cycle = e;
> > +pwm_set_duty_cycle((p), e);
> > |
> > -(p)->duty_cycle
> > +pwm_get_duty_cycle((p))
> > )
> > --->8---
> > ---
> >  drivers/pwm/pwm-crc.c         | 2 +-
> >  drivers/pwm/pwm-lpc18xx-sct.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-crc.c b/drivers/pwm/pwm-crc.c
> > index 7101c70..2f88543 100644
> > --- a/drivers/pwm/pwm-crc.c
> > +++ b/drivers/pwm/pwm-crc.c
> > @@ -75,7 +75,7 @@ static int crc_pwm_config(struct pwm_chip *c, struct pwm_device *pwm,
> >                 return -EINVAL;
> >         }
> >
> > -       if (pwm->period != period_ns) {
> > +       if (pwm_get_period((pwm)) != period_ns) {
> >                 int clk_div;
> >
> >                 /* changing the clk divisor, need to disable fisrt */
> > diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
> > index 9163085..091fa13 100644
> > --- a/drivers/pwm/pwm-lpc18xx-sct.c
> > +++ b/drivers/pwm/pwm-lpc18xx-sct.c
> > @@ -249,7 +249,7 @@ static int lpc18xx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >                            LPC18XX_PWM_EVSTATEMSK(lpc18xx_data->duty_event),
> >                            LPC18XX_PWM_EVSTATEMSK_ALL);
> >
> > -       if (pwm->polarity = PWM_POLARITY_NORMAL) {
> > +       if (pwm_get_polarity((pwm)) = PWM_POLARITY_NORMAL) {
> 
> What is the deal with the double parentheses?
> 
> Think I saw that in some of the other patches as well.

It comes from a typo in my coccinelle script. I already fixed it and
regenerated the faulty patches, so please ignore this aspect while
reviewing (this will be addressed in the next version).

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate
From: Mark Brown @ 2015-11-16 18:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20151116132359.39d84279@bbrezillon>

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Mon, Nov 16, 2015 at 01:23:59PM +0100, Boris Brezillon wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:

> > > -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> > > +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);

> > It's not clear to me that we're not looking for the current period here
> > or in the other use.  Won't configuring based on a period other than the
> > one that has been set give the wrong answer?

> Hm, maybe that's naming problem. What I call the 'default' period here
> is actually the period configured in your board file (using a PWM lookup
> table) or your DT. This value represent the period requested by the PWM
> user not a default value specified by the PWM chip driver.

> The reason we're not using the 'current' period value is because it may
> have been set by the bootloader, and may be inappropriate for our use
> case (ie. the period may be to small to represent the different
> voltages).

> ITOH, we're using the current period value when calculating the current
> voltage, because we want to get the correct voltage value, and the PWM
> device may still use the configuration set by the bootloader (not the
> default one specified in your board or DT files).

> I hope this clarifies the differences between the current and default
> period, and why we should use the default value here.

To be honest I'm still a bit confused here.  When do we actually apply
the default setting and why do we keep on having to constantly override
it rather than doing this once at boot?  It feels wrong to be using it
every time we set anything.  I'd expect it to be something we only need
to do at probe time or which would automatically be handled by the PWM
framework (but that'd have issues changing the state and potentially
breaking things if done in an uncoordiated fashion).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH v4 09/24] regulator: pwm: use pwm_get/set_default_xxx() helpers where appropriate
From: Boris Brezillon @ 2015-11-16 19:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20151116184238.GJ31303@sirena.org.uk>

On Mon, 16 Nov 2015 18:42:38 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Nov 16, 2015 at 01:23:59PM +0100, Boris Brezillon wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> > > On Mon, Nov 16, 2015 at 09:56:32AM +0100, Boris Brezillon wrote:
> 
> > > > -	pwm_reg_period = pwm_get_period(drvdata->pwm);
> > > > +	pwm_reg_period = pwm_get_default_period(drvdata->pwm);
> 
> > > It's not clear to me that we're not looking for the current period here
> > > or in the other use.  Won't configuring based on a period other than the
> > > one that has been set give the wrong answer?
> 
> > Hm, maybe that's naming problem. What I call the 'default' period here
> > is actually the period configured in your board file (using a PWM lookup
> > table) or your DT. This value represent the period requested by the PWM
> > user not a default value specified by the PWM chip driver.
> 
> > The reason we're not using the 'current' period value is because it may
> > have been set by the bootloader, and may be inappropriate for our use
> > case (ie. the period may be to small to represent the different
> > voltages).
> 
> > ITOH, we're using the current period value when calculating the current
> > voltage, because we want to get the correct voltage value, and the PWM
> > device may still use the configuration set by the bootloader (not the
> > default one specified in your board or DT files).
> 
> > I hope this clarifies the differences between the current and default
> > period, and why we should use the default value here.
> 
> To be honest I'm still a bit confused here.  When do we actually apply
> the default setting and why do we keep on having to constantly override
> it rather than doing this once at boot?

That's why I said the 'default' name may be inappropriate. The
default values are actually never directly applied by the PWM framework.
It's the default value for a specific PWM user, so it can be applied by
the PWM user when he wants. It's more here as a reference, nothing
forces the PWM user to use this specific value.

> It feels wrong to be using it
> every time we set anything.  I'd expect it to be something we only need
> to do at probe time or which would automatically be handled by the PWM
> framework (but that'd have issues changing the state and potentially
> breaking things if done in an uncoordiated fashion).

The whole point of this series is to smoothly take over the bootloader
config. This is why we are keeping the PWM untouched until someone
really wants to change the regulator output. We should be able to apply
the 'default' PWM period when probing the device, but this means first
extracting the current voltage from the PWM state and then applying a
new dutycycle and the default period in a single operation. Not sure
it's worth the trouble.

Doing it in the PWM framework is not really possible, because the PWM
lookup table and DT definitions are only defining the 'default' period
value not the 'default' dutycycle, and applying that automatically when
requesting the PWM means generating a glitch on the PWM signal
(dutycycle will be set to 0 until the user changes it using
pwm_config() or pwm_apply_state()) which is exactly what we're trying to
solve here.

Also, note that you have to pass the period anyway when configuring the
PWM, so passing the default one or the current one should be pretty
much the same in term of performances (unless the PWM driver is able
to optimize its setting if the period does not change).

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH v4 1/2] video: fbdev: pxafb: loosen the platform data bond
From: Robert Jarzmik @ 2015-11-16 21:36 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, Robert Jarzmik

In order to prepare the transition to a mixed platform data and
device-tree initialization, remove all the platform data references all
over the driver.

Copy the platform data into the internal structure of the pxafb, and
only use this afterward.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/video/fbdev/pxafb.c | 54 ++++++++++++++++++++++++++++-----------------
 drivers/video/fbdev/pxafb.h |  2 ++
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 94813af97f09..ed4b1a5dc306 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -457,7 +457,7 @@ static int pxafb_adjust_timing(struct pxafb_info *fbi,
 static int pxafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 	int err;
 
 	if (inf->fixed_modes) {
@@ -1230,7 +1230,7 @@ static unsigned int __smart_timing(unsigned time_ns, unsigned long lcd_clk)
 static void setup_smart_timing(struct pxafb_info *fbi,
 				struct fb_var_screeninfo *var)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 	struct pxafb_mode_info *mode = &inf->modes[0];
 	unsigned long lclk = clk_get_rate(fbi->clk);
 	unsigned t1, t2, t3, t4;
@@ -1258,14 +1258,13 @@ static void setup_smart_timing(struct pxafb_info *fbi,
 static int pxafb_smart_thread(void *arg)
 {
 	struct pxafb_info *fbi = arg;
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 
 	if (!inf->smart_update) {
 		pr_err("%s: not properly initialized, thread terminated\n",
 				__func__);
 		return -EINVAL;
 	}
-	inf = dev_get_platdata(fbi->dev);
 
 	pr_debug("%s(): task starting\n", __func__);
 
@@ -1788,11 +1787,11 @@ decode_mode:
 		fbi->video_mem_size = video_mem_size;
 }
 
-static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
+static struct pxafb_info *pxafb_init_fbinfo(struct device *dev,
+					    struct pxafb_mach_info *inf)
 {
 	struct pxafb_info *fbi;
 	void *addr;
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
 
 	/* Alloc the pxafb_info and pseudo_palette in one step */
 	fbi = kmalloc(sizeof(struct pxafb_info) + sizeof(u32) * 16, GFP_KERNEL);
@@ -1801,6 +1800,7 @@ static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
 
 	memset(fbi, 0, sizeof(struct pxafb_info));
 	fbi->dev = dev;
+	fbi->inf = inf;
 
 	fbi->clk = clk_get(dev, NULL);
 	if (IS_ERR(fbi->clk)) {
@@ -1852,10 +1852,9 @@ static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
 }
 
 #ifdef CONFIG_FB_PXA_PARAMETERS
-static int parse_opt_mode(struct device *dev, const char *this_opt)
+static int parse_opt_mode(struct device *dev, const char *this_opt,
+			  struct pxafb_mach_info *inf)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
-
 	const char *name = this_opt+5;
 	unsigned int namelen = strlen(name);
 	int res_specified = 0, bpp_specified = 0;
@@ -1911,9 +1910,9 @@ done:
 	return 0;
 }
 
-static int parse_opt(struct device *dev, char *this_opt)
+static int parse_opt(struct device *dev, char *this_opt,
+		     struct pxafb_mach_info *inf)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
 	struct pxafb_mode_info *mode = &inf->modes[0];
 	char s[64];
 
@@ -1922,7 +1921,7 @@ static int parse_opt(struct device *dev, char *this_opt)
 	if (!strncmp(this_opt, "vmem:", 5)) {
 		video_mem_size = memparse(this_opt + 5, NULL);
 	} else if (!strncmp(this_opt, "mode:", 5)) {
-		return parse_opt_mode(dev, this_opt);
+		return parse_opt_mode(dev, this_opt, inf);
 	} else if (!strncmp(this_opt, "pixclock:", 9)) {
 		mode->pixclock = simple_strtoul(this_opt+9, NULL, 0);
 		sprintf(s, "pixclock: %ld\n", mode->pixclock);
@@ -2011,7 +2010,8 @@ static int parse_opt(struct device *dev, char *this_opt)
 	return 0;
 }
 
-static int pxafb_parse_options(struct device *dev, char *options)
+static int pxafb_parse_options(struct device *dev, char *options,
+			       struct pxafb_mach_info *inf)
 {
 	char *this_opt;
 	int ret;
@@ -2023,7 +2023,7 @@ static int pxafb_parse_options(struct device *dev, char *options)
 
 	/* could be made table driven or similar?... */
 	while ((this_opt = strsep(&options, ",")) != NULL) {
-		ret = parse_opt(dev, this_opt);
+		ret = parse_opt(dev, this_opt, inf);
 		if (ret)
 			return ret;
 	}
@@ -2095,19 +2095,33 @@ static void pxafb_check_options(struct device *dev, struct pxafb_mach_info *inf)
 static int pxafb_probe(struct platform_device *dev)
 {
 	struct pxafb_info *fbi;
-	struct pxafb_mach_info *inf;
+	struct pxafb_mach_info *inf, *pdata;
 	struct resource *r;
-	int irq, ret;
+	int i, irq, ret;
 
 	dev_dbg(&dev->dev, "pxafb_probe\n");
 
-	inf = dev_get_platdata(&dev->dev);
 	ret = -ENOMEM;
-	fbi = NULL;
+	pdata = dev_get_platdata(&dev->dev);
+	inf = devm_kmalloc(&dev->dev, sizeof(*inf), GFP_KERNEL);
 	if (!inf)
 		goto failed;
+	if (pdata) {
+		*inf = *pdata;
+		inf->modes +			devm_kmalloc_array(&dev->dev, pdata->num_modes,
+					   sizeof(inf->modes[0]), GFP_KERNEL);
+		if (!inf->modes)
+			goto failed;
+		for (i = 0; i < inf->num_modes; i++)
+			inf->modes[i] = pdata->modes[i];
+	}
+
+	fbi = NULL;
+	if (!pdata)
+		goto failed;
 
-	ret = pxafb_parse_options(&dev->dev, g_options);
+	ret = pxafb_parse_options(&dev->dev, g_options, inf);
 	if (ret < 0)
 		goto failed;
 
@@ -2125,7 +2139,7 @@ static int pxafb_probe(struct platform_device *dev)
 		goto failed;
 	}
 
-	fbi = pxafb_init_fbinfo(&dev->dev);
+	fbi = pxafb_init_fbinfo(&dev->dev, inf);
 	if (!fbi) {
 		/* only reason for pxafb_init_fbinfo to fail is kmalloc */
 		dev_err(&dev->dev, "Failed to initialize framebuffer device\n");
diff --git a/drivers/video/fbdev/pxafb.h b/drivers/video/fbdev/pxafb.h
index 26ba9fa3f737..5dc414e26fc8 100644
--- a/drivers/video/fbdev/pxafb.h
+++ b/drivers/video/fbdev/pxafb.h
@@ -167,6 +167,8 @@ struct pxafb_info {
 
 	void (*lcd_power)(int, struct fb_var_screeninfo *);
 	void (*backlight_power)(int);
+
+	struct pxafb_mach_info	*inf;
 };
 
 #define TO_INF(ptr,member) container_of(ptr,struct pxafb_info,member)
-- 
2.1.4


^ permalink raw reply related

* [PATCH v4 2/2] video: fbdev: pxafb: initial devicetree conversion
From: Robert Jarzmik @ 2015-11-16 21:36 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, Robert Jarzmik
In-Reply-To: <1447709811-5997-1-git-send-email-robert.jarzmik@free.fr>

This patch brings a first support of pxa framebuffer devices to a
devicetree pxa platform, as was before platform data.

There are restrictions with this port, the biggest one being the lack of
support of smart panels. Moreover the conversion doesn't provide a way
to declare multiple framebuffer configurations with different bits per
pixel, only the LCD hardware bus width is used.

The patch was tested on both pxa25x, pxa27x and pxa3xx platform (namely
lubbock, mainstone and zylonite).

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
Since v1: Philipp's review: of_graph usage
Since v3: of_device_id sentinel, and all compatible ids added
---
 drivers/video/fbdev/Kconfig |   2 +
 drivers/video/fbdev/pxafb.c | 170 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 169 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 8b1d371b5404..1a24ca5a0624 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1878,6 +1878,8 @@ config FB_PXA
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
+	select VIDEOMODE_HELPERS if OF
+	select FB_MODE_HELPERS if OF
 	---help---
 	  Frame buffer driver for the built-in LCD controller in the Intel
 	  PXA2x0 processor.
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index ed4b1a5dc306..18ad4773d1f8 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -55,6 +55,9 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/console.h>
+#include <linux/of_graph.h>
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
 
 #include <mach/hardware.h>
 #include <asm/io.h>
@@ -2092,6 +2095,151 @@ static void pxafb_check_options(struct device *dev, struct pxafb_mach_info *inf)
 #define pxafb_check_options(...)	do {} while (0)
 #endif
 
+#if defined(CONFIG_OF)
+static const char * const lcd_types[] = {
+	"unknown", "mono-stn", "mono-dstn", "color-stn", "color-dstn",
+	"color-tft", "smart-panel", NULL
+};
+
+static int of_get_pxafb_display(struct device *dev, struct device_node *disp,
+				struct pxafb_mach_info *info, u32 bus_width)
+{
+	struct display_timings *timings;
+	struct videomode vm;
+	int i, ret = -EINVAL;
+	const char *s;
+
+	ret = of_property_read_string(disp, "lcd-type", &s);
+	if (ret)
+		s = "color-tft";
+
+	for (i = 0; lcd_types[i]; i++)
+		if (!strcmp(s, lcd_types[i]))
+			break;
+	if (!i || !lcd_types[i]) {
+		dev_err(dev, "lcd-type %s is unknown\n", s);
+		return -EINVAL;
+	}
+	info->lcd_conn |= LCD_CONN_TYPE(i);
+	info->lcd_conn |= LCD_CONN_WIDTH(bus_width);
+
+	timings = of_get_display_timings(disp);
+	if (!timings)
+		goto out;
+
+	ret = -ENOMEM;
+	info->modes = kmalloc_array(timings->num_timings,
+				    sizeof(info->modes[0]), GFP_KERNEL);
+	if (!info->modes)
+		goto out;
+	info->num_modes = timings->num_timings;
+
+	for (i = 0; i < timings->num_timings; i++) {
+		ret = videomode_from_timings(timings, &vm, i);
+		if (ret) {
+			dev_err(dev, "videomode_from_timings %d failed: %d\n",
+				i, ret);
+			goto out;
+		}
+		if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+			info->lcd_conn |= LCD_PCLK_EDGE_RISE;
+		if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+			info->lcd_conn |= LCD_PCLK_EDGE_FALL;
+		if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+			info->lcd_conn |= LCD_BIAS_ACTIVE_HIGH;
+		if (vm.flags & DISPLAY_FLAGS_DE_LOW)
+			info->lcd_conn |= LCD_BIAS_ACTIVE_LOW;
+		if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
+			info->modes[i].sync |= FB_SYNC_HOR_HIGH_ACT;
+		if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
+			info->modes[i].sync |= FB_SYNC_VERT_HIGH_ACT;
+
+		info->modes[i].pixclock = 1000000000UL / (vm.pixelclock / 1000);
+		info->modes[i].xres = vm.hactive;
+		info->modes[i].yres = vm.vactive;
+		info->modes[i].hsync_len = vm.hsync_len;
+		info->modes[i].left_margin = vm.hback_porch;
+		info->modes[i].right_margin = vm.hfront_porch;
+		info->modes[i].vsync_len = vm.vsync_len;
+		info->modes[i].upper_margin = vm.vback_porch;
+		info->modes[i].lower_margin = vm.vfront_porch;
+	}
+	ret = 0;
+
+out:
+	display_timings_release(timings);
+	return ret;
+}
+
+static int of_get_pxafb_mode_info(struct device *dev,
+				  struct pxafb_mach_info *info)
+{
+	struct device_node *display, *np;
+	u32 bus_width, depth = 0;
+	int ret, i;
+
+	of_property_read_u32(dev->of_node, "depth", &depth);
+	np = of_graph_get_next_endpoint(dev->of_node, NULL);
+	if (!np) {
+		dev_err(dev, "could not find endpoint\n");
+		return -EINVAL;
+	}
+	ret = of_property_read_u32(np, "bus-width", &bus_width);
+	if (ret) {
+		dev_err(dev, "no bus-width specified: %d\n", ret);
+		return ret;
+	}
+
+	display = of_graph_get_remote_port_parent(np);
+	of_node_put(np);
+	if (!display) {
+		dev_err(dev, "no display defined\n");
+		return -EINVAL;
+	}
+
+	ret = of_get_pxafb_display(dev, display, info, bus_width);
+	of_node_put(display);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < info->num_modes; i++) {
+		info->modes[i].depth = depth;
+		info->modes[i].bpp = bus_width;
+	}
+
+	return 0;
+}
+
+static struct pxafb_mach_info *of_pxafb_of_mach_info(struct device *dev)
+{
+	int ret;
+	struct pxafb_mach_info *info;
+
+	if (!dev->of_node)
+		return NULL;
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+	ret = of_get_pxafb_mode_info(dev, info);
+	if (ret) {
+		kfree(info->modes);
+		return ERR_PTR(ret);
+	}
+
+	/*
+	 * On purpose, neither lccrX registers nor video memory size can be
+	 * specified through device-tree, they are considered more a debug hack
+	 * available through command line.
+	 */
+	return info;
+}
+#else
+static struct pxafb_mach_info *of_pxafb_of_mach_info(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int pxafb_probe(struct platform_device *dev)
 {
 	struct pxafb_info *fbi;
@@ -2104,8 +2252,7 @@ static int pxafb_probe(struct platform_device *dev)
 	ret = -ENOMEM;
 	pdata = dev_get_platdata(&dev->dev);
 	inf = devm_kmalloc(&dev->dev, sizeof(*inf), GFP_KERNEL);
-	if (!inf)
-		goto failed;
+
 	if (pdata) {
 		*inf = *pdata;
 		inf->modes @@ -2117,8 +2264,9 @@ static int pxafb_probe(struct platform_device *dev)
 			inf->modes[i] = pdata->modes[i];
 	}
 
-	fbi = NULL;
 	if (!pdata)
+		inf = of_pxafb_of_mach_info(&dev->dev);
+	if (IS_ERR_OR_NULL(inf))
 		goto failed;
 
 	ret = pxafb_parse_options(&dev->dev, g_options, inf);
@@ -2313,11 +2461,27 @@ static int pxafb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static const struct of_device_id pxafb_of_dev_id[] = {
+	{
+		.compatible = "marvell,pxa270-lcdc",
+	}, {
+	{
+		.compatible = "marvell,pxa300-lcdc",
+	}, {
+	{
+		.compatible = "marvell,pxa2xx-lcdc",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, pxafb_of_dev_id);
+
 static struct platform_driver pxafb_driver = {
 	.probe		= pxafb_probe,
 	.remove 	= pxafb_remove,
 	.driver		= {
 		.name	= "pxa2xx-fb",
+		.of_match_table = pxafb_of_dev_id,
 #ifdef CONFIG_PM
 		.pm	= &pxafb_pm_ops,
 #endif
-- 
2.1.4


^ permalink raw reply related

* Re: [PATCH v4 2/2] video: fbdev: pxafb: initial devicetree conversion
From: kbuild test robot @ 2015-11-17  0:01 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1447709811-5997-2-git-send-email-robert.jarzmik@free.fr>

[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]

Hi Robert,

[auto build test ERROR on v4.4-rc1]
[also build test ERROR on next-20151116]

url:    https://github.com/0day-ci/linux/commits/Robert-Jarzmik/video-fbdev-pxafb-loosen-the-platform-data-bond/20151117-053946
config: arm-spitz_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

>> drivers/video/fbdev/pxafb.c:2469:3: error: field name not in record or union initializer
      .compatible = "marvell,pxa300-lcdc",
      ^
   drivers/video/fbdev/pxafb.c:2469:3: error: (near initialization for 'pxafb_of_dev_id[1].name')
>> drivers/video/fbdev/pxafb.c:2471:2: warning: braces around scalar initializer
     {
     ^
   drivers/video/fbdev/pxafb.c:2471:2: warning: (near initialization for 'pxafb_of_dev_id[1].type[0]')
   drivers/video/fbdev/pxafb.c:2472:3: error: field name not in record or union initializer
      .compatible = "marvell,pxa2xx-lcdc",
      ^
   drivers/video/fbdev/pxafb.c:2472:3: error: (near initialization for 'pxafb_of_dev_id[1].type[0]')
>> drivers/video/fbdev/pxafb.c:2472:3: warning: initialization makes integer from pointer without a cast
   drivers/video/fbdev/pxafb.c:2472:3: warning: (near initialization for 'pxafb_of_dev_id[1].type[0]')
>> drivers/video/fbdev/pxafb.c:2472:3: error: initializer element is not computable at load time
   drivers/video/fbdev/pxafb.c:2472:3: error: (near initialization for 'pxafb_of_dev_id[1].type[0]')
   drivers/video/fbdev/pxafb.c:2473:2: warning: braces around scalar initializer
     }, {
     ^
   drivers/video/fbdev/pxafb.c:2473:2: warning: (near initialization for 'pxafb_of_dev_id[1].type[1]')
>> drivers/video/fbdev/pxafb.c:2476:2: error: expected '}' before ';' token
    };
     ^
   drivers/video/fbdev/pxafb.c:2243:12: warning: 'pxafb_probe' defined but not used [-Wunused-function]
    static int pxafb_probe(struct platform_device *dev)
               ^
   drivers/video/fbdev/pxafb.c:2425:12: warning: 'pxafb_remove' defined but not used [-Wunused-function]
    static int pxafb_remove(struct platform_device *dev)
               ^

vim +2469 drivers/video/fbdev/pxafb.c

  2463	
  2464	static const struct of_device_id pxafb_of_dev_id[] = {
  2465		{
  2466			.compatible = "marvell,pxa270-lcdc",
  2467		}, {
  2468		{
> 2469			.compatible = "marvell,pxa300-lcdc",
  2470		}, {
> 2471		{
> 2472			.compatible = "marvell,pxa2xx-lcdc",
  2473		}, {
  2474			/* sentinel */
  2475		}
> 2476	};
  2477	MODULE_DEVICE_TABLE(of, pxafb_of_dev_id);
  2478	
  2479	static struct platform_driver pxafb_driver = {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 18416 bytes --]

^ permalink raw reply

* [PATCH] video: fbdev: rivafb: unlock chip before probiding EDID
From: Dmitry Eremin-Solenikov @ 2015-11-17  1:38 UTC (permalink / raw)
  To: linux-fbdev

At least NV3 requires for chip to be unlocked before it is possible to
access I2C registers. Without it, it is not possible to read EDID.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/video/fbdev/riva/fbdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index f1ad274..2ef26ad 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -1765,6 +1765,7 @@ static int riva_get_EDID_i2c(struct fb_info *info)
 	int i;
 
 	NVTRACE_ENTER();
+	par->riva.LockUnlock(&par->riva, 0);
 	riva_create_i2c_busses(par);
 	for (i = 0; i < 3; i++) {
 		if (!par->chan[i].par)
-- 
2.6.2


^ permalink raw reply related

* [PATCH v5 0/2] simplefb: Add regulator handling support
From: Chen-Yu Tsai @ 2015-11-17  4:31 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Hans de Goede
  Cc: Mark Brown, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi everyone,

This is v5 of the simplefb regulator support series. This series adds
regulator claiming and enabling support for simplefb.

Hans, I dropped your Reviewed-by tag from patch 2 since v4.

Changes since v5:
  - Rebased onto v4.4-rc1
  - Dropped dts patches (merged)

Changes since v4:
  - Fixed inverted logic when testing the property name.
  - Fixed regulator supply name string copy length off by 1.
  - Added real world user, MSI Primo 81 dts patches.

Changes since v3:
  - Dropped extra "if" which is always true, leftover from v1.
  - Updated commit message of patch 1

Sometimes the simplefb display output path consits of external conversion
chips and/or LCD drivers and backlights. These devices normally have
GPIOs to turn them on and/or bring them out of reset, and regulators
supplying power to them.

While the kernel does not touch unclaimed GPIOs, the regulator core
happily disables unused regulators. Thus we need simplefb to claim
and enable the regulators used throughout the display pipeline.

The binding supports any named regulator supplies under its device
node. The driver will look through its properties, and claim any
regulators by matching "*-supply", as Mark suggested.

I've not done a generic helper in the regulator core yet, instead doing
the regulator property handling in the simplefb code for now.


Patch 1 adds the regulator properties to the DT binding.

Patch 2 adds code to the simplefb driver to claim and enable regulators.

Regards
ChenYu


Chen-Yu Tsai (2):
  dt-bindings: simplefb: Support regulator supply properties
  simplefb: Claim and enable regulators

 .../bindings/display/simple-framebuffer.txt        |  13 ++-
 drivers/video/fbdev/simplefb.c                     | 120 ++++++++++++++++++++-
 2 files changed, 128 insertions(+), 5 deletions(-)

-- 
2.6.2


^ permalink raw reply

* [PATCH v5 1/2] dt-bindings: simplefb: Support regulator supply properties
From: Chen-Yu Tsai @ 2015-11-17  4:31 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Hans de Goede
  Cc: Mark Brown, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1447734663-19189-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>

The physical display tied to the framebuffer may have regulators
providing power to it, such as power for LCDs or interface conversion
chips.

The number of regulators in use may vary, but the regulator supply
binding can not be a list. Instead just support any named regulator
supply properties under the device node. These should be properly
named to match the device schematics / design. The driver should
take care to go through them all.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
 .../devicetree/bindings/display/simple-framebuffer.txt      | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer.txt b/Documentation/devicetree/bindings/display/simple-framebuffer.txt
index 4474ef6e0b95..8c9e9f515c87 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer.txt
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer.txt
@@ -47,10 +47,14 @@ Required properties:
   - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r).
 
 Optional properties:
-- clocks : List of clocks used by the framebuffer. Clocks listed here
-           are expected to already be configured correctly. The OS must
-           ensure these clocks are not modified or disabled while the
-           simple framebuffer remains active.
+- clocks : List of clocks used by the framebuffer.
+- *-supply : Any number of regulators used by the framebuffer. These should
+	     be named according to the names in the device's design.
+
+  The above resources are expected to already be configured correctly.
+  The OS must ensure they are not modified or disabled while the simple
+  framebuffer remains active.
+
 - display : phandle pointing to the primary display hardware node
 
 Example:
@@ -68,6 +72,7 @@ chosen {
 		stride = <(1600 * 2)>;
 		format = "r5g6b5";
 		clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
+		lcd-supply = <&reg_dc1sw>;
 		display = <&lcdc0>;
 	};
 	stdout-path = "display0";
-- 
2.6.2


^ permalink raw reply related

* [PATCH v5 2/2] simplefb: Claim and enable regulators
From: Chen-Yu Tsai @ 2015-11-17  4:31 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Hans de Goede
  Cc: Mark Brown, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1447734663-19189-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>

This claims and enables regulators listed in the simple framebuffer dt
node. This is needed so that regulators powering the display pipeline
and external hardware, described in the device node and known by the
kernel code, will remain properly enabled.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Mark Brown <broonie@kernel.org>
---
 drivers/video/fbdev/simplefb.c | 120 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index 52c5c7e63b52..48ccf6db62a2 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -28,7 +28,10 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/parser.h>
+#include <linux/regulator/consumer.h>
 
 static struct fb_fix_screeninfo simplefb_fix = {
 	.id		= "simple",
@@ -174,6 +177,10 @@ struct simplefb_par {
 	int clk_count;
 	struct clk **clks;
 #endif
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+	u32 regulator_count;
+	struct regulator **regulators;
+#endif
 };
 
 #if defined CONFIG_OF && defined CONFIG_COMMON_CLK
@@ -269,6 +276,110 @@ static int simplefb_clocks_init(struct simplefb_par *par,
 static void simplefb_clocks_destroy(struct simplefb_par *par) { }
 #endif
 
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+
+#define SUPPLY_SUFFIX "-supply"
+
+/*
+ * Regulator handling code.
+ *
+ * Here we handle the num-supplies and vin*-supply properties of our
+ * "simple-framebuffer" dt node. This is necessary so that we can make sure
+ * that any regulators needed by the display hardware that the bootloader
+ * set up for us (and for which it provided a simplefb dt node), stay up,
+ * for the life of the simplefb driver.
+ *
+ * When the driver unloads, we cleanly disable, and then release the
+ * regulators.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up simplefb, and the regulator definitions in the device tree. Chances are
+ * that there are no adverse effects, and if there are, a clean teardown of
+ * the fb probe will not help us much either. So just complain and carry on,
+ * and hope that the user actually gets a working fb at the end of things.
+ */
+static int simplefb_regulators_init(struct simplefb_par *par,
+	struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct property *prop;
+	struct regulator *regulator;
+	const char *p;
+	int count = 0, i = 0, ret;
+
+	if (dev_get_platdata(&pdev->dev) || !np)
+		return 0;
+
+	/* Count the number of regulator supplies */
+	for_each_property_of_node(np, prop) {
+		p = strstr(prop->name, SUPPLY_SUFFIX);
+		if (p && p != prop->name)
+			count++;
+	}
+
+	if (!count)
+		return 0;
+
+	par->regulators = devm_kcalloc(&pdev->dev, count,
+				       sizeof(struct regulator *), GFP_KERNEL);
+	if (!par->regulators)
+		return -ENOMEM;
+
+	/* Get all the regulators */
+	for_each_property_of_node(np, prop) {
+		char name[32]; /* 32 is max size of property name */
+
+		p = strstr(prop->name, SUPPLY_SUFFIX);
+		if (!p || p = prop->name)
+			continue;
+
+		strlcpy(name, prop->name,
+			strlen(prop->name) - strlen(SUPPLY_SUFFIX) + 1);
+		regulator = devm_regulator_get_optional(&pdev->dev, name);
+		if (IS_ERR(regulator)) {
+			if (PTR_ERR(regulator) = -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			dev_err(&pdev->dev, "regulator %s not found: %ld\n",
+				name, PTR_ERR(regulator));
+			continue;
+		}
+		par->regulators[i++] = regulator;
+	}
+	par->regulator_count = i;
+
+	/* Enable all the regulators */
+	for (i = 0; i < par->regulator_count; i++) {
+		ret = regulator_enable(par->regulators[i]);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed to enable regulator %d: %d\n",
+				i, ret);
+			devm_regulator_put(par->regulators[i]);
+			par->regulators[i] = NULL;
+		}
+	}
+
+	return 0;
+}
+
+static void simplefb_regulators_destroy(struct simplefb_par *par)
+{
+	int i;
+
+	if (!par->regulators)
+		return;
+
+	for (i = 0; i < par->regulator_count; i++)
+		if (par->regulators[i])
+			regulator_disable(par->regulators[i]);
+}
+#else
+static int simplefb_regulators_init(struct simplefb_par *par,
+	struct platform_device *pdev) { return 0; }
+static void simplefb_regulators_destroy(struct simplefb_par *par) { }
+#endif
+
 static int simplefb_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -340,6 +451,10 @@ static int simplefb_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_unmap;
 
+	ret = simplefb_regulators_init(par, pdev);
+	if (ret < 0)
+		goto error_clocks;
+
 	dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
 			     info->fix.smem_start, info->fix.smem_len,
 			     info->screen_base);
@@ -351,13 +466,15 @@ static int simplefb_probe(struct platform_device *pdev)
 	ret = register_framebuffer(info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret);
-		goto error_clocks;
+		goto error_regulators;
 	}
 
 	dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
 
 	return 0;
 
+error_regulators:
+	simplefb_regulators_destroy(par);
 error_clocks:
 	simplefb_clocks_destroy(par);
 error_unmap:
@@ -373,6 +490,7 @@ static int simplefb_remove(struct platform_device *pdev)
 	struct simplefb_par *par = info->par;
 
 	unregister_framebuffer(info);
+	simplefb_regulators_destroy(par);
 	simplefb_clocks_destroy(par);
 	framebuffer_release(info);
 
-- 
2.6.2


^ permalink raw reply related

* [PATCH] [media] move media platform data to linux/platform_data/media
From: Mauro Carvalho Chehab @ 2015-11-17  9:15 UTC (permalink / raw)
  To: linux-fbdev
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
	Mauro Carvalho Chehab, Acked-by: Arnd Bergmann, Shawn Guo,
	Sascha Hauer, Russell King, Daniel Mack, Haojian Zhuang,
	Robert Jarzmik, Daniel Ribeiro, Stefan Schmidt, Harald Welte,
	Tomas Cech, Sergey Lapin, Vinod Koul, Dan Williams, Philipp Zabel,
	Guennadi Liakhovetski, Ulf Hansson, Mark Brown,
	Greg Kroah-Hartman

Now that media has its own subdirectory inside platform_data,
let's move the headers that are already there to such subdir.

After moving those files, the references were adjusted using this
script:

    MAIN_DIR="linux/platform_data/"
    PREV_DIR="linux/platform_data/"
    DIRS="media/"

    echo "Checking affected files" >&2
    for i in $DIRS; do
	for j in $(find include/$MAIN_DIR/$i -type f -name '*.h'); do
		 n=`basename $j`
		git grep -l $n
	done
    done|sort|uniq >files && (
	echo "Handling files..." >&2;
	echo "for i in \$(cat files|grep -v Documentation); do cat \$i | \\";
	(
		cd include/$MAIN_DIR;
		for j in $DIRS; do
			for i in $(ls $j); do
				echo "perl -ne 's,(include [\\\"\\<])$PREV_DIR($i)([\\\"\\>]),\1$MAIN_DIR$j\2\3,; print \$_' |\\";
			done;
		done;
		echo "cat > a && mv a \$i; done";
	);
	echo "Handling documentation..." >&2;
	echo "for i in MAINTAINERS \$(cat files); do cat \$i | \\";
	(
		cd include/$MAIN_DIR;
		for j in $DIRS; do
			for i in $(ls $j); do
				echo "  perl -ne 's,include/$PREV_DIR($i)\b,include/$MAIN_DIR$j\1,; print \$_' |\\";
			done;
		done;
		echo "cat > a && mv a \$i; done"
	);
    ) >script && . ./script

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 arch/arm/mach-imx/devices/devices-common.h            | 4 ++--
 arch/arm/mach-pxa/devices.c                           | 2 +-
 arch/arm/mach-pxa/em-x270.c                           | 2 +-
 arch/arm/mach-pxa/ezx.c                               | 2 +-
 arch/arm/mach-pxa/mioa701.c                           | 2 +-
 arch/arm/mach-pxa/palmtreo.c                          | 2 +-
 arch/arm/mach-pxa/palmz72.c                           | 2 +-
 arch/arm/mach-pxa/pcm990-baseboard.c                  | 2 +-
 drivers/dma/imx-dma.c                                 | 2 +-
 drivers/dma/imx-sdma.c                                | 2 +-
 drivers/media/platform/coda/coda-common.c             | 2 +-
 drivers/media/platform/soc_camera/mx2_camera.c        | 2 +-
 drivers/media/platform/soc_camera/mx3_camera.c        | 4 ++--
 drivers/media/platform/soc_camera/pxa_camera.c        | 2 +-
 drivers/media/platform/soc_camera/rcar_vin.c          | 2 +-
 drivers/mmc/host/mxcmmc.c                             | 2 +-
 drivers/spi/spi-imx.c                                 | 2 +-
 drivers/tty/serial/imx.c                              | 2 +-
 drivers/video/fbdev/mx3fb.c                           | 2 +-
 include/linux/platform_data/{ => media}/camera-mx2.h  | 0
 include/linux/platform_data/{ => media}/camera-mx3.h  | 0
 include/linux/platform_data/{ => media}/camera-pxa.h  | 0
 include/linux/platform_data/{ => media}/camera-rcar.h | 0
 include/linux/platform_data/{ => media}/coda.h        | 0
 include/linux/platform_data/{ => media}/dma-imx.h     | 0
 sound/soc/fsl/fsl_asrc.c                              | 2 +-
 sound/soc/fsl/fsl_asrc_dma.c                          | 2 +-
 sound/soc/fsl/imx-pcm.h                               | 2 +-
 sound/soc/fsl/imx-ssi.h                               | 2 +-
 29 files changed, 25 insertions(+), 25 deletions(-)
 rename include/linux/platform_data/{ => media}/camera-mx2.h (100%)
 rename include/linux/platform_data/{ => media}/camera-mx3.h (100%)
 rename include/linux/platform_data/{ => media}/camera-pxa.h (100%)
 rename include/linux/platform_data/{ => media}/camera-rcar.h (100%)
 rename include/linux/platform_data/{ => media}/coda.h (100%)
 rename include/linux/platform_data/{ => media}/dma-imx.h (100%)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 67f7fb13050d..09cebd8cef2b 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -177,7 +177,7 @@ struct platform_device *__init imx_add_imx_uart_1irq(
 		const struct imxuart_platform_data *pdata);
 
 #include <linux/platform_data/video-mx3fb.h>
-#include <linux/platform_data/camera-mx3.h>
+#include <linux/platform_data/media/camera-mx3.h>
 struct imx_ipu_core_data {
 	resource_size_t iobase;
 	resource_size_t synirq;
@@ -192,7 +192,7 @@ struct platform_device *__init imx_add_mx3_sdc_fb(
 		const struct imx_ipu_core_data *data,
 		struct mx3fb_platform_data *pdata);
 
-#include <linux/platform_data/camera-mx2.h>
+#include <linux/platform_data/media/camera-mx2.h>
 struct imx_mx2_camera_data {
 	const char *devid;
 	resource_size_t iobasecsi;
diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
index 2a6e0ae2b920..d1211a40f400 100644
--- a/arch/arm/mach-pxa/devices.c
+++ b/arch/arm/mach-pxa/devices.c
@@ -14,7 +14,7 @@
 #include <mach/irqs.h>
 #include <linux/platform_data/usb-ohci-pxa27x.h>
 #include <linux/platform_data/keypad-pxa27x.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 #include <mach/audio.h>
 #include <mach/hardware.h>
 #include <linux/platform_data/mmp_dma.h>
diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index 9d7072b04045..8b1f89e096c6 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -46,7 +46,7 @@
 #include <linux/platform_data/usb-ohci-pxa27x.h>
 #include <linux/platform_data/mmc-pxamci.h>
 #include <linux/platform_data/keypad-pxa27x.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 
 #include "generic.h"
 #include "devices.h"
diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
index 9a9c15bfcd34..12af6e2d597c 100644
--- a/arch/arm/mach-pxa/ezx.c
+++ b/arch/arm/mach-pxa/ezx.c
@@ -34,7 +34,7 @@
 #include <linux/platform_data/usb-ohci-pxa27x.h>
 #include <mach/hardware.h>
 #include <linux/platform_data/keypad-pxa27x.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 
 #include "devices.h"
 #include "generic.h"
diff --git a/arch/arm/mach-pxa/mioa701.c b/arch/arm/mach-pxa/mioa701.c
index 3b52b1aa0659..ccfd2b63c6a4 100644
--- a/arch/arm/mach-pxa/mioa701.c
+++ b/arch/arm/mach-pxa/mioa701.c
@@ -54,7 +54,7 @@
 #include <linux/platform_data/mmc-pxamci.h>
 #include <mach/udc.h>
 #include <mach/pxa27x-udc.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 #include <mach/audio.h>
 #include <mach/smemc.h>
 #include <media/soc_camera.h>
diff --git a/arch/arm/mach-pxa/palmtreo.c b/arch/arm/mach-pxa/palmtreo.c
index d8b937c870de..2dc56062fb7e 100644
--- a/arch/arm/mach-pxa/palmtreo.c
+++ b/arch/arm/mach-pxa/palmtreo.c
@@ -43,7 +43,7 @@
 #include <linux/platform_data/usb-ohci-pxa27x.h>
 #include <mach/pxa2xx-regs.h>
 #include <linux/platform_data/asoc-palm27x.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 #include <mach/palm27x.h>
 
 #include <sound/pxa2xx-lib.h>
diff --git a/arch/arm/mach-pxa/palmz72.c b/arch/arm/mach-pxa/palmz72.c
index 1a35ddf218da..e3df17a7e8d4 100644
--- a/arch/arm/mach-pxa/palmz72.c
+++ b/arch/arm/mach-pxa/palmz72.c
@@ -49,7 +49,7 @@
 #include <mach/palm27x.h>
 
 #include <mach/pm.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 
 #include <media/soc_camera.h>
 
diff --git a/arch/arm/mach-pxa/pcm990-baseboard.c b/arch/arm/mach-pxa/pcm990-baseboard.c
index e3b58cb84c06..8459239a093c 100644
--- a/arch/arm/mach-pxa/pcm990-baseboard.c
+++ b/arch/arm/mach-pxa/pcm990-baseboard.c
@@ -30,7 +30,7 @@
 #include <media/i2c/mt9v022.h>
 #include <media/soc_camera.h>
 
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 #include <asm/mach/map.h>
 #include <mach/pxa27x.h>
 #include <mach/audio.h>
diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index 48d85f8b95fe..8d1856369af4 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -31,7 +31,7 @@
 #include <linux/of_dma.h>
 
 #include <asm/irq.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 
 #include "dmaengine.h"
 #define IMXDMA_MAX_CHAN_DESCRIPTORS	16
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 7058d58ba588..fde0537b55ed 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -41,7 +41,7 @@
 
 #include <asm/irq.h>
 #include <linux/platform_data/dma-imx-sdma.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 15516a6e3a39..f821627d015b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -28,7 +28,7 @@
 #include <linux/slab.h>
 #include <linux/videodev2.h>
 #include <linux/of.h>
-#include <linux/platform_data/coda.h>
+#include <linux/platform_data/media/coda.h>
 #include <linux/reset.h>
 
 #include <media/v4l2-ctrls.h>
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c b/drivers/media/platform/soc_camera/mx2_camera.c
index 55437ec3a3e2..276beaefca7c 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -39,7 +39,7 @@
 
 #include <linux/videodev2.h>
 
-#include <linux/platform_data/camera-mx2.h>
+#include <linux/platform_data/media/camera-mx2.h>
 
 #include <asm/dma.h>
 
diff --git a/drivers/media/platform/soc_camera/mx3_camera.c b/drivers/media/platform/soc_camera/mx3_camera.c
index 3e67b9517a5a..a7d449dd84f9 100644
--- a/drivers/media/platform/soc_camera/mx3_camera.c
+++ b/drivers/media/platform/soc_camera/mx3_camera.c
@@ -25,8 +25,8 @@
 #include <media/soc_camera.h>
 #include <media/drv-intf/soc_mediabus.h>
 
-#include <linux/platform_data/camera-mx3.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/camera-mx3.h>
+#include <linux/platform_data/media/dma-imx.h>
 
 #define MX3_CAM_DRV_NAME "mx3-camera"
 
diff --git a/drivers/media/platform/soc_camera/pxa_camera.c b/drivers/media/platform/soc_camera/pxa_camera.c
index 34762a82ebd2..415f3bda60bf 100644
--- a/drivers/media/platform/soc_camera/pxa_camera.c
+++ b/drivers/media/platform/soc_camera/pxa_camera.c
@@ -39,7 +39,7 @@
 #include <linux/videodev2.h>
 
 #include <mach/dma.h>
-#include <linux/platform_data/camera-pxa.h>
+#include <linux/platform_data/media/camera-pxa.h>
 
 #define PXA_CAM_VERSION "0.0.6"
 #define PXA_CAM_DRV_NAME "pxa27x-camera"
diff --git a/drivers/media/platform/soc_camera/rcar_vin.c b/drivers/media/platform/soc_camera/rcar_vin.c
index 32aa64c3fc7e..defee08f073c 100644
--- a/drivers/media/platform/soc_camera/rcar_vin.c
+++ b/drivers/media/platform/soc_camera/rcar_vin.c
@@ -21,7 +21,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/platform_data/camera-rcar.h>
+#include <linux/platform_data/media/camera-rcar.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index d110f9e98c4b..9b8aba09a56d 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -44,7 +44,7 @@
 #include <asm/irq.h>
 #include <linux/platform_data/mmc-mxcmmc.h>
 
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 
 #define DRIVER_NAME "mxc-mmc"
 #define MXCMCI_TIMEOUT_MS 10000
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 0e5723ab47f0..0a84ab47c8c9 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -39,7 +39,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <linux/platform_data/spi-imx.h>
 
 #define DRIVER_NAME "spi_imx"
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 016e4be05cec..cfc622fd02ab 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -42,7 +42,7 @@
 
 #include <asm/irq.h>
 #include <linux/platform_data/serial-imx.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 
 /* Register definitions */
 #define URXD0 0x0  /* Receiver Register */
diff --git a/drivers/video/fbdev/mx3fb.c b/drivers/video/fbdev/mx3fb.c
index 7947634ee6b0..702b6ff23370 100644
--- a/drivers/video/fbdev/mx3fb.c
+++ b/drivers/video/fbdev/mx3fb.c
@@ -29,7 +29,7 @@
 #include <linux/dma/ipu-dma.h>
 #include <linux/backlight.h>
 
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <linux/platform_data/video-mx3fb.h>
 
 #include <asm/io.h>
diff --git a/include/linux/platform_data/camera-mx2.h b/include/linux/platform_data/media/camera-mx2.h
similarity index 100%
rename from include/linux/platform_data/camera-mx2.h
rename to include/linux/platform_data/media/camera-mx2.h
diff --git a/include/linux/platform_data/camera-mx3.h b/include/linux/platform_data/media/camera-mx3.h
similarity index 100%
rename from include/linux/platform_data/camera-mx3.h
rename to include/linux/platform_data/media/camera-mx3.h
diff --git a/include/linux/platform_data/camera-pxa.h b/include/linux/platform_data/media/camera-pxa.h
similarity index 100%
rename from include/linux/platform_data/camera-pxa.h
rename to include/linux/platform_data/media/camera-pxa.h
diff --git a/include/linux/platform_data/camera-rcar.h b/include/linux/platform_data/media/camera-rcar.h
similarity index 100%
rename from include/linux/platform_data/camera-rcar.h
rename to include/linux/platform_data/media/camera-rcar.h
diff --git a/include/linux/platform_data/coda.h b/include/linux/platform_data/media/coda.h
similarity index 100%
rename from include/linux/platform_data/coda.h
rename to include/linux/platform_data/media/coda.h
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/media/dma-imx.h
similarity index 100%
rename from include/linux/platform_data/dma-imx.h
rename to include/linux/platform_data/media/dma-imx.h
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 9f087d4f73ed..0ca75915ff12 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -15,7 +15,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <linux/pm_runtime.h>
 #include <sound/dmaengine_pcm.h>
 #include <sound/pcm_params.h>
diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
index ffc000bc1f15..96e3fb0db535 100644
--- a/sound/soc/fsl/fsl_asrc_dma.c
+++ b/sound/soc/fsl/fsl_asrc_dma.c
@@ -12,7 +12,7 @@
 
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <sound/dmaengine_pcm.h>
 #include <sound/pcm_params.h>
 
diff --git a/sound/soc/fsl/imx-pcm.h b/sound/soc/fsl/imx-pcm.h
index 133c4470acad..367e1a768d42 100644
--- a/sound/soc/fsl/imx-pcm.h
+++ b/sound/soc/fsl/imx-pcm.h
@@ -13,7 +13,7 @@
 #ifndef _IMX_PCM_H
 #define _IMX_PCM_H
 
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 
 /*
  * Do not change this as the FIQ handler depends on this size
diff --git a/sound/soc/fsl/imx-ssi.h b/sound/soc/fsl/imx-ssi.h
index be6562365b6a..7a7d55b54cf5 100644
--- a/sound/soc/fsl/imx-ssi.h
+++ b/sound/soc/fsl/imx-ssi.h
@@ -186,7 +186,7 @@
 #define DRV_NAME "imx-ssi"
 
 #include <linux/dmaengine.h>
-#include <linux/platform_data/dma-imx.h>
+#include <linux/platform_data/media/dma-imx.h>
 #include <sound/dmaengine_pcm.h>
 #include "imx-pcm.h"
 
-- 
2.5.0


^ permalink raw reply related

* Re: [PATCH] [media] move media platform data to linux/platform_data/media
From: Arnd Bergmann @ 2015-11-17  9:23 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mauro Carvalho Chehab, Ulf Hansson, Xiubo Li,
	Benjamin Herrenschmidt, Sergey Lapin, Timur Tabi, Jaroslav Kysela,
	Tomas Cech, Robert Jarzmik, Markus Elfring, Harald Welte,
	openezx-devel, Russell King, Vinod Koul, Takashi Iwai,
	Tomi Valkeinen, linux-serial, Jiri Slaby,
	Jean-Christophe Plagniol-Villard, Daniel Ribeiro,
	Linux Media Mailing List, Fabio Estevam
In-Reply-To: <4d99e49726942dc4d6a6ee1debf6665b2b47908b.1447751746.git.mchehab@osg.samsung.com>

On Tuesday 17 November 2015 07:15:59 Mauro Carvalho Chehab wrote:
> Now that media has its own subdirectory inside platform_data,
> let's move the headers that are already there to such subdir.
> 
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [PATCH] [media] move media platform data to linux/platform_data/media
From: Mark Brown @ 2015-11-17 10:37 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
	Acked-by: Arnd Bergmann, Shawn Guo, Sascha Hauer, Russell King,
	Daniel Mack, Haojian Zhuang, Robert Jarzmik, Daniel Ribeiro,
	Stefan Schmidt, Harald Welte, Tomas Cech, Sergey Lapin,
	Vinod Koul, Dan Williams, Philipp Zabel, Guennadi Liakhovetski,
	Ulf Hansson, Greg Kroah-Hartman, Jiri Slaby,
	Jean-Christophe Plagniol-Villard <plagnioj@
In-Reply-To: <4d99e49726942dc4d6a6ee1debf6665b2b47908b.1447751746.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Tue, Nov 17, 2015 at 07:15:59AM -0200, Mauro Carvalho Chehab wrote:
> Now that media has its own subdirectory inside platform_data,
> let's move the headers that are already there to such subdir.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH] [media] move media platform data to linux/platform_data/media
From: Anton Bondarenko @ 2015-11-17 10:43 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: Ulf Hansson, Xiubo Li, Benjamin Herrenschmidt, Sergey Lapin,
	Timur Tabi, Jaroslav Kysela, Tomas Cech, Shawn Guo,
	Robert Jarzmik, Markus Elfring, Harald Welte, openezx-devel,
	Russell King, Vinod Koul, Tomi Valkeinen, linux-serial,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Daniel Ribeiro,
	Linux Media Mailing List, Stefan Schmidt, Sascha Hauer,
	alsa-devel
In-Reply-To: <7319142.Cp8MurgLWk@wuerfel>


On 17.11.2015 10:23, Arnd Bergmann wrote:
> On Tuesday 17 November 2015 07:15:59 Mauro Carvalho Chehab wrote:
>> Now that media has its own subdirectory inside platform_data,
>> let's move the headers that are already there to such subdir.
>>
>>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Why does dma-imx.h also moved? It's the generic file, not a media specific.

BR, Anton

^ permalink raw reply

* Re: [PATCH] [media] move media platform data to linux/platform_data/media
From: Arnd Bergmann @ 2015-11-17 10:58 UTC (permalink / raw)
  To: Anton Bondarenko
  Cc: Ulf Hansson, Xiubo Li, Benjamin Herrenschmidt, Sergey Lapin,
	Timur Tabi, Jaroslav Kysela, Tomas Cech, Robert Jarzmik,
	Markus Elfring, Harald Welte, openezx-devel, Russell King,
	Vinod Koul, Takashi Iwai, Tomi Valkeinen, linux-serial,
	Jiri Slaby, Jean-Christophe Plagniol-Villard, Daniel Ribeiro,
	Linux Media Mailing List, Fabio Estevam, Philipp Zabel,
	alsa-devel
In-Reply-To: <564B04CF.1090507@gmail.com>

On Tuesday 17 November 2015 11:43:27 Anton Bondarenko wrote:
> On 17.11.2015 10:23, Arnd Bergmann wrote:
> > On Tuesday 17 November 2015 07:15:59 Mauro Carvalho Chehab wrote:
> >> Now that media has its own subdirectory inside platform_data,
> >> let's move the headers that are already there to such subdir.
> >>
> >>
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> Why does dma-imx.h also moved? It's the generic file, not a media specific.

You are right, I missed that when I looked at the patch. That part
of the patch needs to be taken out.

	Arnd

^ permalink raw reply

* Re: [PATCH] [media] hdmi: added functions for MPEG InfoFrames
From: Thierry Reding @ 2015-11-17 12:55 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: linux-fbdev, Mauro Carvalho Chehab, linux-kernel, dri-devel,
	Tomi Valkeinen, Hans Verkuil, Martin Bugge,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <CAFqH_514hjjgPHbP=u1nQ2FPoQe2zLuK5L2R6JcLXjOQdpqZ9g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

On Mon, Nov 16, 2015 at 05:28:24PM +0100, Enric Balletbo Serra wrote:
> Hello Thierry,
> 
> Many thanks for your comments.
> 
> 2015-11-16 12:50 GMT+01:00 Thierry Reding <treding@nvidia.com>:
> > On Sat, Nov 14, 2015 at 07:38:19PM +0100, Enric Balletbo i Serra wrote:
> >> The MPEG Source (MS) InfoFrame is in EIA/CEA-861B. It describes aspects of
> >> the compressed video stream that were used to produce the uncompressed
> >> video.
> >>
> >> The patch adds functions to work with MPEG InfoFrames.
> >>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >>  drivers/video/hdmi.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/hdmi.h |  24 ++++++++
> >>  2 files changed, 180 insertions(+)
> >
> > According to the CEA specification a source is expected to send this
> > type of infoframe once per video frame. I'm curious how you envision
> > this to be ensured. Would hardware provide a mechanism to store this
> > data and send the infoframe automatically? How would you ensure that
> > updates sent to the hardware match the upcoming frame?
> >
> 
> To be honest I'm not sure if I have the full picture. In the use case
> I'm trying there is a hardware mechanism to store the data and send
> the infoframe through a "Packet Send Control Register".

Okay, sounds like the hardware will automatically send out packets at
the right time. That still leaves open the issue of how to ensure this
is synchronized with userspace. Perhaps this could be done by attaching
a property to a framebuffer, so that we'd know what exact frame the meta
data is attached to and when to update the FIFOs for the infoframe.

Usually it's a good idea to send this type of patch as part of a larger
series precisely so that people can see how it is used. That should make
it easier to see if this is good enough or needs some more thought on
how to synchronize. Do you have any code that you could post that makes
use of this new infoframe?

> >> @@ -899,6 +978,41 @@ static void hdmi_audio_infoframe_log(const char *level,
> >>                       frame->downmix_inhibit ? "Yes" : "No");
> >>  }
> >>
> >> +static const char *hdmi_mpeg_picture_get_name(enum hdmi_mpeg_picture_type type)
> >> +{
> >> +     switch (type) {
> >> +     case HDMI_MPEG_PICTURE_TYPE_UNKNOWN:
> >> +             return "Unknown";
> >> +     case HDMI_MPEG_PICTURE_TYPE_I:
> >> +             return "Intra-coded picture";
> >> +     case HDMI_MPEG_PICTURE_TYPE_B:
> >> +             return "Bi-predictive picture";
> >> +     case HDMI_MPEG_PICTURE_TYPE_P:
> >> +             return "Predicted picture";
> >> +     }
> >
> > I'd have chosen the slightly more canonical "I-Frame", "P-Frame",
> > "B-Frame" here, but that's not a strong objection.
> >
> 
> I don't have any inconvenient to change, are the following names more
> canonical ?
> 
>        HDMI_MPEG_UNKNOWN_FRAME = 0x00,
>        HDMI_MPEG_I_FRAME = 0x01,
>        HDMI_MPEG_B_FRAME = 0x02,
>        HDMI_MPEG_P_FRAME = 0x03,

I wasn't very clear. What I meant was the names for the constants. At
least personally I know immediately what is meant when I see "I-Frame",
"P-Frame" or "B-Frame", whereas "Bi-predictive picture" needs more
thinking.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v4 05/24] misc: max77693-haptic: use pwm_get_xxx() helpers where appropriate
From: Dmitry Torokhov @ 2015-11-17 17:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1447664207-24370-6-git-send-email-boris.brezillon@free-electrons.com>

On Mon, Nov 16, 2015 at 09:56:28AM +0100, Boris Brezillon wrote:
> Use pwm_get_xxx() helpers instead of directly accessing the pwm->xxx field.
> Doing that will ease adaptation of the PWM framework to support atomic
> update.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

Could you please tag input-related bits with "Input: driver - ... "
instead of "misc: ... "?

Thanks!

-- 
Dmitry

^ permalink raw reply

* [PATCH v5 1/2] video: fbdev: pxafb: loosen the platform data bond
From: Robert Jarzmik @ 2015-11-17 20:32 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, Robert Jarzmik

In order to prepare the transition to a mixed platform data and
device-tree initialization, remove all the platform data references all
over the driver.

Copy the platform data into the internal structure of the pxafb, and
only use this afterward.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/video/fbdev/pxafb.c | 54 ++++++++++++++++++++++++++++-----------------
 drivers/video/fbdev/pxafb.h |  2 ++
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 94813af97f09..ed4b1a5dc306 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -457,7 +457,7 @@ static int pxafb_adjust_timing(struct pxafb_info *fbi,
 static int pxafb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 {
 	struct pxafb_info *fbi = container_of(info, struct pxafb_info, fb);
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 	int err;
 
 	if (inf->fixed_modes) {
@@ -1230,7 +1230,7 @@ static unsigned int __smart_timing(unsigned time_ns, unsigned long lcd_clk)
 static void setup_smart_timing(struct pxafb_info *fbi,
 				struct fb_var_screeninfo *var)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 	struct pxafb_mode_info *mode = &inf->modes[0];
 	unsigned long lclk = clk_get_rate(fbi->clk);
 	unsigned t1, t2, t3, t4;
@@ -1258,14 +1258,13 @@ static void setup_smart_timing(struct pxafb_info *fbi,
 static int pxafb_smart_thread(void *arg)
 {
 	struct pxafb_info *fbi = arg;
-	struct pxafb_mach_info *inf = dev_get_platdata(fbi->dev);
+	struct pxafb_mach_info *inf = fbi->inf;
 
 	if (!inf->smart_update) {
 		pr_err("%s: not properly initialized, thread terminated\n",
 				__func__);
 		return -EINVAL;
 	}
-	inf = dev_get_platdata(fbi->dev);
 
 	pr_debug("%s(): task starting\n", __func__);
 
@@ -1788,11 +1787,11 @@ decode_mode:
 		fbi->video_mem_size = video_mem_size;
 }
 
-static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
+static struct pxafb_info *pxafb_init_fbinfo(struct device *dev,
+					    struct pxafb_mach_info *inf)
 {
 	struct pxafb_info *fbi;
 	void *addr;
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
 
 	/* Alloc the pxafb_info and pseudo_palette in one step */
 	fbi = kmalloc(sizeof(struct pxafb_info) + sizeof(u32) * 16, GFP_KERNEL);
@@ -1801,6 +1800,7 @@ static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
 
 	memset(fbi, 0, sizeof(struct pxafb_info));
 	fbi->dev = dev;
+	fbi->inf = inf;
 
 	fbi->clk = clk_get(dev, NULL);
 	if (IS_ERR(fbi->clk)) {
@@ -1852,10 +1852,9 @@ static struct pxafb_info *pxafb_init_fbinfo(struct device *dev)
 }
 
 #ifdef CONFIG_FB_PXA_PARAMETERS
-static int parse_opt_mode(struct device *dev, const char *this_opt)
+static int parse_opt_mode(struct device *dev, const char *this_opt,
+			  struct pxafb_mach_info *inf)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
-
 	const char *name = this_opt+5;
 	unsigned int namelen = strlen(name);
 	int res_specified = 0, bpp_specified = 0;
@@ -1911,9 +1910,9 @@ done:
 	return 0;
 }
 
-static int parse_opt(struct device *dev, char *this_opt)
+static int parse_opt(struct device *dev, char *this_opt,
+		     struct pxafb_mach_info *inf)
 {
-	struct pxafb_mach_info *inf = dev_get_platdata(dev);
 	struct pxafb_mode_info *mode = &inf->modes[0];
 	char s[64];
 
@@ -1922,7 +1921,7 @@ static int parse_opt(struct device *dev, char *this_opt)
 	if (!strncmp(this_opt, "vmem:", 5)) {
 		video_mem_size = memparse(this_opt + 5, NULL);
 	} else if (!strncmp(this_opt, "mode:", 5)) {
-		return parse_opt_mode(dev, this_opt);
+		return parse_opt_mode(dev, this_opt, inf);
 	} else if (!strncmp(this_opt, "pixclock:", 9)) {
 		mode->pixclock = simple_strtoul(this_opt+9, NULL, 0);
 		sprintf(s, "pixclock: %ld\n", mode->pixclock);
@@ -2011,7 +2010,8 @@ static int parse_opt(struct device *dev, char *this_opt)
 	return 0;
 }
 
-static int pxafb_parse_options(struct device *dev, char *options)
+static int pxafb_parse_options(struct device *dev, char *options,
+			       struct pxafb_mach_info *inf)
 {
 	char *this_opt;
 	int ret;
@@ -2023,7 +2023,7 @@ static int pxafb_parse_options(struct device *dev, char *options)
 
 	/* could be made table driven or similar?... */
 	while ((this_opt = strsep(&options, ",")) != NULL) {
-		ret = parse_opt(dev, this_opt);
+		ret = parse_opt(dev, this_opt, inf);
 		if (ret)
 			return ret;
 	}
@@ -2095,19 +2095,33 @@ static void pxafb_check_options(struct device *dev, struct pxafb_mach_info *inf)
 static int pxafb_probe(struct platform_device *dev)
 {
 	struct pxafb_info *fbi;
-	struct pxafb_mach_info *inf;
+	struct pxafb_mach_info *inf, *pdata;
 	struct resource *r;
-	int irq, ret;
+	int i, irq, ret;
 
 	dev_dbg(&dev->dev, "pxafb_probe\n");
 
-	inf = dev_get_platdata(&dev->dev);
 	ret = -ENOMEM;
-	fbi = NULL;
+	pdata = dev_get_platdata(&dev->dev);
+	inf = devm_kmalloc(&dev->dev, sizeof(*inf), GFP_KERNEL);
 	if (!inf)
 		goto failed;
+	if (pdata) {
+		*inf = *pdata;
+		inf->modes +			devm_kmalloc_array(&dev->dev, pdata->num_modes,
+					   sizeof(inf->modes[0]), GFP_KERNEL);
+		if (!inf->modes)
+			goto failed;
+		for (i = 0; i < inf->num_modes; i++)
+			inf->modes[i] = pdata->modes[i];
+	}
+
+	fbi = NULL;
+	if (!pdata)
+		goto failed;
 
-	ret = pxafb_parse_options(&dev->dev, g_options);
+	ret = pxafb_parse_options(&dev->dev, g_options, inf);
 	if (ret < 0)
 		goto failed;
 
@@ -2125,7 +2139,7 @@ static int pxafb_probe(struct platform_device *dev)
 		goto failed;
 	}
 
-	fbi = pxafb_init_fbinfo(&dev->dev);
+	fbi = pxafb_init_fbinfo(&dev->dev, inf);
 	if (!fbi) {
 		/* only reason for pxafb_init_fbinfo to fail is kmalloc */
 		dev_err(&dev->dev, "Failed to initialize framebuffer device\n");
diff --git a/drivers/video/fbdev/pxafb.h b/drivers/video/fbdev/pxafb.h
index 26ba9fa3f737..5dc414e26fc8 100644
--- a/drivers/video/fbdev/pxafb.h
+++ b/drivers/video/fbdev/pxafb.h
@@ -167,6 +167,8 @@ struct pxafb_info {
 
 	void (*lcd_power)(int, struct fb_var_screeninfo *);
 	void (*backlight_power)(int);
+
+	struct pxafb_mach_info	*inf;
 };
 
 #define TO_INF(ptr,member) container_of(ptr,struct pxafb_info,member)
-- 
2.1.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox