Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 22/23] video: da8xx-fb: set upstream clock rate (if reqd)
From: Tomi Valkeinen @ 2013-06-26  9:51 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1372170171-9561-23-git-send-email-detheridge@ti.com>

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

On 25/06/13 17:22, Darren Etheridge wrote:
> From: Afzal Mohammed <afzal@ti.com>
> 
> LCDC IP has a clock divider to adjust pixel clock, this limits pixel
> clock range to fck/255 - fck/2(fck - rate of input clock to LCDC IP).
> In the case of AM335x, where this IP is present, default fck is not
> sufficient to provide normal pixel clock rates, hence rendering this
> driver unusable on AM335x.
> 
> If input clock too is configurable, allowable range of pixel clock
> would increase. Here initially it is checked whether with present fck,
> divider in IP could be configured to obtain required rate, if not,
> fck is adjusted. This makes it usable on AM335x.
> 
> Note:
> Another solution would be to model an inherited basic clock divider of
> CCF, an advantage would be a better possible resolution for pixel clk.
> And trying to instantiate a CCF clock would mean that to be consistent,
> 3 bits being turned on to enable clocks of LCDC IP would have to be
> modeled as gate clocks. Now that would bring in a total of 4 clocks,
> including necessity to create a new inherited divider clock, and that
> mean a branch of clock tree would be present in LCDC driver. This
> would add complexity to LCDC driver bringing in considerable amount
> of clock handling code, and this would not bring in much advantage
> for existing use cases other than providing a higher resolution of
> pixel clock. And existing use cases work without relying on clock
> modeling. Another fact is that out of the two platform's using this
> driver DaVinci is not yet converted to CCF. In future if higher
> resolution of pixel clock is required, and probably after DaVinci is
> CCF'ed, modeling clock nodes inside driver may be considered.
> 
> Signed-off-by: Afzal Mohammed <afzal@ti.com>
> Signed-off-by: Darren Etheridge <detheridge@ti.com>
> ---
>  drivers/video/da8xx-fb.c |   76 +++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 58 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 5455682..09dfa12 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -133,6 +133,9 @@
>  #define WSI_TIMEOUT	50
>  #define PALETTE_SIZE	256
>  
> +#define	CLK_MIN_DIV	2
> +#define	CLK_MAX_DIV	255
> +
>  static void __iomem *da8xx_fb_reg_base;
>  static struct resource *lcdc_regs;
>  static unsigned int lcd_revision;
> @@ -683,23 +686,21 @@ static void da8xx_fb_lcd_reset(void)
>  	}
>  }
>  
> -static inline unsigned da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
> -						 unsigned pixclock)
> -{
> -	return par->lcd_fck_rate / (PICOS2KHZ(pixclock) * 1000);
> -}
> -
> -static inline unsigned da8xx_fb_round_clk(struct da8xx_fb_par *par,
> -					  unsigned pixclock)
> +static int da8xx_fb_config_clk_divider(struct da8xx_fb_par *par,
> +					      unsigned div, unsigned rate)

I would suggest to use better names than 'div' and 'rate'. What div?
What rate?

>  {
> -	unsigned div;
> +	int ret;
>  
> -	div = da8xx_fb_calc_clk_divider(par, pixclock);
> -	return KHZ2PICOS(par->lcd_fck_rate / (1000 * div));
> -}
> +	if (par->lcd_fck_rate != rate) {
> +		ret = clk_set_rate(par->lcdc_clk, rate);
> +		if (IS_ERR_VALUE(ret)) {
> +			dev_err(par->dev,
> +				"unable to set clock rate at %u\n", rate);
> +			return ret;
> +		}
> +		par->lcd_fck_rate = clk_get_rate(par->lcdc_clk);
> +	}

I think it would be good to require that the caller has calculated the
rate properly, and used clk_round_rate. So instead of silently storing
the actual clock rate to lcd_fck_rate, either fail or give a warning if
the resulting clock rate is different than the requested one.

> -static inline void da8xx_fb_config_clk_divider(unsigned div)
> -{
>  	/* Configure the LCD clock divisor. */
>  	lcdc_write(LCD_CLK_DIVISOR(div) |
>  			(LCD_RASTER_MODE & 0x1), LCD_CTRL_REG);
> @@ -707,14 +708,49 @@ static inline void da8xx_fb_config_clk_divider(unsigned div)
>  	if (lcd_revision == LCD_VERSION_2)
>  		lcdc_write(LCD_V2_DMA_CLK_EN | LCD_V2_LIDD_CLK_EN |
>  				LCD_V2_CORE_CLK_EN, LCD_CLK_ENABLE_REG);
> +
> +	return 0;
> +}
> +
> +static unsigned int da8xx_fb_calc_clk_divider(struct da8xx_fb_par *par,
> +					      unsigned pixclock,
> +					      unsigned *rate)
> +{
> +	unsigned div;
> +
> +	pixclock = PICOS2KHZ(pixclock) * 1000;
> +
> +	*rate = par->lcd_fck_rate;
> +
> +	if (pixclock < (*rate / CLK_MAX_DIV)) {
> +		*rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MAX_DIV);
> +		div = CLK_MAX_DIV;
> +	} else if (pixclock > (*rate / CLK_MIN_DIV)) {
> +		*rate = clk_round_rate(par->lcdc_clk, pixclock * CLK_MIN_DIV);
> +		div = CLK_MIN_DIV;
> +	} else {
> +		div = *rate / pixclock;
> +	}
> +
> +	return div;
>  }

