[PATCH] Allows setting bpp from dt

[PATCH] Allows setting bpp from dt

[Radoslaw Pietrzyk]

Allows setting bpp from dt

Radoslaw Pietrzyk (1):
  staging: fbtft: Allows bpp to be set from dt

 drivers/staging/fbtft/fbtft-core.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.9.1

[PATCH] staging: fbtft: Allows bpp to be set from dt

[Radoslaw Pietrzyk]

Allows bpp to be set from dt

Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
 drivers/staging/fbtft/fbtft-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7c8af29..b25288b 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -685,6 +685,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	/* override driver values? */
 	if (pdata->fps)
 		fps = pdata->fps;
+	if (pdata->bpp)
+		bpp = pdata->bpp;
 	if (pdata->txbuflen)
 		txbuflen = pdata->txbuflen;
 	if (pdata->display.init_sequence)
-- 
1.9.1

Re: [PATCH] staging: fbtft: Allows bpp to be set from dt

[Greg KH]

On Fri, Mar 10, 2017 at 10:25:27AM +0100, Radoslaw Pietrzyk wrote:
> Allows bpp to be set from dt
> 
> Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft-core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 7c8af29..b25288b 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -685,6 +685,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
>  	/* override driver values? */
>  	if (pdata->fps)
>  		fps = pdata->fps;
> +	if (pdata->bpp)
> +		bpp = pdata->bpp;
>  	if (pdata->txbuflen)
>  		txbuflen = pdata->txbuflen;
>  	if (pdata->display.init_sequence)

This breaks the build, how did you test this???

thanks,

greg k-h

Re: [PATCH] staging: fbtft: Allows bpp to be set from dt

[kbuild test robot]

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

Hi Radoslaw,

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Radoslaw-Pietrzyk/staging-fbtft-Allows-bpp-to-be-set-from-dt/20170313-053026
config: i386-randconfig-x000-201711 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/linux/list.h:4,
                    from include/linux/module.h:9,
                    from drivers/staging/fbtft/fbtft-core.c:19:
   drivers/staging/fbtft/fbtft-core.c: In function 'fbtft_framebuffer_alloc':
   drivers/staging/fbtft/fbtft-core.c:702:11: error: 'struct fbtft_platform_data' has no member named 'bpp'
     if (pdata->bpp)
              ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/staging/fbtft/fbtft-core.c:702:2: note: in expansion of macro 'if'
     if (pdata->bpp)
     ^~
   drivers/staging/fbtft/fbtft-core.c:702:11: error: 'struct fbtft_platform_data' has no member named 'bpp'
     if (pdata->bpp)
              ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/staging/fbtft/fbtft-core.c:702:2: note: in expansion of macro 'if'
     if (pdata->bpp)
     ^~
   drivers/staging/fbtft/fbtft-core.c:702:11: error: 'struct fbtft_platform_data' has no member named 'bpp'
     if (pdata->bpp)
              ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/staging/fbtft/fbtft-core.c:702:2: note: in expansion of macro 'if'
     if (pdata->bpp)
     ^~
   drivers/staging/fbtft/fbtft-core.c:703:14: error: 'struct fbtft_platform_data' has no member named 'bpp'
      bpp = pdata->bpp;
                 ^~

vim +/if +702 drivers/staging/fbtft/fbtft-core.c

   686		}
   687	
   688		/* defaults */
   689		if (!fps)
   690			fps = 20;
   691		if (!bpp)
   692			bpp = 16;
   693	
   694		if (!pdata) {
   695			dev_err(dev, "platform data is missing\n");
   696			return NULL;
   697		}
   698	
   699		/* override driver values? */
   700		if (pdata->fps)
   701			fps = pdata->fps;
 > 702		if (pdata->bpp)
   703			bpp = pdata->bpp;
   704		if (pdata->txbuflen)
   705			txbuflen = pdata->txbuflen;
   706		if (pdata->display.init_sequence)
   707			init_sequence = pdata->display.init_sequence;
   708		if (pdata->gamma)
   709			gamma = pdata->gamma;
   710		if (pdata->display.debug)

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 21476 bytes --]

[PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Radoslaw Pietrzyk]

Allows bpp to be set from dt

Signed-off-by: Radoslaw Pietrzyk <radoslaw.pietrzyk@gmail.com>
---
 drivers/staging/fbtft/fbtft-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 7c8af29..cb0c10d 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -685,6 +685,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	/* override driver values? */
 	if (pdata->fps)
 		fps = pdata->fps;
+	if (pdata->display.bpp)
+		bpp = pdata->display.bpp;
 	if (pdata->txbuflen)
 		txbuflen = pdata->txbuflen;
 	if (pdata->display.init_sequence)
-- 
1.9.1

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Dan Carpenter]

On Mon, Mar 13, 2017 at 11:28:45AM +0100, Radoslaw Pietrzyk wrote:
> Allows bpp to be set from dt
> 

Who does this affect in real life?  You haven't tested it apparently so
it's hard to say why you are doing this.

regards,
dan carpenter

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Radosław Pietrzyk]

I have and used with stm32 and fb_ili9341. First patch was shitty, my bad.

2017-03-13 12:00 GMT+01:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Mon, Mar 13, 2017 at 11:28:45AM +0100, Radoslaw Pietrzyk wrote:
>> Allows bpp to be set from dt
>>
>
> Who does this affect in real life?  You haven't tested it apparently so
> it's hard to say why you are doing this.
>
> regards,
> dan carpenter
>

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Dan Carpenter]

On Mon, Mar 13, 2017 at 12:07:20PM +0100, Radosław Pietrzyk wrote:
> I have and used with stm32 and fb_ili9341. First patch was shitty, my bad.
> 

Could you resend with a patch description which says why you are doing
this?

"With stm32 and fb_ili9341 the monitor just sits there blinking because
bpp is not set" or something.

regards,
dan carpenter

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Radosław Pietrzyk]

Ok I will. I just have thought that in general it might be helpful to
have this possibility as DT is parsed for this option anyway. If you
think it is not needed at all that why to bother parsing DT for this
option in the first place ?

2017-03-13 12:11 GMT+01:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Mon, Mar 13, 2017 at 12:07:20PM +0100, Radosław Pietrzyk wrote:
>> I have and used with stm32 and fb_ili9341. First patch was shitty, my bad.
>>
>
> Could you resend with a patch description which says why you are doing
> this?
>
> "With stm32 and fb_ili9341 the monitor just sits there blinking because
> bpp is not set" or something.
>
> regards,
> dan carpenter
>

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Dan Carpenter]

On Mon, Mar 13, 2017 at 12:16:13PM +0100, Radosław Pietrzyk wrote:
> Ok I will. I just have thought that in general it might be helpful to
> have this possibility as DT is parsed for this option anyway. If you
> think it is not needed at all that why to bother parsing DT for this
> option in the first place ?

I'm not saying it's a wrong patch, I just think the patch description
doesn't explain why you are doing it or the user visible effects of the
change.

regards,
dan carpenter

Re: [PATCH v2] staging: fbtft: Allows bpp to be set from dt

[Radosław Pietrzyk]

Understood

2017-03-13 12:19 GMT+01:00 Dan Carpenter <dan.carpenter@oracle.com>:
> On Mon, Mar 13, 2017 at 12:16:13PM +0100, Radosław Pietrzyk wrote:
>> Ok I will. I just have thought that in general it might be helpful to
>> have this possibility as DT is parsed for this option anyway. If you
>> think it is not needed at all that why to bother parsing DT for this
>> option in the first place ?
>
> I'm not saying it's a wrong patch, I just think the patch description
> doesn't explain why you are doing it or the user visible effects of the
> change.
>
> regards,
> dan carpenter
>