mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] parameter: fix read only int support
@ 2017-03-09  9:57 Jean-Christophe PLAGNIOL-VILLARD
  2017-03-09  9:57 ` [PATCH 2/2] video: some framebuffer such as efi does support enable/disable Jean-Christophe PLAGNIOL-VILLARD
  2017-03-10  7:14 ` [PATCH 1/2] parameter: fix read only int support Sascha Hauer
  0 siblings, 2 replies; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09  9:57 UTC (permalink / raw)
  To: barebox

pass PARAM_FLAG_RO flag for read only it
so we can not change them

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 lib/parameter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/parameter.c b/lib/parameter.c
index 9f96d0760..65d6c7c0d 100644
--- a/lib/parameter.c
+++ b/lib/parameter.c
@@ -692,7 +692,7 @@ struct param_d *dev_add_param_int_ro(struct device_d *dev, const char *name,
 
 	piro = xzalloc(sizeof(*piro));
 
-	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, 0);
+	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, PARAM_FLAG_RO);
 	if (ret) {
 		free(piro);
 		return ERR_PTR(ret);
@@ -718,7 +718,7 @@ struct param_d *dev_add_param_llint_ro(struct device_d *dev, const char *name,
 
 	piro = xzalloc(sizeof(*piro));
 
-	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, 0);
+	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, PARAM_FLAG_RO);
 	if (ret) {
 		free(piro);
 		return ERR_PTR(ret);
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 2/2] video: some framebuffer such as efi does support enable/disable
  2017-03-09  9:57 [PATCH 1/2] parameter: fix read only int support Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-09  9:57 ` Jean-Christophe PLAGNIOL-VILLARD
  2017-03-10  7:26   ` Sascha Hauer
  2017-03-10  7:14 ` [PATCH 1/2] parameter: fix read only int support Sascha Hauer
  1 sibling, 1 reply; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-09  9:57 UTC (permalink / raw)
  To: barebox

so allow them to do not pass this function via fbops
and indicate they are always on.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/video/fb.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

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);
 
 	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;
+	}
+
 	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);
 
 	if (IS_ENABLED(CONFIG_DRIVER_VIDEO_EDID))
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] parameter: fix read only int support
  2017-03-09  9:57 [PATCH 1/2] parameter: fix read only int support Jean-Christophe PLAGNIOL-VILLARD
  2017-03-09  9:57 ` [PATCH 2/2] video: some framebuffer such as efi does support enable/disable Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-10  7:14 ` Sascha Hauer
  1 sibling, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2017-03-10  7:14 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Thu, Mar 09, 2017 at 10:57:20AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> pass PARAM_FLAG_RO flag for read only it
> so we can not change them
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  lib/parameter.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied (only this one), thanks

Sascha

> 
> diff --git a/lib/parameter.c b/lib/parameter.c
> index 9f96d0760..65d6c7c0d 100644
> --- a/lib/parameter.c
> +++ b/lib/parameter.c
> @@ -692,7 +692,7 @@ struct param_d *dev_add_param_int_ro(struct device_d *dev, const char *name,
>  
>  	piro = xzalloc(sizeof(*piro));
>  
> -	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, 0);
> +	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, PARAM_FLAG_RO);
>  	if (ret) {
>  		free(piro);
>  		return ERR_PTR(ret);
> @@ -718,7 +718,7 @@ struct param_d *dev_add_param_llint_ro(struct device_d *dev, const char *name,
>  
>  	piro = xzalloc(sizeof(*piro));
>  
> -	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, 0);
> +	ret = __dev_add_param(&piro->param, dev, name, NULL, NULL, PARAM_FLAG_RO);
>  	if (ret) {
>  		free(piro);
>  		return ERR_PTR(ret);
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] video: some framebuffer such as efi does support enable/disable
  2017-03-09  9:57 ` [PATCH 2/2] video: some framebuffer such as efi does support enable/disable Jean-Christophe PLAGNIOL-VILLARD
@ 2017-03-10  7:26   ` Sascha Hauer
  2017-03-10  9:37     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2017-03-10  7:26 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

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 <plagnioj@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.

> +
>  	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.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] video: some framebuffer such as efi does support enable/disable
  2017-03-10  7:26   ` Sascha Hauer
@ 2017-03-10  9:37     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 5+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2017-03-10  9:37 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox


> On Mar 10, 2017, at 3:26 PM, Sascha Hauer <s.hauer@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 <plagnioj@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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-10  9:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09  9:57 [PATCH 1/2] parameter: fix read only int support Jean-Christophe PLAGNIOL-VILLARD
2017-03-09  9:57 ` [PATCH 2/2] video: some framebuffer such as efi does support enable/disable Jean-Christophe PLAGNIOL-VILLARD
2017-03-10  7:26   ` Sascha Hauer
2017-03-10  9:37     ` Jean-Christophe PLAGNIOL-VILLARD
2017-03-10  7:14 ` [PATCH 1/2] parameter: fix read only int support Sascha Hauer

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