If I understand correctly, the function returns the LCDC clock divider
and the fclk clock rate, calculated for the given pixclock.

What happens if this function is called with a pixel clock of 1Hz or
1GHz (ie. something not achievable)?

And wouldn't it be better to always set the fclk clock rate? Even if the
pixclock is in the range achieved with a divider between CLK_MIN_DIV and
CLK_MAX_DIV, it can be quite far from the requested clock rate.
Adjusting the fclk rate could give a much better match.

> -static inline void da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,
> +static inline int da8xx_fb_calc_config_clk_divider(struct da8xx_fb_par *par,

Don't use inline. The compiler can decide if it's worth to inline a
function or not. I guess inline is fine for funcs like lcdc_read(), but
generally leave it for the compiler.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v4 0/7] xilinxfb changes
From: Michal Simek @ 2013-06-26 10:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jean-Christophe PLAGNIOL-VILLARD, Michal Simek, linux-kernel,
	Timur Tabi, devicetree-discuss, linux-fbdev, Tomi Valkeinen,
	Grant Likely, Rob Herring
In-Reply-To: <201306172307.19114.arnd@arndb.de>

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

On 06/17/2013 11:07 PM, Arnd Bergmann wrote:
> On Monday 17 June 2013, Michal Simek wrote:
>> On 06/17/2013 10:56 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 07:23 Mon 17 Jun     , Michal Simek wrote:
>>>> On 06/06/2013 06:23 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On 12:13 Mon 03 Jun     , Michal Simek wrote:
>>>>>> Hi,
>>>>>>
>>>>>
>>>>> Arnd can you take look on it again please
>>>>>
>>>>> I'll take a look on it next week
>>>>
>>>> Any update on this?
>>> look ok but I want the Ack from Arnd
> 
> Sorry for the delay, everything looks good to me.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Jean-Christophe: Will you apply this series?
Or should I take it through my microblaze tree?

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Re: [V2 1/7] video: mmp: rb swap setting update for new LCD driver
From: jett zhou @ 2013-06-26 10:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMLZHHTdgU2-BAsUn-QoUP5JL--pPVjsT4AW0Yv+bPL__EbHCw@mail.gmail.com>

2013/6/25 Daniel Drake <dsd@laptop.org>:
> On Mon, Jun 24, 2013 at 8:23 PM, jett zhou <jett.zhou@gmail.com> wrote:
>>> What if I'm working with a display that doesn't need or want RB
>>> swapping? Lets say I am working with format PIXFMT_RGB565, and running
>>> your patch. dmafetch_set_fmt() gets called, and fmt_to_reg() sets
>>> rbswap to 1.
>>> This means that dmafetch_set_fmt() writes a '1' into the appropriate
>>> RB-swapping bit in the LCD_PN_CTRL0 register, and this triggers the
>>> "DMA input" swapping that you mentioned. But I never asked for RB
>>> swapping...
>>
>> Yes, if you configure it as PIXFMT_RGB565, it will set rbswap in "DMA
>> input" part.
>> So, for your case, you need to use PIXFMT_BGR565 instead of PIXFMT_RGB565.
>
> So let me get this straight. I have a display that wants RGB565
> format, no RB swapping. I don't do anything special in link_config to
> affect any swapping.
>
> After your patch, I must request BGR565 format in order to get RGB565?
> That sounds backwards to me.

HI Daniel
     My above explanation may be not so precise and correct, actually
you can RGB565 directly and does not need BGR565, mmp display
controller handle it accordingly in driver.
     Take RGB565 and BGR565 as example:
          RGB565 indicates the source data in memory is that Red is
[15~11] , Green is [10~5], Blue is [4:0], Red is MSB, Blue is LSB, but
for the display dma input default setting(rbswap = 0), it only support
Blue is [15~11] , Green is [10~5], Red is [4:0], Red is LSB, Blue is
MSB, so for this format(RGB565), display controller need to set rbswap
= 1 and it can support the MSB/LSB correctly.
          In conclusion, no matter you use RGB565 or BGR565, mmp
display driver did the rbswap setiing based on the format and what is
MSB/LSB, so platform does not need handle it by link_config based on
this patch.
Thanks

