Re: [PATCH 07/13] eeepc-laptop: make disp attribute really write-only

2014-09-15 Thread Frans Klaver 
On Mon, Sep 15, 2014 at 01:00:07PM -0700, Darren Hart wrote:
> On Sat, Sep 13, 2014 at 01:06:46AM +0200, Frans Klaver wrote:
> > The disp attribute is write-only, but sysfs doesn't know this. Currently
> > show_sys_acpi() is mimicking sysfs behavior, if the underlying acpi call
> > should fail. This is not ideal; behaving like sysfs is better left to
> > sysfs.
> > 
> > Introduce EEEPC_CREATE_DEVICE_ATTR_WO() to instantiate a write-only
> > attribute, and declare the disp attribute with it. Sysfs makes sure
> > userspace can only write to disp at all times. This also means we can
> > back to propagating any errors from get_acpi().
> > 
> > Signed-off-by: Frans Klaver 
> > ---
> >  drivers/platform/x86/eeepc-laptop.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/eeepc-laptop.c 
> > b/drivers/platform/x86/eeepc-laptop.c
> > index 3c04b77..4f6490d 100644
> > --- a/drivers/platform/x86/eeepc-laptop.c
> > +++ b/drivers/platform/x86/eeepc-laptop.c
> > @@ -291,7 +291,7 @@ static ssize_t show_sys_acpi(struct device *dev, int 
> > cm, char *buf)
> > int value = get_acpi(eeepc, cm);
> >  
> > if (value < 0)
> > -   return -EIO;
> > +   return value;
> > return sprintf(buf, "%d\n", value);
> >  }
> >  
> > @@ -316,9 +316,13 @@ static ssize_t show_sys_acpi(struct device *dev, int 
> > cm, char *buf)
> > EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
> > static DEVICE_ATTR(_name, _mode, show_##_name, store_##_name)
> >  
> > +#define EEEPC_CREATE_DEVICE_ATTR_WO(_name, _mode, _cm) 
> > \
> > +   EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
> > +   static DEVICE_ATTR(_name, _mode, NULL, store_##_name)
> 
> Per Greg's previous reply, I presume DEVICE_ATTR_WO here?

Yes, already taken care of in all similar cases.

Thanks,
Frans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/13] eeepc-laptop: make disp attribute really write-only

2014-09-15 Thread Darren Hart 
On Sat, Sep 13, 2014 at 01:06:46AM +0200, Frans Klaver wrote:
> The disp attribute is write-only, but sysfs doesn't know this. Currently
> show_sys_acpi() is mimicking sysfs behavior, if the underlying acpi call
> should fail. This is not ideal; behaving like sysfs is better left to
> sysfs.
> 
> Introduce EEEPC_CREATE_DEVICE_ATTR_WO() to instantiate a write-only
> attribute, and declare the disp attribute with it. Sysfs makes sure
> userspace can only write to disp at all times. This also means we can
> back to propagating any errors from get_acpi().
> 
> Signed-off-by: Frans Klaver 
> ---
>  drivers/platform/x86/eeepc-laptop.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/eeepc-laptop.c 
> b/drivers/platform/x86/eeepc-laptop.c
> index 3c04b77..4f6490d 100644
> --- a/drivers/platform/x86/eeepc-laptop.c
> +++ b/drivers/platform/x86/eeepc-laptop.c
> @@ -291,7 +291,7 @@ static ssize_t show_sys_acpi(struct device *dev, int cm, 
> char *buf)
>   int value = get_acpi(eeepc, cm);
>  
>   if (value < 0)
> - return -EIO;
> + return value;
>   return sprintf(buf, "%d\n", value);
>  }
>  
> @@ -316,9 +316,13 @@ static ssize_t show_sys_acpi(struct device *dev, int cm, 
> char *buf)
>   EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
>   static DEVICE_ATTR(_name, _mode, show_##_name, store_##_name)
>  
> +#define EEEPC_CREATE_DEVICE_ATTR_WO(_name, _mode, _cm)   
> \
> + EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
> + static DEVICE_ATTR(_name, _mode, NULL, store_##_name)

Per Greg's previous reply, I presume DEVICE_ATTR_WO here?

> +
>  EEEPC_CREATE_DEVICE_ATTR(camera, S_IWUSR | S_IRUGO, CM_ASL_CAMERA);
>  EEEPC_CREATE_DEVICE_ATTR(cardr, S_IWUSR | S_IRUGO, CM_ASL_CARDREADER);
> -EEEPC_CREATE_DEVICE_ATTR(disp, S_IWUSR, CM_ASL_DISPLAYSWITCH);
> +EEEPC_CREATE_DEVICE_ATTR_WO(disp, S_IWUSR, CM_ASL_DISPLAYSWITCH);
>  
>  struct eeepc_cpufv {
>   int num;
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe 
> platform-driver-x86" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/13] eeepc-laptop: make disp attribute really write-only

2014-09-15 Thread Darren Hart 
On Sat, Sep 13, 2014 at 01:06:46AM +0200, Frans Klaver wrote:
 The disp attribute is write-only, but sysfs doesn't know this. Currently
 show_sys_acpi() is mimicking sysfs behavior, if the underlying acpi call
 should fail. This is not ideal; behaving like sysfs is better left to
 sysfs.
 
 Introduce EEEPC_CREATE_DEVICE_ATTR_WO() to instantiate a write-only
 attribute, and declare the disp attribute with it. Sysfs makes sure
 userspace can only write to disp at all times. This also means we can
 back to propagating any errors from get_acpi().
 
 Signed-off-by: Frans Klaver franskla...@gmail.com
 ---
  drivers/platform/x86/eeepc-laptop.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/platform/x86/eeepc-laptop.c 
 b/drivers/platform/x86/eeepc-laptop.c
 index 3c04b77..4f6490d 100644
 --- a/drivers/platform/x86/eeepc-laptop.c
 +++ b/drivers/platform/x86/eeepc-laptop.c
 @@ -291,7 +291,7 @@ static ssize_t show_sys_acpi(struct device *dev, int cm, 
 char *buf)
   int value = get_acpi(eeepc, cm);
  
   if (value  0)
 - return -EIO;
 + return value;
   return sprintf(buf, %d\n, value);
  }
  
 @@ -316,9 +316,13 @@ static ssize_t show_sys_acpi(struct device *dev, int cm, 
 char *buf)
   EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
   static DEVICE_ATTR(_name, _mode, show_##_name, store_##_name)
  
 +#define EEEPC_CREATE_DEVICE_ATTR_WO(_name, _mode, _cm)   
 \
 + EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
 + static DEVICE_ATTR(_name, _mode, NULL, store_##_name)

Per Greg's previous reply, I presume DEVICE_ATTR_WO here?

 +
  EEEPC_CREATE_DEVICE_ATTR(camera, S_IWUSR | S_IRUGO, CM_ASL_CAMERA);
  EEEPC_CREATE_DEVICE_ATTR(cardr, S_IWUSR | S_IRUGO, CM_ASL_CARDREADER);
 -EEEPC_CREATE_DEVICE_ATTR(disp, S_IWUSR, CM_ASL_DISPLAYSWITCH);
 +EEEPC_CREATE_DEVICE_ATTR_WO(disp, S_IWUSR, CM_ASL_DISPLAYSWITCH);
  
  struct eeepc_cpufv {
   int num;
 -- 
 2.1.0
 
 --
 To unsubscribe from this list: send the line unsubscribe 
 platform-driver-x86 in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/13] eeepc-laptop: make disp attribute really write-only

2014-09-15 Thread Frans Klaver 
On Mon, Sep 15, 2014 at 01:00:07PM -0700, Darren Hart wrote:
 On Sat, Sep 13, 2014 at 01:06:46AM +0200, Frans Klaver wrote:
  The disp attribute is write-only, but sysfs doesn't know this. Currently
  show_sys_acpi() is mimicking sysfs behavior, if the underlying acpi call
  should fail. This is not ideal; behaving like sysfs is better left to
  sysfs.
  
  Introduce EEEPC_CREATE_DEVICE_ATTR_WO() to instantiate a write-only
  attribute, and declare the disp attribute with it. Sysfs makes sure
  userspace can only write to disp at all times. This also means we can
  back to propagating any errors from get_acpi().
  
  Signed-off-by: Frans Klaver franskla...@gmail.com
  ---
   drivers/platform/x86/eeepc-laptop.c | 8 ++--
   1 file changed, 6 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/platform/x86/eeepc-laptop.c 
  b/drivers/platform/x86/eeepc-laptop.c
  index 3c04b77..4f6490d 100644
  --- a/drivers/platform/x86/eeepc-laptop.c
  +++ b/drivers/platform/x86/eeepc-laptop.c
  @@ -291,7 +291,7 @@ static ssize_t show_sys_acpi(struct device *dev, int 
  cm, char *buf)
  int value = get_acpi(eeepc, cm);
   
  if (value  0)
  -   return -EIO;
  +   return value;
  return sprintf(buf, %d\n, value);
   }
   
  @@ -316,9 +316,13 @@ static ssize_t show_sys_acpi(struct device *dev, int 
  cm, char *buf)
  EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
  static DEVICE_ATTR(_name, _mode, show_##_name, store_##_name)
   
  +#define EEEPC_CREATE_DEVICE_ATTR_WO(_name, _mode, _cm) 
  \
  +   EEEPC_ACPI_STORE_FUNC(_name, _cm)   \
  +   static DEVICE_ATTR(_name, _mode, NULL, store_##_name)
 
 Per Greg's previous reply, I presume DEVICE_ATTR_WO here?

Yes, already taken care of in all similar cases.

Thanks,
Frans
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/