> On Mar 10, 2017, at 3:26 PM, Sascha Hauer <s.ha...@pengutronix.de> wrote:
> 
> On Thu, Mar 09, 2017 at 10:57:21AM +0100, Jean-Christophe PLAGNIOL-VILLARD 
> wrote:
>> so allow them to do not pass this function via fbops
>> and indicate they are always on.
>> 
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagn...@jcrosoft.com>
>> ---
>> drivers/video/fb.c | 29 ++++++++++++++++++++++++-----
>> 1 file changed, 24 insertions(+), 5 deletions(-)
> 
> I partly applied this one on top of
> 
> [PATCH] video: call fb_[en|dis]able instead of fops directly
> 
>> 
>> diff --git a/drivers/video/fb.c b/drivers/video/fb.c
>> index 6b88f2df9..63a818eb2 100644
>> --- a/drivers/video/fb.c
>> +++ b/drivers/video/fb.c
>> @@ -19,10 +19,12 @@ static int fb_ioctl(struct cdev* cdev, int req, void 
>> *data)
>>              *fb = info;
>>              break;
>>      case FBIO_ENABLE:
>> -            info->fbops->fb_enable(info);
>> +            if (info->fbops->fb_enable)
>> +                    info->fbops->fb_enable(info);
>>              break;
>>      case FBIO_DISABLE:
>> -            info->fbops->fb_disable(info);
>> +            if (info->fbops->fb_disable)
>> +                    info->fbops->fb_disable(info);
>>              break;
>>      default:
>>              return -ENOSYS;
>> @@ -94,7 +96,8 @@ int fb_enable(struct fb_info *info)
>>      if (ret)
>>              return ret;
>> 
>> -    info->fbops->fb_enable(info);
>> +    if (info->fbops->fb_enable)
>> +            info->fbops->fb_enable(info);
>> 
>>      info->enabled = true;
>> 
>> @@ -106,7 +109,8 @@ int fb_disable(struct fb_info *info)
>>      if (!info->enabled)
>>              return 0;
>> 
>> -    info->fbops->fb_disable(info);
>> +    if (info->fbops->fb_disable)
>> +            info->fbops->fb_disable(info);
> 
> Making fb_enable/disable optional is fine and I applied this part.
> 
>> 
>>      fb_release_shadowfb(info);
>> 
>> @@ -266,6 +270,8 @@ static int fb_set_shadowfb(struct param_d *p, void *priv)
>>      return fb_alloc_shadowfb(info);
>> }
>> 
>> +struct fb_ops dummy_fbops;
>> +
>> int register_framebuffer(struct fb_info *info)
>> {
>>      int id = get_free_deviceid("fb");
>> @@ -273,6 +279,16 @@ int register_framebuffer(struct fb_info *info)
>>      int ret, num_modes, i;
>>      const char **names;
>> 
>> +    if (!info->p_enable) {
>> +            if (!info->fbops)
>> +                    return -EINVAL;
>> +            if (!info->fbops->fb_enable && !info->fbops->fb_disable)
>> +                    return -EINVAL;
>> +    } else {
>> +            if (!info->fbops)
>> +                    info->fbops = &dummy_fbops;
>> +    }
> 
> Making fbops optional is not what the patch descsription says.
It does as if you need nothing from fbops you will not even implement it
> 
>> +
>>      dev = &info->dev;
>> 
>>      /*
>> @@ -308,7 +324,10 @@ int register_framebuffer(struct fb_info *info)
>>      if (ret)
>>              goto err_free;
>> 
>> -    dev_add_param_bool(dev, "enable", fb_enable_set, NULL,
>> +    if (info->p_enable)
>> +            dev_add_param_int_ro(dev, "enable", info->p_enable, "%d");
>> +    else
>> +            dev_add_param_bool(dev, "enable", fb_enable_set, NULL,
>>                      &info->p_enable, info);
> 
> Even when the framebuffer cannot physically disabled I think we
> shouldn't throw an error when the user tries to.
this will be done by the env as I fixed the …_int_ro support
as enable will not we able to changed

Best Regards,
J.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to