>
>>> Your comment above suggests that this RB-swapping behaviour is
>>> something that is imposed by the output device. In which case, this
>>> should be a configuration parameter on the panel, not on the path
>>> structure.
>>>
>>>>            TTC_dkb does not support dsi, the link_config is no used anymore.
>>>
>>> Then you should fix up ttc_dkb before submitting this patch.
>>
>> After we add one new field for this output rbswap setting based on dsi
>> interface, it can be used by new stepping of mmp display controller,
>> ttc_dkb platform just leave and not touch it, it will be tranparent
>> for ttc_dkb, does not need to nothing for platform configuration for
>> ttc_dkb usage.
>> It means , ttc_dkb can only configure rbswap in "dma input" part, not
>> support rbswap in dsi interface part.
>> What do you think?
>
> The point I am trying to make is that your patch is changing behaviour
> for ttc_dkb, so you need to address that at the same time.
>
> Right now ttc_dkb does this:
>
> #define CFG_GRA_SWAPRB(x)      (x << 0) /* 1: rbswap enabled */
> .link_config = CFG_DUMBMODE(0x2) | CFG_GRA_SWAPRB(0x1),
>
> i.e. SWAPRB requested through bit 0 in link_config
>
> And this is obeyed by the existing code in fmt_to_reg:
>
>    u32 link_config = path_to_path_plat(overlay->path)->link_config;
>    rbswap = link_config & 0x1;
>
> Your patch removes the handling of link_config bit 0, without fixing
> up its only user (even if that user was incorrect). That is not good
> practice.
>
> Another question: why is this change needed?
> We can request rb swapping either in DMA input or in the output
> interface. I can understand the driver maybe supporting one option or
> the other. But after your patch, it seems like both are supported: RB
> swapping could be enabled either through choice of input format, or
> through configuration of output parameters.
For TTC dkb,  you are right, I need to fix the rbswap in the platform
part, I will add one fix patch for ttc_dkb based on new rbswap patch.
For output dsi interface, it has this feature to do rbswap again if it
needs specifc byte sequence of RGB byte of DSI panel.
eg. If display content is set RGB565 in memory and DMA input part set
rbswap in driver to support Red as MSB , Blue LSB, but dsi panel only
support Red as LSB, Blue as MSB, then it can use this feature.
If there is no this requirement of panel, this dsi output part is not needed.

Thanks
>




--

----------------------------------
Best Regards
Jett Zhou

^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Prabhakar Lad @ 2013-06-26 11:32 UTC (permalink / raw)
  To: tomi.valkeinen, Florian Tobias Schandinat
  Cc: LKML, Lad, Prabhakar, DLOS, LFBDEV
In-Reply-To: <1368688257-18705-3-git-send-email-prabhakar.csengg@gmail.com>

Hi Tomi/ Florian

On Thu, May 16, 2013 at 12:40 PM, Lad Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>
> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
> devm_request_irq() instead of request_irq().
>
> This ensures more consistent error values and simplifies error paths.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---

Can you pick up this patch and drop 1/2.

Regards,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH v2 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Kishon Vijay Abraham I @ 2013-06-26 11:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130625205452.GC9748@arwen.pp.htv.fi>

Hi,

On Wednesday 26 June 2013 02:24 AM, Felipe Balbi wrote:
> Hi,
>
> On Tue, Jun 25, 2013 at 07:44:52PM +0200, Sylwester Nawrocki wrote:
>>>> +struct exynos_video_phy {
>>>> +	spinlock_t slock;
>>>> +	struct phy *phys[NUM_PHYS];
>>>
>>> more than one phy ? This means you should instantiate driver multiple
>>> drivers. Each phy id should call probe again.
>>
>> Why ? This single PHY _provider_ can well handle multiple PHYs.
>> I don't see a good reason to further complicate this driver like
>> this. Please note that MIPI-CSIS 0 and MIPI DSIM 0 share MMIO
>> register, so does MIPI CSIS 1 and MIPI DSIM 1. There are only 2
>> registers for those 4 PHYs. I could have the involved object
>> multiplied, but it would have been just a waste of resources
>> with no difference to the PHY consumers.
>
> alright, I misunderstood your code then. When I looked over your id
> usage I missed the "/2" part and assumed that you would have separate
> EXYNOS_MIPI_PHY_CONTROL() register for each ;-)
>
> My bad, you can disregard the other comments.
>
>>>> +static int exynos_video_phy_probe(struct platform_device *pdev)
>>>> +{
>>>> +	struct exynos_video_phy *state;
>>>> +	struct device *dev = &pdev->dev;
>>>> +	struct resource *res;
>>>> +	struct phy_provider *phy_provider;
>>>> +	int i;
>>>> +
>>>> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
>>>> +	if (!state)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +
>>>> +	state->regs = devm_ioremap_resource(dev, res);
>>>> +	if (IS_ERR(state->regs))
>>>> +		return PTR_ERR(state->regs);
>>>> +
>>>> +	dev_set_drvdata(dev, state);
>>>
>>> you can use platform_set_drvdata(pdev, state);
>>
>> I had it in the previous version, but changed for symmetry with
>> dev_set_drvdata(). I guess those could be replaced with
>> phy_{get, set}_drvdata as you suggested.

right. currently I was setting dev_set_drvdata of phy (core) device
in phy-core.c and the corresponding dev_get_drvdata in phy provider driver
which is little confusing.
So I'll add phy_set_drvdata and phy_get_drvdata in phy.h (as suggested by
Felipe) to be used by phy provider drivers. So after creating the PHY, the
phy provider should use phy_set_drvdata and in phy_ops, it can use
phy_get_drvdata. (I'll remove the dev_set_drvdata in phy_create).

This also means _void *priv_ in phy_create is useless. So I'll be removing
_priv_ from phy_create.

Thanks
Kishon

>
> hmm, you do need to set the drvdata() to the phy object, but also to the
> pdev object (should you need it on a suspend/resume callback, for
> instance). Those are separate struct device instances.


^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Tomi Valkeinen @ 2013-06-26 11:39 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: DLOS, LFBDEV, Florian Tobias Schandinat, LKML, Etheridge, Darren
In-Reply-To: <1368688257-18705-3-git-send-email-prabhakar.csengg@gmail.com>

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

On 16/05/13 10:10, Lad Prabhakar wrote:
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> 
> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
> devm_request_irq() instead of request_irq().
> 
> This ensures more consistent error values and simplifies error paths.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---
>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>  1 files changed, 7 insertions(+), 32 deletions(-)

There's something similar in Darren's da8xx-fb series. Can you check if
there are differences?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v4 0/7] xilinxfb changes
From: Tomi Valkeinen @ 2013-06-26 11:41 UTC (permalink / raw)
  To: monstr
  Cc: Arnd Bergmann, Jean-Christophe PLAGNIOL-VILLARD, Michal Simek,
	linux-kernel, Timur Tabi, devicetree-discuss, linux-fbdev,
	Grant Likely, Rob Herring
In-Reply-To: <51CABC2A.50209@monstr.eu>

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

Hi,

On 26/06/13 13:02, Michal Simek wrote:
> On 06/17/2013 11:07 PM, Arnd Bergmann wrote:
>> On Monday 17 June 2013, Michal Simek wrote:
>>> On 06/17/2013 10:56 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> On 07:23 Mon 17 Jun     , Michal Simek wrote:
>>>>> On 06/06/2013 06:23 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>> On 12:13 Mon 03 Jun     , Michal Simek wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>
>>>>>> Arnd can you take look on it again please
>>>>>>
>>>>>> I'll take a look on it next week
>>>>>
>>>>> Any update on this?
>>>> look ok but I want the Ack from Arnd
>>
>> Sorry for the delay, everything looks good to me.
>>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> Jean-Christophe: Will you apply this series?
> Or should I take it through my microblaze tree?

I've added this to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH -next] video: mxsfb: remove redundant dev_err call in mxsfb_probe()
From: Tomi Valkeinen @ 2013-06-26 11:43 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <CAPgLHd80wMDBg8ADjtDvZC1=yUoigS7_btHMz1=r8WQTwpgSvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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

On 26/06/13 04:50, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> There is a error message within devm_ioremap_resource
> already, so remove the dev_err call to avoid redundant
> error message.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/video/mxsfb.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 21223d4..251bbec 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -899,7 +899,6 @@ static int mxsfb_probe(struct platform_device *pdev)
>  
>  	host->base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(host->base)) {
> -		dev_err(&pdev->dev, "ioremap failed\n");
>  		ret = PTR_ERR(host->base);
>  		goto fb_release;
>  	}
> 

I've added this to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH V2] video: remove unnecessary platform_set_drvdata()
From: Tomi Valkeinen @ 2013-06-26 11:45 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <000101ce7147$28bd62c0$7a382840$@samsung.com>

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

On 25/06/13 04:56, Jingoo Han wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure, since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
> (device-core: Ensure drvdata = NULL when no driver is bound).
> Thus, it is not needed to manually clear the device driver data to NULL.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> Cc: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

I've added this to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 1/1] video: imxfb: Make local symbols static
From: Tomi Valkeinen @ 2013-06-26 11:53 UTC (permalink / raw)
  To: linux-fbdev

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

On 10/05/13 14:55, Sachin Kamat wrote:
> These symbols are used only in this file.
> Make them static.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/video/imxfb.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I've added this and the following three other small cleanups to my
fbdev-3.11 branch:

video: smscufx: Use NULL instead of 0
video: udlfb: Use NULL instead of 0
video: udlfb: Make local symbol static

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Prabhakar Lad @ 2013-06-26 11:57 UTC (permalink / raw)
  To: Tomi Valkeinen, Etheridge, Darren
  Cc: DLOS, LFBDEV, Florian Tobias Schandinat, LKML
In-Reply-To: <51CAD305.30407@ti.com>

Hi Tomi,

On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 16/05/13 10:10, Lad Prabhakar wrote:
>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>
>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>> devm_request_irq() instead of request_irq().
>>
>> This ensures more consistent error values and simplifies error paths.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>> ---
>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>  1 files changed, 7 insertions(+), 32 deletions(-)
>
> There's something similar in Darren's da8xx-fb series. Can you check if
> there are differences?
>
I don't see similar changes in his patch series.

Darren while you are at it you can take this patch along you series.
let me know how you plan to.

Regards,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Tomi Valkeinen @ 2013-06-26 12:00 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: Etheridge, Darren, DLOS, LFBDEV, Florian Tobias Schandinat, LKML
In-Reply-To: <CA+V-a8uhK5-QJKifkiSUN6Nz0aBc2moraWQUJPCmZg9zaFX0eg@mail.gmail.com>

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

On 26/06/13 14:56, Prabhakar Lad wrote:
> Hi Tomi,
> 
> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>
>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>> devm_request_irq() instead of request_irq().
>>>
>>> This ensures more consistent error values and simplifies error paths.
>>>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>> ---
>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>
>> There's something similar in Darren's da8xx-fb series. Can you check if
>> there are differences?
>>
> I don't see similar changes in his patch series.

[PATCH 14/23] video: da8xx-fb: use devres

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Sylwester Nawrocki @ 2013-06-26 12:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51CACEBE.9000505@ti.com>

On 06/26/2013 01:21 PM, Kishon Vijay Abraham I wrote:
>>>>> +static int exynos_video_phy_probe(struct platform_device *pdev)
>>>>> >>>> +{
>>>>> >>>> +	struct exynos_video_phy *state;
>>>>> >>>> +	struct device *dev = &pdev->dev;
>>>>> >>>> +	struct resource *res;
>>>>> >>>> +	struct phy_provider *phy_provider;
>>>>> >>>> +	int i;
>>>>> >>>> +
>>>>> >>>> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
>>>>> >>>> +	if (!state)
>>>>> >>>> +		return -ENOMEM;
>>>>> >>>> +
>>>>> >>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> >>>> +
>>>>> >>>> +	state->regs = devm_ioremap_resource(dev, res);
>>>>> >>>> +	if (IS_ERR(state->regs))
>>>>> >>>> +		return PTR_ERR(state->regs);
>>>>> >>>> +
>>>>> >>>> +	dev_set_drvdata(dev, state);
>>>> >>>
>>>> >>> you can use platform_set_drvdata(pdev, state);
>>> >>
>>> >> I had it in the previous version, but changed for symmetry with
>>> >> dev_set_drvdata(). I guess those could be replaced with
>>> >> phy_{get, set}_drvdata as you suggested.
>
> right. currently I was setting dev_set_drvdata of phy (core) device
> in phy-core.c and the corresponding dev_get_drvdata in phy provider driver
> which is little confusing.
> So I'll add phy_set_drvdata and phy_get_drvdata in phy.h (as suggested by
> Felipe) to be used by phy provider drivers. So after creating the PHY, the
> phy provider should use phy_set_drvdata and in phy_ops, it can use
> phy_get_drvdata. (I'll remove the dev_set_drvdata in phy_create).
> 
> This also means _void *priv_ in phy_create is useless. So I'll be removing
> _priv_ from phy_create.

Yeah, sounds good. Then in the phy ops phy_get_drvdata(&phy->dev) would
be used and in a custom of_xlate dev_get_drvdata(dev) (assuming the phy
provider sets drvdata on its device beforehand).

I can't see any races from runtime PM with this approach.

Regards,
Sylwester

^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Tomi Valkeinen @ 2013-06-26 12:11 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: Etheridge, Darren, DLOS, LFBDEV, Florian Tobias Schandinat, LKML
In-Reply-To: <CA+V-a8u6vjSc5EV3z4FCQ8LEHxL79bUXtHzX8svhQwqVtj6w=g@mail.gmail.com>

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

On 26/06/13 15:06, Prabhakar Lad wrote:
> Hi Tomi,
> 
> On Wed, Jun 26, 2013 at 5:30 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 26/06/13 14:56, Prabhakar Lad wrote:
>>> Hi Tomi,
>>>
>>> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>>>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>>
>>>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>>>> devm_request_irq() instead of request_irq().
>>>>>
>>>>> This ensures more consistent error values and simplifies error paths.
>>>>>
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>> ---
>>>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>>>
>>>> There's something similar in Darren's da8xx-fb series. Can you check if
>>>> there are differences?
>>>>
>>> I don't see similar changes in his patch series.
>>
>> [PATCH 14/23] video: da8xx-fb: use devres
>>
> Oops missed it out :), patch is almost similar except
> for following block,
> 
> -       fb_clk = clk_get(&device->dev, "fck");
> +       fb_clk = devm_clk_get(&device->dev, "fck");
>         if (IS_ERR(fb_clk)) {
>                 dev_err(&device->dev, "Can not get device clock\n");
> -               ret = -ENODEV;
> -               goto err_ioremap;
> +               return -ENODEV;
>         }
> 
> How do you suggest to handle it ?

I think it's easiest if Darren handles the da8xx-fb series for 3.11. So
if there's something missing from Darren's series, please discuss with him.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2 v2] fb: fix atyfb unused data warnings
From: Tomi Valkeinen @ 2013-06-26 12:15 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Jean-Christophe PLAGNIOL-VILLARD, LKML,
	Linux Fbdev development list, Andrew Morton, Paul Mackerras,
	Benjamin Herrenschmidt
In-Reply-To: <51C887CC.6000301@infradead.org>

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

On 24/06/13 20:54, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Fix compiler warnings of data defined but not used by using the
> __maybe_unused attribute.  The date are only used with certain kconfig
> settings.
> 
> drivers/video/aty/atyfb_base.c:534:13: warning: 'ram_dram' defined but not used [-Wunused-variable]
> drivers/video/aty/atyfb_base.c:535:13: warning: 'ram_resv' defined but not used [-Wunused-variable]
> 

I've added this and the first patch in the series to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] uvesafb: Correct/simplify warning message
From: Tomi Valkeinen @ 2013-06-26 12:18 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1366380325-2038-1-git-send-email-bp@alien8.de>

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

On 19/04/13 17:05, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Streamline it a bit. No functional change.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Wang YanQing <udknight@gmail.com>
> Cc: Michal Januszewski <spock@gentoo.org>
> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> ---
>  drivers/video/uvesafb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
> index d4284458377e..a309f9fabe8b 100644
> --- a/drivers/video/uvesafb.c
> +++ b/drivers/video/uvesafb.c
> @@ -819,8 +819,8 @@ static int uvesafb_vbe_init(struct fb_info *info)
>  	if (par->pmi_setpal || par->ypan) {
>  		if (__supported_pte_mask & _PAGE_NX) {
>  			par->pmi_setpal = par->ypan = 0;
> -			printk(KERN_WARNING "uvesafb: NX protection is actively."
> -				"We have better not to use the PMI.\n");
> +			printk(KERN_WARNING "uvesafb: NX protection is active, "
> +					    "better not use the PMI.\n");
>  		} else {
>  			uvesafb_vbe_getpmi(task, par);
>  		}
> 

I've added this to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] video:da8xx-fb: Convert to devm_* api
From: Prabhakar Lad @ 2013-06-26 12:18 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Etheridge, Darren, DLOS, LFBDEV, Florian Tobias Schandinat, LKML
In-Reply-To: <51CAD7E4.2000403@ti.com>

Hi Tomi,

On Wed, Jun 26, 2013 at 5:30 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 26/06/13 14:56, Prabhakar Lad wrote:
>> Hi Tomi,
>>
>> On Wed, Jun 26, 2013 at 5:09 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On 16/05/13 10:10, Lad Prabhakar wrote:
>>>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>
>>>> Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
>>>> devm_request_irq() instead of request_irq().
>>>>
>>>> This ensures more consistent error values and simplifies error paths.
>>>>
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>> ---
>>>>  drivers/video/da8xx-fb.c |   39 +++++++--------------------------------
>>>>  1 files changed, 7 insertions(+), 32 deletions(-)
>>>
>>> There's something similar in Darren's da8xx-fb series. Can you check if
>>> there are differences?
>>>
>> I don't see similar changes in his patch series.
>
> [PATCH 14/23] video: da8xx-fb: use devres
>
Oops missed it out :), patch is almost similar except
for following block,

-       fb_clk = clk_get(&device->dev, "fck");
+       fb_clk = devm_clk_get(&device->dev, "fck");
        if (IS_ERR(fb_clk)) {
                dev_err(&device->dev, "Can not get device clock\n");
-               ret = -ENODEV;
-               goto err_ioremap;
+               return -ENODEV;
        }

How do you suggest to handle it ?

Regards,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH] video: replace strict_strtoul() with kstrtoul()
From: Tomi Valkeinen @ 2013-06-26 12:20 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <001401ce5e9a$03b485e0$0b1d91a0$@samsung.com>

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

On 01/06/13 10:31, Jingoo Han wrote:
> The usage of strict_strtoul() is not preferred, because
> strict_strtoul() is obsolete. Thus, kstrtoul() should be
> used.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/video/fsl-diu-fb.c                |    4 ++--
>  drivers/video/omap2/displays/panel-taal.c |    6 +++---
>  drivers/video/wm8505fb.c                  |    2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)

I've added this to my fbdev-3.11 branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Felipe Balbi @ 2013-06-26 12:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51CAD89E.3060800@samsung.com>

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

On Wed, Jun 26, 2013 at 02:03:42PM +0200, Sylwester Nawrocki wrote:
> On 06/26/2013 01:21 PM, Kishon Vijay Abraham I wrote:
> >>>>> +static int exynos_video_phy_probe(struct platform_device *pdev)
> >>>>> >>>> +{
> >>>>> >>>> +	struct exynos_video_phy *state;
> >>>>> >>>> +	struct device *dev = &pdev->dev;
> >>>>> >>>> +	struct resource *res;
> >>>>> >>>> +	struct phy_provider *phy_provider;
> >>>>> >>>> +	int i;
> >>>>> >>>> +
> >>>>> >>>> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> >>>>> >>>> +	if (!state)
> >>>>> >>>> +		return -ENOMEM;
> >>>>> >>>> +
> >>>>> >>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>>> >>>> +
> >>>>> >>>> +	state->regs = devm_ioremap_resource(dev, res);
> >>>>> >>>> +	if (IS_ERR(state->regs))
> >>>>> >>>> +		return PTR_ERR(state->regs);
> >>>>> >>>> +
> >>>>> >>>> +	dev_set_drvdata(dev, state);
> >>>> >>>
> >>>> >>> you can use platform_set_drvdata(pdev, state);
> >>> >>
> >>> >> I had it in the previous version, but changed for symmetry with
> >>> >> dev_set_drvdata(). I guess those could be replaced with
> >>> >> phy_{get, set}_drvdata as you suggested.
> >
> > right. currently I was setting dev_set_drvdata of phy (core) device
> > in phy-core.c and the corresponding dev_get_drvdata in phy provider driver
> > which is little confusing.
> > So I'll add phy_set_drvdata and phy_get_drvdata in phy.h (as suggested by
> > Felipe) to be used by phy provider drivers. So after creating the PHY, the
> > phy provider should use phy_set_drvdata and in phy_ops, it can use
> > phy_get_drvdata. (I'll remove the dev_set_drvdata in phy_create).
> > 
> > This also means _void *priv_ in phy_create is useless. So I'll be removing
> > _priv_ from phy_create.
> 
> Yeah, sounds good. Then in the phy ops phy_get_drvdata(&phy->dev) would

phy_get_drvdata(phy);

accessing the dev pointer will be done inside the helper :-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Kishon Vijay Abraham I @ 2013-06-26 12:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <51CAD89E.3060800@samsung.com>

On Wednesday 26 June 2013 05:33 PM, Sylwester Nawrocki wrote:
> On 06/26/2013 01:21 PM, Kishon Vijay Abraham I wrote:
>>>>>> +static int exynos_video_phy_probe(struct platform_device *pdev)
>>>>>>>>>> +{
>>>>>>>>>> +	struct exynos_video_phy *state;
>>>>>>>>>> +	struct device *dev = &pdev->dev;
>>>>>>>>>> +	struct resource *res;
>>>>>>>>>> +	struct phy_provider *phy_provider;
>>>>>>>>>> +	int i;
>>>>>>>>>> +
>>>>>>>>>> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
>>>>>>>>>> +	if (!state)
>>>>>>>>>> +		return -ENOMEM;
>>>>>>>>>> +
>>>>>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>>>>>> +
>>>>>>>>>> +	state->regs = devm_ioremap_resource(dev, res);
>>>>>>>>>> +	if (IS_ERR(state->regs))
>>>>>>>>>> +		return PTR_ERR(state->regs);
>>>>>>>>>> +
>>>>>>>>>> +	dev_set_drvdata(dev, state);
>>>>>>>>
>>>>>>>> you can use platform_set_drvdata(pdev, state);
>>>>>>
>>>>>> I had it in the previous version, but changed for symmetry with
>>>>>> dev_set_drvdata(). I guess those could be replaced with
>>>>>> phy_{get, set}_drvdata as you suggested.
>>
>> right. currently I was setting dev_set_drvdata of phy (core) device
>> in phy-core.c and the corresponding dev_get_drvdata in phy provider driver
>> which is little confusing.
>> So I'll add phy_set_drvdata and phy_get_drvdata in phy.h (as suggested by
>> Felipe) to be used by phy provider drivers. So after creating the PHY, the
>> phy provider should use phy_set_drvdata and in phy_ops, it can use
>> phy_get_drvdata. (I'll remove the dev_set_drvdata in phy_create).
>>
>> This also means _void *priv_ in phy_create is useless. So I'll be removing
>> _priv_ from phy_create.
>
> Yeah, sounds good. Then in the phy ops phy_get_drvdata(&phy->dev) would
> be used and in a custom of_xlate dev_get_drvdata(dev) (assuming the phy
> provider sets drvdata on its device beforehand).

thats correct. btw when you send the next version just have MODULE_LICENSE set
to GPL v2. Apart from that this patch looks good to me.

Thanks
Kishon

^ permalink raw reply

* Re: [PATCH v2 1/5] phy: Add driver for Exynos MIPI CSIS/DSIM DPHYs
From: Kishon Vijay Abraham I @ 2013-06-26 12:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20130626122238.GD12640@arwen.pp.htv.fi>

On Wednesday 26 June 2013 05:52 PM, Felipe Balbi wrote:
> On Wed, Jun 26, 2013 at 02:03:42PM +0200, Sylwester Nawrocki wrote:
>> On 06/26/2013 01:21 PM, Kishon Vijay Abraham I wrote:
>>>>>>> +static int exynos_video_phy_probe(struct platform_device *pdev)
>>>>>>>>>>> +{
>>>>>>>>>>> +	struct exynos_video_phy *state;
>>>>>>>>>>> +	struct device *dev = &pdev->dev;
>>>>>>>>>>> +	struct resource *res;
>>>>>>>>>>> +	struct phy_provider *phy_provider;
>>>>>>>>>>> +	int i;
>>>>>>>>>>> +
>>>>>>>>>>> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
>>>>>>>>>>> +	if (!state)
>>>>>>>>>>> +		return -ENOMEM;
>>>>>>>>>>> +
>>>>>>>>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>>>>>>> +
>>>>>>>>>>> +	state->regs = devm_ioremap_resource(dev, res);
>>>>>>>>>>> +	if (IS_ERR(state->regs))
>>>>>>>>>>> +		return PTR_ERR(state->regs);
>>>>>>>>>>> +
>>>>>>>>>>> +	dev_set_drvdata(dev, state);
>>>>>>>>>
>>>>>>>>> you can use platform_set_drvdata(pdev, state);
>>>>>>>
>>>>>>> I had it in the previous version, but changed for symmetry with
>>>>>>> dev_set_drvdata(). I guess those could be replaced with
>>>>>>> phy_{get, set}_drvdata as you suggested.
>>>
>>> right. currently I was setting dev_set_drvdata of phy (core) device
>>> in phy-core.c and the corresponding dev_get_drvdata in phy provider driver
>>> which is little confusing.
>>> So I'll add phy_set_drvdata and phy_get_drvdata in phy.h (as suggested by
>>> Felipe) to be used by phy provider drivers. So after creating the PHY, the
>>> phy provider should use phy_set_drvdata and in phy_ops, it can use
>>> phy_get_drvdata. (I'll remove the dev_set_drvdata in phy_create).
>>>
>>> This also means _void *priv_ in phy_create is useless. So I'll be removing
>>> _priv_ from phy_create.
>>
>> Yeah, sounds good. Then in the phy ops phy_get_drvdata(&phy->dev) would
>
> phy_get_drvdata(phy);
>
> accessing the dev pointer will be done inside the helper :-)

right :-)

-Kishon

^ permalink raw reply

* Re: [PATCH v4 0/7] xilinxfb changes
From: Michal Simek @ 2013-06-26 12:51 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Arnd Bergmann, Jean-Christophe PLAGNIOL-VILLARD, Michal Simek,
	linux-kernel, Timur Tabi, devicetree-discuss, linux-fbdev,
	Grant Likely, Rob Herring
In-Reply-To: <51CAD385.8060400@ti.com>

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

On 06/26/2013 01:41 PM, Tomi Valkeinen wrote:
> Hi,
> 
> On 26/06/13 13:02, Michal Simek wrote:
>> On 06/17/2013 11:07 PM, Arnd Bergmann wrote:
>>> On Monday 17 June 2013, Michal Simek wrote:
>>>> On 06/17/2013 10:56 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On 07:23 Mon 17 Jun     , Michal Simek wrote:
>>>>>> On 06/06/2013 06:23 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>>>> On 12:13 Mon 03 Jun     , Michal Simek wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>
>>>>>>> Arnd can you take look on it again please
>>>>>>>
>>>>>>> I'll take a look on it next week
>>>>>>
>>>>>> Any update on this?
>>>>> look ok but I want the Ack from Arnd
>>>
>>> Sorry for the delay, everything looks good to me.
>>>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>
>> Jean-Christophe: Will you apply this series?
>> Or should I take it through my microblaze tree?
> 
> I've added this to my fbdev-3.11 branch.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply

* Re: [PATCH] video: of_display_timing.h: Include <video/display_timing.h>
From: Tomi Valkeinen @ 2013-06-26 12:59 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1371476589-4660-1-git-send-email-fabio.estevam@freescale.com>

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

On 17/06/13 16:43, Fabio Estevam wrote:
> Commit ffa3fd21de ("videomode: implement public of_get_display_timing()") causes
> the following build warning:
> 
> include/video/of_display_timing.h:18:10: warning: 'struct display_timing' declared inside parameter list [enabled by default]
> include/video/of_display_timing.h:18:10: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default]
> 
> As 'struct display_timing' is defined at <video/display_timing.h>, let's include
> this header to avoid the warning.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  include/video/of_display_timing.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/video/of_display_timing.h b/include/video/of_display_timing.h
> index 6562ad9..a136f58 100644
> --- a/include/video/of_display_timing.h
> +++ b/include/video/of_display_timing.h
> @@ -8,6 +8,7 @@
>  
>  #ifndef __LINUX_OF_DISPLAY_TIMING_H
>  #define __LINUX_OF_DISPLAY_TIMING_H
> +#include <video/display_timing.h>
>  
>  struct device_node;
>  struct display_timings;
> 

We don't need to include display_timing.h, we can just add:

struct display_timing;

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [patch] fbmem: return -EFAULT on copy_to_user() failure
From: Tomi Valkeinen @ 2013-06-26 13:04 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel,
	kernel-janitors
In-Reply-To: <20130618070529.GA12329@elgon.mountain>

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

On 18/06/13 10:05, Dan Carpenter wrote:
> copy_to_user() returns the number of bytes remaining to be copied.
> put_user() returns -EFAULT on error.
> 
> This function ORs a bunch of stuff together and returns jumbled non-zero
> values on error.  It should return -EFAULT.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 098bfc6..9217be3 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1305,7 +1305,9 @@ static int do_fscreeninfo_to_user(struct fb_fix_screeninfo *fix,
>  	err |= copy_to_user(fix32->reserved, fix->reserved,
>  			    sizeof(fix->reserved));
>  
> -	return err;
> +	if (err)
> +		return -EFAULT;
> +	return 0;
>  }
>  
>  static int fb_get_fscreeninfo(struct fb_info *info, unsigned int cmd,

I've added this to fbdev-3.11 branch.

 Tomi




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 9/9] radeon: use pdev->pm_cap instead of pci_find_capability(..,PCI_CAP_ID_PM)
From: Tomi Valkeinen @ 2013-06-26 13:15 UTC (permalink / raw)
  To: Yijing Wang
  Cc: Andrew Morton, linux-kernel, Benjamin Herrenschmidt,
	Jean-Christophe Plagniol-Villard, linux-fbdev
In-Reply-To: <1372209221-16492-1-git-send-email-wangyijing@huawei.com>

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

On 26/06/13 04:13, Yijing Wang wrote:
> Pci core has been saved pm cap register offset by pdev->pm_cap in pci_pm_init()
> in init path. So we can use pdev->pm_cap instead of using
> pci_find_capability(pdev, PCI_CAP_ID_PM) for better performance and simplified code.
> 
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/video/aty/radeon_pm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/aty/radeon_pm.c b/drivers/video/aty/radeon_pm.c
> index 92bda58..f7091ec 100644
> --- a/drivers/video/aty/radeon_pm.c
> +++ b/drivers/video/aty/radeon_pm.c
> @@ -2805,7 +2805,7 @@ static void radeonfb_early_resume(void *data)
>  void radeonfb_pm_init(struct radeonfb_info *rinfo, int dynclk, int ignore_devlist, int force_sleep)
>  {
>  	/* Find PM registers in config space if any*/
> -	rinfo->pm_reg = pci_find_capability(rinfo->pdev, PCI_CAP_ID_PM);
> +	rinfo->pm_reg = rinfo->pdev->pm_cap;
>  
>  	/* Enable/Disable dynamic clocks: TODO add sysfs access */
>  	if (rinfo->family == CHIP_FAMILY_RS480)

I couldn't find the rest of this series, and I'm not familiar with PCI.
So: is this patch and "aty128fb: use pdev->pm_cap instead of
pci_find_capability(..,PCI_CAP_ID_PM)" safe to apply for fbdev-3.11
without anything else? I.e. has the PCI core changes been merged in 3.10
or ealier?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply


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