Re: [PATCH v6 12/13] Documentation: remove stale firmware API reference

[Mauro Carvalho Chehab]

Em Tue,  8 May 2018 11:12:46 -0700
"Luis R. Rodriguez" <mcg...@kernel.org> escreveu:

> It refers to a pending patch, but this was merged eons ago.

Didn't know that such patch was already merged. Great!

Regards,
Mauro

> 
> Signed-off-by: Luis R. Rodriguez <mcg...@kernel.org>
> ---
>  Documentation/dell_rbu.txt | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt
> index 0fdb6aa2704c..077fdc29a0d0 100644
> --- a/Documentation/dell_rbu.txt
> +++ b/Documentation/dell_rbu.txt
> @@ -121,9 +121,6 @@ read back the image downloaded.
>  
>  .. note::
>  
> -   This driver requires a patch for firmware_class.c which has the modified
> -   request_firmware_nowait function.
> -

> Also after updating the BIOS image a user mode application needs to 
> execute
> code which sends the BIOS update request to the BIOS. So on the next 
> reboot
> the BIOS knows about the new image downloaded and it updates itself.

You should likely remove the "Also" here.

With that,

Reviewed-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>

Regards,
Mauro

Re: [PATCH v6 13/13] Documentation: clarify firmware_class provenance and why we can't rename the module

[Mauro Carvalho Chehab]

Em Tue,  8 May 2018 11:12:47 -0700
"Luis R. Rodriguez" <mcg...@kernel.org> escreveu:

> Clarify the provenance of the firmware loader firmware_class module name
> and why we cannot rename the module in the future.
> 
> Signed-off-by: Luis R. Rodriguez <mcg...@kernel.org>
> ---
>  .../driver-api/firmware/fallback-mechanisms.rst  | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst 
> b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> index a39323ef7d29..a8047be4a96e 100644
> --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst
> +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst
> @@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device 
> hierarchy by
>  associating the device used to make the request as the device's parent.
>  The sysfs directory's file attributes are defined and controlled through
>  the new device's class (firmware_class) and group (fw_dev_attr_groups).
> -This is actually where the original firmware_class.c file name comes from,
> -as originally the only firmware loading mechanism available was the
> -mechanism we now use as a fallback mechanism.
> +This is actually where the original firmware_class module name came from,
> +given that originally the only firmware loading mechanism available was the
> +mechanism we now use as a fallback mechanism, which which registers a
> +struct class firmware_class. Because the attributes exposed are part of the
> +module name, the module name firmware_class cannot be renamed in the future, 
> to
> +ensure backward compatibilty with old userspace.

Ah, now the explanation makes a lot more sense to me :-)

Reviewed-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>

>  
>  To load firmware using the sysfs interface we expose a loading indicator,
>  and a file upload firmware into:



Thanks,
Mauro

Re: [trivial PATCH] treewide: Align function definition open/close braces

[Mauro Carvalho Chehab]

Em Sun, 17 Dec 2017 16:28:44 -0800
Joe Perches <j...@perches.com> escreveu:

> Some functions definitions have either the initial open brace and/or
> the closing brace outside of column 1.
> 
> Move those braces to column 1.
> 
> This allows various function analyzers like gnu complexity to work
> properly for these modified functions.
> 
> Miscellanea:
> 
> o Remove extra trailing ; and blank line from xfs_agf_verify
> 
> Signed-off-by: Joe Perches <j...@perches.com>

For the media patch:

Acked-by: Mauro Carvalho Chehab <mche...@s-opensource.com>

> ---
> git diff -w shows no difference other than the above 'Miscellanea'
> 
> (this is against -next, but it applies against Linus' tree
>  with a couple offsets)
> 
>  arch/x86/include/asm/atomic64_32.h   |  2 +-
>  drivers/acpi/custom_method.c |  2 +-
>  drivers/acpi/fan.c   |  2 +-
>  drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
>  drivers/media/i2c/msp3400-kthreads.c |  2 +-
>  drivers/message/fusion/mptsas.c  |  2 +-
>  drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
>  drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
>  drivers/platform/x86/eeepc-laptop.c  |  2 +-
>  drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
>  drivers/scsi/dpt_i2o.c   |  2 +-
>  drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
>  fs/locks.c   |  2 +-
>  fs/ocfs2/stack_user.c|  2 +-
>  fs/xfs/libxfs/xfs_alloc.c|  5 ++---
>  fs/xfs/xfs_export.c  |  2 +-
>  kernel/audit.c   |  6 +++---
>  kernel/trace/trace_printk.c  |  4 ++--
>  lib/raid6/sse2.c | 14 +++---
>  sound/soc/fsl/fsl_dma.c  |  2 +-
>  20 files changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/x86/include/asm/atomic64_32.h 
> b/arch/x86/include/asm/atomic64_32.h
> index 97c46b8169b7..d4d4883080fa 100644
> --- a/arch/x86/include/asm/atomic64_32.h
> +++ b/arch/x86/include/asm/atomic64_32.h
> @@ -122,7 +122,7 @@ static inline long long atomic64_read(const atomic64_t *v)
>   long long r;
>   alternative_atomic64(read, "=" (r), "c" (v) : "memory");
>   return r;
> - }
> +}
>  
>  /**
>   * atomic64_add_return - add and return
> diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
> index c68e72414a67..e967c1173ba3 100644
> --- a/drivers/acpi/custom_method.c
> +++ b/drivers/acpi/custom_method.c
> @@ -94,7 +94,7 @@ static void __exit acpi_custom_method_exit(void)
>  {
>   if (cm_dentry)
>   debugfs_remove(cm_dentry);
> - }
> +}
>  
>  module_init(acpi_custom_method_init);
>  module_exit(acpi_custom_method_exit);
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 6cf4988206f2..3563103590c6 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
> unsigned long state)
>   return fan_set_state_acpi4(device, state);
>   else
>   return fan_set_state(device, state);
> - }
> +}
>  
>  static const struct thermal_cooling_device_ops fan_cooling_ops = {
>   .get_max_state = fan_get_max_state,
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index d1488d5ee028..1e0d1e7c5324 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -461,7 +461,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
> dc_state *context)
>   
> **/
>  
>  struct dc *dc_create(const struct dc_init_data *init_params)
> - {
> +{
>   struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
>   unsigned int full_pipe_count;
>  
> diff --git a/drivers/media/i2c/msp3400-kthreads.c 
> b/drivers/media/i2c/msp3400-kthreads.c
> index 4dd01e9f553b..dc6cb8d475b3 100644
> --- a/drivers/media/i2c/msp3400-kthreads.c
> +++ b/drivers/media/i2c/msp3400-kthreads.c
> @@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
>  }
>  
>  static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
> - {
> +{
>   struct msp_state *state = to_state(i2c_get_clientdata(client));
>   int source, matrix;
>  
> diff --git 

Re: [PATCH 00/18] use ARRAY_SIZE macro

[Mauro Carvalho Chehab]

Em Sun, 1 Oct 2017 20:52:20 -0400
Jérémy Lefaure  escreveu:

> Anyway, I can tell to each maintainer that they can apply the patches
> they're concerned about and next time I may send individual patches.

In the case of media, we'll handle it as if they were individual ones.

Thanks,
Mauro

[PATCH v2 33/53] scsi: fix some kernel-doc markups

[Mauro Carvalho Chehab]

Sphinx is very pedantic with regards to ident/spacing.
Fix some kernel-doc markups in order to solve those
errors/warnings:

./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsi_scan.c:1056: ERROR: Unexpected indentation.
./drivers/scsi/scsi_scan.c:1057: WARNING: Block quote ends without a blank 
line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2918: ERROR: Unexpected indentation.
./drivers/scsi/scsi_transport_fc.c:2921: WARNING: Block quote ends without a 
blank line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2922: WARNING: Enumerated list ends without 
a blank line; unexpected unindent.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---
 drivers/scsi/scsi_scan.c |  7 ---
 drivers/scsi/scsi_transport_fc.c | 18 ++
 drivers/scsi/scsicam.c   |  4 ++--
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f49c30..69979574004f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1051,10 +1051,11 @@ static unsigned char *scsi_inq_str(unsigned char *buf, 
unsigned char *inq,
  * allocate and set it up by calling scsi_add_lun.
  *
  * Return:
- * SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
- * SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
+ *
+ *   - SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
+ *   - SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
  * attached at the LUN
- * SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
+ *   - SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
  **/
 static int scsi_probe_and_add_lun(struct scsi_target *starget,
  u64 lun, int *bflagsp,
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index d4cf32d55546..1df77453f6b6 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2914,16 +2914,18 @@ EXPORT_SYMBOL(fc_remote_port_add);
  * port is no longer part of the topology. Note: Although a port
  * may no longer be part of the topology, it may persist in the remote
  * ports displayed by the fc_host. We do this under 2 conditions:
+ *
  * 1) If the port was a scsi target, we delay its deletion by "blocking" it.
- *   This allows the port to temporarily disappear, then reappear without
- *   disrupting the SCSI device tree attached to it. During the "blocked"
- *   period the port will still exist.
+ *This allows the port to temporarily disappear, then reappear without
+ *disrupting the SCSI device tree attached to it. During the "blocked"
+ *period the port will still exist.
+ *
  * 2) If the port was a scsi target and disappears for longer than we
- *   expect, we'll delete the port and the tear down the SCSI device tree
- *   attached to it. However, we want to semi-persist the target id assigned
- *   to that port if it eventually does exist. The port structure will
- *   remain (although with minimal information) so that the target id
- *   bindings remails.
+ *expect, we'll delete the port and the tear down the SCSI device tree
+ *attached to it. However, we want to semi-persist the target id assigned
+ *to that port if it eventually does exist. The port structure will
+ *remain (although with minimal information) so that the target id
+ *bindings remails.
  *
  * If the remote port is not an FCP Target, it will be fully torn down
  * and deallocated, including the fc_remote_port class device.
diff --git a/drivers/scsi/scsicam.c b/drivers/scsi/scsicam.c
index 910f4a7a3924..31273468589c 100644
--- a/drivers/scsi/scsicam.c
+++ b/drivers/scsi/scsicam.c
@@ -116,8 +116,8 @@ EXPORT_SYMBOL(scsicam_bios_param);
  * @hds: put heads here
  * @secs: put sectors here
  *
- * Description: determine the BIOS mapping/geometry used to create the 
partition
- *  table, storing the results in *cyls, *hds, and *secs 
+ * Determine the BIOS mapping/geometry used to create the partition
+ * table, storing the results in @cyls, @hds, and @secs
  *
  * Returns: -1 on failure, 0 on success.
  */
-- 
2.9.3

[PATCH v2 46/53] ia64, scsi: update references for the device-io book

[Mauro Carvalho Chehab]

The book is now at Documentation/driver-api/device-io.rst.
Update such references.

Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---
 arch/ia64/include/asm/io.h | 2 +-
 arch/ia64/sn/kernel/iomv.c | 2 +-
 drivers/scsi/qla1280.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 5de673ac9cb1..a2540e21f919 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -117,7 +117,7 @@ extern int valid_mmap_phys_addr_range (unsigned long pfn, 
size_t count);
  * following the barrier will arrive after all previous writes.  For most
  * ia64 platforms, this is a simple 'mf.a' instruction.
  *
- * See Documentation/DocBook/deviceiobook.tmpl for more information.
+ * See Documentation/driver-api/device-io.rst for more information.
  */
 static inline void ___ia64_mmiowb(void)
 {
diff --git a/arch/ia64/sn/kernel/iomv.c b/arch/ia64/sn/kernel/iomv.c
index c77ebdf98119..2b22a71663c1 100644
--- a/arch/ia64/sn/kernel/iomv.c
+++ b/arch/ia64/sn/kernel/iomv.c
@@ -63,7 +63,7 @@ EXPORT_SYMBOL(sn_io_addr);
 /**
  * __sn_mmiowb - I/O space memory barrier
  *
- * See arch/ia64/include/asm/io.h and Documentation/DocBook/deviceiobook.tmpl
+ * See arch/ia64/include/asm/io.h and Documentation/driver-api/device-io.rst
  * for details.
  *
  * On SN2, we wait for the PIO_WRITE_STATUS SHub register to clear.
diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 634254a52301..8a29fb09db14 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -3390,7 +3390,7 @@ qla1280_isp_cmd(struct scsi_qla_host *ha)
 *On PCI bus, order reverses and write of 6 posts, then index 5,
 *   causing chip to issue full queue of stale commands
 * The mmiowb() prevents future writes from crossing the barrier.
-* See Documentation/DocBook/deviceiobook.tmpl for more information.
+* See Documentation/driver-api/device-io.rst for more information.
 */
WRT_REG_WORD(>mailbox4, ha->req_ring_index);
mmiowb();
-- 
2.9.3

[PATCH 06/13] ia64, scsi: update references for the device-io book

[Mauro Carvalho Chehab]

The book is now at Documentation/driver-api/device-io.rst.
Update such references.

Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---
 arch/ia64/include/asm/io.h | 2 +-
 arch/ia64/sn/kernel/iomv.c | 2 +-
 drivers/scsi/qla1280.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index 5de673ac9cb1..a2540e21f919 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -117,7 +117,7 @@ extern int valid_mmap_phys_addr_range (unsigned long pfn, 
size_t count);
  * following the barrier will arrive after all previous writes.  For most
  * ia64 platforms, this is a simple 'mf.a' instruction.
  *
- * See Documentation/DocBook/deviceiobook.tmpl for more information.
+ * See Documentation/driver-api/device-io.rst for more information.
  */
 static inline void ___ia64_mmiowb(void)
 {
diff --git a/arch/ia64/sn/kernel/iomv.c b/arch/ia64/sn/kernel/iomv.c
index c77ebdf98119..2b22a71663c1 100644
--- a/arch/ia64/sn/kernel/iomv.c
+++ b/arch/ia64/sn/kernel/iomv.c
@@ -63,7 +63,7 @@ EXPORT_SYMBOL(sn_io_addr);
 /**
  * __sn_mmiowb - I/O space memory barrier
  *
- * See arch/ia64/include/asm/io.h and Documentation/DocBook/deviceiobook.tmpl
+ * See arch/ia64/include/asm/io.h and Documentation/driver-api/device-io.rst
  * for details.
  *
  * On SN2, we wait for the PIO_WRITE_STATUS SHub register to clear.
diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 634254a52301..8a29fb09db14 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -3390,7 +3390,7 @@ qla1280_isp_cmd(struct scsi_qla_host *ha)
 *On PCI bus, order reverses and write of 6 posts, then index 5,
 *   causing chip to issue full queue of stale commands
 * The mmiowb() prevents future writes from crossing the barrier.
-* See Documentation/DocBook/deviceiobook.tmpl for more information.
+* See Documentation/driver-api/device-io.rst for more information.
 */
WRT_REG_WORD(>mailbox4, ha->req_ring_index);
mmiowb();
-- 
2.9.3

[PATCH 34/36] scsi: fix some kernel-doc markups

[Mauro Carvalho Chehab]

Sphinx is very pedantic with regards to ident/spacing.
Fix some kernel-doc markups in order to solve those
errors/warnings:

./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsicam.c:121: WARNING: Inline emphasis start-string without 
end-string.
./drivers/scsi/scsi_scan.c:1056: ERROR: Unexpected indentation.
./drivers/scsi/scsi_scan.c:1057: WARNING: Block quote ends without a blank 
line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2918: ERROR: Unexpected indentation.
./drivers/scsi/scsi_transport_fc.c:2921: WARNING: Block quote ends without a 
blank line; unexpected unindent.
./drivers/scsi/scsi_transport_fc.c:2922: WARNING: Enumerated list ends without 
a blank line; unexpected unindent.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---
 drivers/scsi/scsi_scan.c |  7 ---
 drivers/scsi/scsi_transport_fc.c | 18 ++
 drivers/scsi/scsicam.c   |  4 ++--
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6f7128f49c30..69979574004f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1051,10 +1051,11 @@ static unsigned char *scsi_inq_str(unsigned char *buf, 
unsigned char *inq,
  * allocate and set it up by calling scsi_add_lun.
  *
  * Return:
- * SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
- * SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
+ *
+ *   - SCSI_SCAN_NO_RESPONSE: could not allocate or setup a scsi_device
+ *   - SCSI_SCAN_TARGET_PRESENT: target responded, but no device is
  * attached at the LUN
- * SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
+ *   - SCSI_SCAN_LUN_PRESENT: a new scsi_device was allocated and initialized
  **/
 static int scsi_probe_and_add_lun(struct scsi_target *starget,
  u64 lun, int *bflagsp,
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index d4cf32d55546..1df77453f6b6 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2914,16 +2914,18 @@ EXPORT_SYMBOL(fc_remote_port_add);
  * port is no longer part of the topology. Note: Although a port
  * may no longer be part of the topology, it may persist in the remote
  * ports displayed by the fc_host. We do this under 2 conditions:
+ *
  * 1) If the port was a scsi target, we delay its deletion by "blocking" it.
- *   This allows the port to temporarily disappear, then reappear without
- *   disrupting the SCSI device tree attached to it. During the "blocked"
- *   period the port will still exist.
+ *This allows the port to temporarily disappear, then reappear without
+ *disrupting the SCSI device tree attached to it. During the "blocked"
+ *period the port will still exist.
+ *
  * 2) If the port was a scsi target and disappears for longer than we
- *   expect, we'll delete the port and the tear down the SCSI device tree
- *   attached to it. However, we want to semi-persist the target id assigned
- *   to that port if it eventually does exist. The port structure will
- *   remain (although with minimal information) so that the target id
- *   bindings remails.
+ *expect, we'll delete the port and the tear down the SCSI device tree
+ *attached to it. However, we want to semi-persist the target id assigned
+ *to that port if it eventually does exist. The port structure will
+ *remain (although with minimal information) so that the target id
+ *bindings remails.
  *
  * If the remote port is not an FCP Target, it will be fully torn down
  * and deallocated, including the fc_remote_port class device.
diff --git a/drivers/scsi/scsicam.c b/drivers/scsi/scsicam.c
index 910f4a7a3924..31273468589c 100644
--- a/drivers/scsi/scsicam.c
+++ b/drivers/scsi/scsicam.c
@@ -116,8 +116,8 @@ EXPORT_SYMBOL(scsicam_bios_param);
  * @hds: put heads here
  * @secs: put sectors here
  *
- * Description: determine the BIOS mapping/geometry used to create the 
partition
- *  table, storing the results in *cyls, *hds, and *secs 
+ * Determine the BIOS mapping/geometry used to create the partition
+ * table, storing the results in @cyls, @hds, and @secs
  *
  * Returns: -1 on failure, 0 on success.
  */
-- 
2.9.3

Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper

[Mauro Carvalho Chehab]

Hi Russell,

Em Mon, 30 Sep 2013 13:57:47 +0200
Hans Verkuil hverk...@xs4all.nl escreveu:

 On 09/19/2013 11:44 PM, Russell King wrote:
  Replace the following sequence:
  
  dma_set_mask(dev, mask);
  dma_set_coherent_mask(dev, mask);
  
  with a call to the new helper dma_set_mask_and_coherent().
  
  Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
 
 Acked-by: Hans Verkuil hans.verk...@cisco.com

Somehow, I lost your original post (I got unsubscribed on a few days 
from all vger mailing lists at the end of september).

I suspect that you want to sent this via your tree, right?
If so:

Acked-by: Mauro Carvalho Chehab m.che...@samsung.com

 
 Regards,
 
   Hans
 
  ---
   drivers/staging/media/dt3155v4l/dt3155v4l.c |5 +
   1 files changed, 1 insertions(+), 4 deletions(-)
  
  diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c 
  b/drivers/staging/media/dt3155v4l/dt3155v4l.c
  index 90d6ac4..081407b 100644
  --- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
  +++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
  @@ -901,10 +901,7 @@ dt3155_probe(struct pci_dev *pdev, const struct 
  pci_device_id *id)
  int err;
  struct dt3155_priv *pd;
   
  -   err = dma_set_mask(pdev-dev, DMA_BIT_MASK(32));
  -   if (err)
  -   return -ENODEV;
  -   err = dma_set_coherent_mask(pdev-dev, DMA_BIT_MASK(32));
  +   err = dma_set_mask_and_coherent(pdev-dev, DMA_BIT_MASK(32));
  if (err)
  return -ENODEV;
  pd = kzalloc(sizeof(*pd), GFP_KERNEL);
  
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/28] proc: Add proc_mkdir_data() [RFC]

[Mauro Carvalho Chehab]

Em 16-04-2013 15:26, David Howells escreveu:

Add proc_mkdir_data() to allow procfs directories to be created that are
annotated at the time of creation with private data rather than doing this
post-creation.  This means no access is then required to the proc_dir_entry
struct to set this.

Signed-off-by: David Howells dhowe...@redhat.com
cc: Neela Syam Kolli megaraidli...@lsi.com
cc: Jerry Chuang jerry-chu...@realtek.com
cc: Mauro Carvalho Chehab mche...@redhat.com
cc: linux-scsi@vger.kernel.org
cc: de...@driverdev.osuosl.org
cc: linux-wirel...@vger.kernel.org
---

  drivers/message/i2o/i2o_proc.c |8 ++--
  drivers/scsi/megaraid.c|4 ++--
  drivers/staging/rtl8192u/r8192U_core.c |3 +--


For the three patches that touch rtl8192u (patches 10, 12 and 14):

Acked-by: Mauro Carvalho Chehab mche...@redhat.com


  fs/proc/generic.c  |   30 --
  include/linux/proc_fs.h|   11 ---
  5 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/drivers/message/i2o/i2o_proc.c b/drivers/message/i2o/i2o_proc.c
index 70a840f..b7d87cd 100644
--- a/drivers/message/i2o/i2o_proc.c
+++ b/drivers/message/i2o/i2o_proc.c
@@ -1913,14 +1913,12 @@ static void i2o_proc_device_add(struct proc_dir_entry 
*dir,

osm_debug(adding device /proc/i2o/%s/%s\n, dev-iop-name, buff);

-   devdir = proc_mkdir(buff, dir);
+   devdir = proc_mkdir_data(buff, 0, dir, dev);
if (!devdir) {
osm_warn(Could not allocate procdir!\n);
return;
}

-   devdir-data = dev;
-
i2o_proc_create_entries(devdir, generic_dev_entries, dev);

/* Inform core that we want updates about this device's status */
@@ -1954,12 +1952,10 @@ static int i2o_proc_iop_add(struct proc_dir_entry *dir,

osm_debug(adding IOP /proc/i2o/%s\n, c-name);

-   iopdir = proc_mkdir(c-name, dir);
+   iopdir = proc_mkdir_data(c-name, 0, dir, c);
if (!iopdir)
return -1;

-   iopdir-data = c;
-
i2o_proc_create_entries(iopdir, i2o_proc_generic_iop_entries, c);

list_for_each_entry(dev, c-devices, list)
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index a1c90bd..ef3384d 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -2818,12 +2818,12 @@ mega_create_proc_entry(int index, struct proc_dir_entry 
*parent)

sprintf(string, hba%d, adapter-host-host_no);

-   dir = adapter-controller_proc_dir_entry = proc_mkdir(string, parent);
+   dir = adapter-controller_proc_dir_entry =
+   proc_mkdir_data(string, 0, parent, adapter);
if(!dir) {
printk(KERN_WARNING \nmegaraid: proc_mkdir failed\n);
return;
}
-   dir-data = adapter;

for (f = mega_proc_files; f-name; f++) {
de = proc_create_data(f-name, S_IRUSR, dir, mega_proc_fops,
diff --git a/drivers/staging/rtl8192u/r8192U_core.c 
b/drivers/staging/rtl8192u/r8192U_core.c
index 433c3df..d81d7d5 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -672,13 +672,12 @@ void rtl8192_proc_init_one(struct net_device *dev)
struct r8192_priv *priv = (struct r8192_priv *)ieee80211_priv(dev);

if (rtl8192_proc) {
-   priv-dir_dev = proc_mkdir(dev-name, rtl8192_proc);
+   priv-dir_dev = proc_mkdir_data(dev-name, 0, rtl8192_proc, 
dev);
if (!priv-dir_dev) {
RT_TRACE(COMP_ERR, Unable to initialize 
/proc/net/rtl8192/%s\n,
 dev-name);
return;
}
-   priv-dir_dev-data = dev;

for (f = rtl8192_proc_files; f-name[0]; f++) {
if (!proc_create_data(f-name, S_IFREG | S_IRUGO,
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 5f6f6c3..4074da5 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -428,13 +428,17 @@ struct proc_dir_entry *proc_symlink(const char *name,
  }
  EXPORT_SYMBOL(proc_symlink);

-struct proc_dir_entry *proc_mkdir_mode(const char *name, umode_t mode,
-   struct proc_dir_entry *parent)
+struct proc_dir_entry *proc_mkdir_data(const char *name, umode_t mode,
+   struct proc_dir_entry *parent, void *data)
  {
struct proc_dir_entry *ent;

+   if (mode == 0)
+   mode = S_IRUGO | S_IXUGO;
+
ent = __proc_create(parent, name, S_IFDIR | mode, 2);
if (ent) {
+   ent-data = data;
if (proc_register(parent, ent)  0) {
kfree(ent);
ent = NULL;
@@ -442,29 +446,19 @@ struct proc_dir_entry *proc_mkdir_mode(const char *name, 
umode_t mode,
}
return ent;
  }
-EXPORT_SYMBOL(proc_mkdir_mode);
+EXPORT_SYMBOL_GPL(proc_mkdir_data);

-struct proc_dir_entry *proc_net_mkdir(struct net *net

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Mon, 18 Feb 2013 23:44:05 +0100
Borislav Petkov b...@alien8.de escreveu:

 On Mon, Feb 18, 2013 at 02:26:18PM -0800, Greg KH wrote:
  I don't know, it depends on if userspace can handle this properly or
  not. What tools rely on this sysfs file? WHat happens when they get a
  non-number in the file?

The thing with sdram_scrub_rate is that this is not supported by any
userspace application I know. I suspect that this is used by userspace
scripts. So, we'll never know in advance what behavior those scripts would
expect.

 
 I'm not aware of any, frankly speaking.
 
 If there are any, those tools should be able to handle the -ENODEV
 they get. Now, if this gets changed this way, the read would succeed
 but they'll have to parse the returned value and see that it is not an
 integer.
 
 So I don't know either.
 
 But my gut feeling says to stay concervative and not touch this code -
 we don't know what uses it and how much we would break by fixing it.
 The current situation is not that big of a deal IMVHO and I'd be willing
 to accept the small inconcistency versus possibly breaking userspace.

I remember I saw some discussions about it in the past at bluesmoke ML,
saying that -ENODEV is the expected behavior when this is not supported.

Changing from -ENODEV to N/A will break anything that would be relying
on the previous behavior. So, I think that such change will for sure break
userspace.

If we're willing to change it, not creating the sdram_scrub_rate sysfs 
node is less likely to affect userspace.

Regards,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 12:11:21 +0200
Felipe Balbi ba...@ti.com escreveu:

 Hi,
 
 On Tue, Feb 19, 2013 at 07:03:10AM -0300, Mauro Carvalho Chehab wrote:
   But my gut feeling says to stay concervative and not touch this code -
   we don't know what uses it and how much we would break by fixing it.
   The current situation is not that big of a deal IMVHO and I'd be willing
   to accept the small inconcistency versus possibly breaking userspace.
  
  I remember I saw some discussions about it in the past at bluesmoke ML,
  saying that -ENODEV is the expected behavior when this is not supported.
  
  Changing from -ENODEV to N/A will break anything that would be relying
  on the previous behavior. So, I think that such change will for sure break
  userspace.
  
  If we're willing to change it, not creating the sdram_scrub_rate sysfs 
  node is less likely to affect userspace.
 
 yeah, I agree with this. Guess we shouldn't be creating files which
 aren't supported by the underlying HW and having a read() return -ENODEV
 is quite weird IMO since that's actually 'breaking' read() interface
 although that's up to interpretations.

The enclosed (untested) patch will likely do the trick. It needs to be
tested with one of the drivers that support scrub setting (amd64_edac.c,
cpc925_edac.c, e752x_edac.c, i5100_edac.c or i7core_edac.c).

Regards,
Mauro

-

[PATCH] edac: only create sdram_scrub_rate where supported

Currently, sdram_scrub_rate sysfs node is created even if the device
doesn't support get/set the scub rate. Change the logic to only
create this device node when the operation is supported.

Reported-by: Felipe Balbi ba...@ti.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 9c58da6..937975b 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -7,7 +7,7 @@
  *
  * Written Doug Thompson nor...@xmission.com www.softwarebitmaker.com
  *
- * (c) 2012 - Mauro Carvalho Chehab mche...@redhat.com
+ * (c) 2012-2013 - Mauro Carvalho Chehab mche...@redhat.com
  * The entire API were re-written, and ported to use struct device
  *
  */
@@ -681,9 +681,6 @@ static ssize_t mci_sdram_scrub_rate_store(struct device 
*dev,
unsigned long bandwidth = 0;
int new_bw = 0;
 
-   if (!mci-set_sdram_scrub_rate)
-   return -ENODEV;
-
if (strict_strtoul(data, 10, bandwidth)  0)
return -EINVAL;
 
@@ -707,9 +704,6 @@ static ssize_t mci_sdram_scrub_rate_show(struct device *dev,
struct mem_ctl_info *mci = to_mci(dev);
int bandwidth = 0;
 
-   if (!mci-get_sdram_scrub_rate)
-   return -ENODEV;
-
bandwidth = mci-get_sdram_scrub_rate(mci);
if (bandwidth  0) {
edac_printk(KERN_DEBUG, EDAC_MC, Error reading scrub rate\n);
@@ -882,7 +876,6 @@ static struct attribute *mci_attrs[] = {
dev_attr_ce_noinfo_count.attr,
dev_attr_ue_count.attr,
dev_attr_ce_count.attr,
-   dev_attr_sdram_scrub_rate.attr,
dev_attr_max_location.attr,
NULL
 };
@@ -1017,6 +1010,14 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info 
*mci)
return err;
}
 
+   if (mci-set_sdram_scrub_rate  mci-get_sdram_scrub_rate) {
+   err = device_create_file(mci-dev,
+dev_attr_sdram_scrub_rate);
+   if (err) {
+   edac_dbg(1, failure: create sdram_scrub_rate\n);
+   goto fail2;
+   }
+   }
/*
 * Create the dimm/rank devices
 */
@@ -1061,6 +1062,7 @@ fail:
continue;
device_unregister(dimm-dev);
}
+fail2:
device_unregister(mci-dev);
bus_unregister(mci-bus);
kfree(mci-bus.name);



signature.asc
Description: PGP signature

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 12:43:46 +0100
Borislav Petkov b...@alien8.de escreveu:

 On Tue, Feb 19, 2013 at 08:11:49AM -0300, Mauro Carvalho Chehab wrote:
I remember I saw some discussions about it in the past at bluesmoke ML,
saying that -ENODEV is the expected behavior when this is not supported.

Changing from -ENODEV to N/A will break anything that would be relying
on the previous behavior. So, I think that such change will for sure 
break
userspace.

If we're willing to change it, not creating the sdram_scrub_rate 
sysfs 
node is less likely to affect userspace.
 
 This will break scripts which assume this file's presence implicitly.

If are there any script like that, then yes.

 [ … ]
 
  @@ -1017,6 +1010,14 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info 
  *mci)
  return err;
  }
   
  +   if (mci-set_sdram_scrub_rate  mci-get_sdram_scrub_rate) {
 
 This will break cpc925_edac.c because it defines a
 -get_sdram_scrub_rate but not a -set_sdram_scrub_rate.

True. Patch below fixes it.

 I think a maybe better fix would be to figure out the sysfs file
 permissions based on the presence of the two functions and *then* add
 the attribute.
 
 This way, the only visible change to userspace is the corrected sysfs
 file permissions.

I'm not sure if is there a way to pass fs permissions to something similar
to device_create_file().

On both cases, an error will happen at open:
- if file doesn't exist (this approach), it will return -ENOENT;
- if file is opened with wrong permissions, open will return -EPERM.

However, if the file is not created, readdir() won't show the file.
So, if userspace first seeks for the sdram_scrub_rate, it won't fail.
That makes the logic below a little safer, IMO.


[PATCH EDAC] edac: only create sdram_scrub_rate where supported

Currently, sdram_scrub_rate sysfs node is created even if the device
doesn't support get/set the scub rate. Change the logic to only
create this device node when the operation is supported.

If only read or only write is supported, it will keep returning -ENODEV
just like before.

Reported-by: Felipe Balbi ba...@ti.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 9c58da6..29b66a2 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -7,7 +7,7 @@
  *
  * Written Doug Thompson nor...@xmission.com www.softwarebitmaker.com
  *
- * (c) 2012 - Mauro Carvalho Chehab mche...@redhat.com
+ * (c) 2012-2013 - Mauro Carvalho Chehab mche...@redhat.com
  * The entire API were re-written, and ported to use struct device
  *
  */
@@ -882,7 +882,6 @@ static struct attribute *mci_attrs[] = {
dev_attr_ce_noinfo_count.attr,
dev_attr_ue_count.attr,
dev_attr_ce_count.attr,
-   dev_attr_sdram_scrub_rate.attr,
dev_attr_max_location.attr,
NULL
 };
@@ -1017,6 +1016,14 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info 
*mci)
return err;
}
 
+   if (mci-set_sdram_scrub_rate || mci-get_sdram_scrub_rate) {
+   err = device_create_file(mci-dev,
+dev_attr_sdram_scrub_rate);
+   if (err) {
+   edac_dbg(1, failure: create sdram_scrub_rate\n);
+   goto fail2;
+   }
+   }
/*
 * Create the dimm/rank devices
 */
@@ -1061,6 +1068,7 @@ fail:
continue;
device_unregister(dimm-dev);
}
+fail2:
device_unregister(mci-dev);
bus_unregister(mci-bus);
kfree(mci-bus.name);
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 13:35:02 +0100
Borislav Petkov b...@alien8.de escreveu:

 On Tue, Feb 19, 2013 at 09:16:10AM -0300, Mauro Carvalho Chehab wrote:
  I'm not sure if is there a way to pass fs permissions to something similar
  to device_create_file().
 
 struct device_attribute.attr.mode? I.e., second arg.

Ah, now I see what you're meaning. That would require to dynamically
create a per-mci DEVICE_ATTR().

  On both cases, an error will happen at open:
  - if file doesn't exist (this approach), it will return -ENOENT;
  - if file is opened with wrong permissions, open will return -EPERM.
  
  However, if the file is not created, readdir() won't show the file.
 
 Right, and in that case userspace which *assumes* it is always created -
 like it is now - will fail when accessing it.
 
 If simply you adjust the attributes accordingly but *always* create the
 file and it has the correct permissions, everyone is happy. Right?

No, on both cases, open() will return an error (-ENOENT against -EPERM). 

If userspace doesn't check if open() failed, I can't see why
changing the open return error code would help.

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 14:06:26 +0100
Borislav Petkov b...@alien8.de escreveu:

  No, on both cases, open() will return an error (-ENOENT against -EPERM).
 
 What if it is a shell script doing:
 
 cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
 
 or similar?

Well, cat will return 1 if an error is found, no matter what error happened. 

With an existing file (-ENOSYS):

$ cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate; echo $?

cat: /sys/devices/system/edac/mc/mc0/sdram_scrub_rate: No such device
1

When the file doesn't exist (-ENOENT):
$ cat /sys/devices/system/edac/mc/mc0/sdram_scrub_rate_not_exist; echo 
$?
cat: /sys/devices/system/edac/mc/mc0/sdram_scrub_rate_not_exist: No 
such file or directory
1

When permission doesn't match (-EPERM):
$ cat /sys/devices/system/cpu/probe; echo $?
cat: /sys/devices/system/cpu/probe: Permission denied
1

When everything is ok, it will return 0
$ cat /sys/devices/system/edac/mc/mc0/ce_count; echo $? /dev/null
0

A script ready to handle -ENOSYS would be doing, instead:

RATE=$(cat /sys/devices/system/edac/mc/mc0/ce_count 2/dev/null)
if [ $? == 0 ]; then echo Scrub rate: $RATE; fi

So, a bash script won't notice any difference.

The only difference will be noticed if the script is written on some other 
language and the script is dumb enough to assume success if the errno is
different than -ENOSYS.

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 15:38:09 +0200
Felipe Balbi ba...@ti.com escreveu:

 Hi,
 
 On Tue, Feb 19, 2013 at 02:28:54PM +0100, Borislav Petkov wrote:
  On Tue, Feb 19, 2013 at 03:15:00PM +0200, Felipe Balbi wrote:
   what's the problem with that ?
  
  Not a problem - simply annoying.
  
  $ ./test.sh
  cat: /path/to/file/that/doesnt/exist.txt: No such file or directory
  Setting scrubrate to:
  
  I'm sure someone would ask themselves why all of a sudden the file is
  gone.
  
   $ cat /path/to/file/that/doesnt/exist.txt
   cat: /path/to/file/that/doesnt/exist.txt: No such file or directory
   
   Didn't see any gates to hell opening here...
  
  And I don't see why we have to debate such a trivial thing when we can
  fix it *properly* without *absolutely* *not* upsetting userspace.
 
 because changing the permission will cause the same issue:

It is actually worse, as if someone is using a C code to open the device
with
fp = open (/sys/devices/system/edac/mc/mc0/sdram_scrub_rate, O_RDWR);

It will now start to fail if the device doesn't have both permissions.

Regards,
Mauro


signature.asc
Description: PGP signature

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 14:58:07 +0100
Hannes Reinecke h...@suse.de escreveu:

 On 02/19/2013 02:50 PM, Borislav Petkov wrote:
  On Tue, Feb 19, 2013 at 03:38:09PM +0200, Felipe Balbi wrote:
  because changing the permission will cause the same issue:
 
  Actually, I take that back. Mauro's patch will already create the file
  anyway:
 
  if (mci-set_sdram_scrub_rate || mci-get_sdram_scrub_rate)
 
  Adjusting the permissions is simply the last missing piece to this patch
  to make the interface to userspace 100% coherent.
 
  --
  From: Mauro Carvalho Chehab mche...@redhat.com
  Date: Tue, 19 Feb 2013 09:16:10 -0300
  Subject: [PATCH] EDAC: only create sdram_scrub_rate where supported
 
  Currently, sdram_scrub_rate sysfs node is created even if the device
  doesn't support get/set the scub rate. Change the logic to only
  create this device node when the operation is supported.
 
  If only read or only write is supported, it will keep returning -ENODEV
  just like before.
 
  Reported-by: Felipe Balbi ba...@ti.com
  Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
  ---
drivers/edac/edac_mc_sysfs.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
  index 0ca1ca71157f..5a788e60ff67 100644
  --- a/drivers/edac/edac_mc_sysfs.c
  +++ b/drivers/edac/edac_mc_sysfs.c
  @@ -7,7 +7,7 @@
 *
 * Written Doug Thompson nor...@xmission.com www.softwarebitmaker.com
 *
  - * (c) 2012 - Mauro Carvalho Chehab mche...@redhat.com
  + * (c) 2012-2013 - Mauro Carvalho Chehab mche...@redhat.com
 *The entire API were re-written, and ported to use struct device
 *
 */
  @@ -878,7 +878,6 @@ static struct attribute *mci_attrs[] = {
  dev_attr_ce_noinfo_count.attr,
  dev_attr_ue_count.attr,
  dev_attr_ce_count.attr,
  -   dev_attr_sdram_scrub_rate.attr,
  dev_attr_max_location.attr,
  NULL
};
  @@ -1012,6 +1011,23 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info 
  *mci)
  return err;
  }
 
  +   if (mci-set_sdram_scrub_rate || mci-get_sdram_scrub_rate) {
  +   umode_t mode = 0;
  +
  +   if (mci-get_sdram_scrub_rate)
  +   mode = S_IRUGO;
  +
  +   if (mci-set_sdram_scrub_rate)
  +   mode |= S_IWUSR;
  +
  +   dev_attr_sdram_scrub_rate.attr.mode = mode;
  +
  +   err = device_create_file(mci-dev, dev_attr_sdram_scrub_rate);
  +   if (err) {
  +   edac_dbg(1, failure: create sdram_scrub_rate\n);
  +   goto fail2;
  +   }
  +   }
  /*
   * Create the dimm/rank devices
   */
  @@ -1056,6 +1072,7 @@ fail:
  continue;
  device_unregister(dimm-dev);
  }
  +fail2:
  device_unregister(mci-dev);
  bus_unregister(mci-bus);
  kfree(mci-bus.name);
 
 And of course you all know that creating sysfs attributes via 
 'device_create_file' opens all sort of funny race conditions, 
 especially when checking these attributes from udev ...

Yes, we know that, but this subsystem has already lots of other
attributes created via device_create_file(). It used to be a
lot worse than that, as, on a very recent past (before Kernel 3.5),
those attributes were created via sysfs_create_file().

There's not much that can be done to avoid it on this subsystem.

 
 Please consider adding a default attribute which return '-EINVAL' or 
 somesuch when the function pointers are not set.
 But _not_ adding it via device_create_file(). That's evil.

This thread started with Felipe's complain about it returning -ENOSYS ;)
when this feature is not supported.

Regards,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Tue, 19 Feb 2013 14:50:56 +0100
Borislav Petkov b...@alien8.de escreveu:

 On Tue, Feb 19, 2013 at 03:38:09PM +0200, Felipe Balbi wrote:
  because changing the permission will cause the same issue:
 
 Actually, I take that back. Mauro's patch will already create the file
 anyway:
 
   if (mci-set_sdram_scrub_rate || mci-get_sdram_scrub_rate)
 
 Adjusting the permissions is simply the last missing piece to this patch
 to make the interface to userspace 100% coherent.

That's covers everything but Hannes arguments. I don't think that
adding just one more device_create_file() on a driver that creates
dozens (or hundreds) of sys nodes, depending on the number of DIMMS
on the system would make it any worse.

Regards,
Mauro

[PATCH EDAC] edac: only create sdram_scrub_rate where supported

Currently, sdram_scrub_rate sysfs node is created even if the device
doesn't support get/set the scub rate. Change the logic to only
create this device node when the operation is supported.

Reported-by: Felipe Balbi ba...@ti.com
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 9c58da6..4f4b613 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -7,7 +7,7 @@
  *
  * Written Doug Thompson nor...@xmission.com www.softwarebitmaker.com
  *
- * (c) 2012 - Mauro Carvalho Chehab mche...@redhat.com
+ * (c) 2012-2013 - Mauro Carvalho Chehab mche...@redhat.com
  * The entire API were re-written, and ported to use struct device
  *
  */
@@ -681,9 +681,6 @@ static ssize_t mci_sdram_scrub_rate_store(struct device 
*dev,
unsigned long bandwidth = 0;
int new_bw = 0;
 
-   if (!mci-set_sdram_scrub_rate)
-   return -ENODEV;
-
if (strict_strtoul(data, 10, bandwidth)  0)
return -EINVAL;
 
@@ -707,9 +704,6 @@ static ssize_t mci_sdram_scrub_rate_show(struct device *dev,
struct mem_ctl_info *mci = to_mci(dev);
int bandwidth = 0;
 
-   if (!mci-get_sdram_scrub_rate)
-   return -ENODEV;
-
bandwidth = mci-get_sdram_scrub_rate(mci);
if (bandwidth  0) {
edac_printk(KERN_DEBUG, EDAC_MC, Error reading scrub rate\n);
@@ -870,8 +864,7 @@ DEVICE_ATTR(ce_count, S_IRUGO, mci_ce_count_show, NULL);
 DEVICE_ATTR(max_location, S_IRUGO, mci_max_location_show, NULL);
 
 /* memory scrubber attribute file */
-DEVICE_ATTR(sdram_scrub_rate, S_IRUGO | S_IWUSR, mci_sdram_scrub_rate_show,
-   mci_sdram_scrub_rate_store);
+DEVICE_ATTR(sdram_scrub_rate, 0, NULL, NULL);
 
 static struct attribute *mci_attrs[] = {
dev_attr_reset_counters.attr,
@@ -882,7 +875,6 @@ static struct attribute *mci_attrs[] = {
dev_attr_ce_noinfo_count.attr,
dev_attr_ue_count.attr,
dev_attr_ce_count.attr,
-   dev_attr_sdram_scrub_rate.attr,
dev_attr_max_location.attr,
NULL
 };
@@ -1017,6 +1009,22 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info 
*mci)
return err;
}
 
+   if (mci-set_sdram_scrub_rate || mci-get_sdram_scrub_rate) {
+   if (mci-get_sdram_scrub_rate) {
+   dev_attr_sdram_scrub_rate.attr.mode |= S_IRUGO;
+   dev_attr_sdram_scrub_rate.show = 
mci_sdram_scrub_rate_show;
+   }
+   if (mci-set_sdram_scrub_rate) {
+   dev_attr_sdram_scrub_rate.attr.mode |= S_IWUSR;
+   dev_attr_sdram_scrub_rate.store = 
mci_sdram_scrub_rate_store;
+   }
+   err = device_create_file(mci-dev,
+dev_attr_sdram_scrub_rate);
+   if (err) {
+   edac_dbg(1, failure: create sdram_scrub_rate\n);
+   goto fail2;
+   }
+   }
/*
 * Create the dimm/rank devices
 */
@@ -1061,6 +1069,7 @@ fail:
continue;
device_unregister(dimm-dev);
}
+fail2:
device_unregister(mci-dev);
bus_unregister(mci-bus);
kfree(mci-bus.name);

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Mon, 18 Feb 2013 20:46:33 +0200
Felipe Balbi ba...@ti.com escreveu:

 Hi, On Mon, Feb 18, 2013 at 09:49:16AM -0800, Greg KH wrote:
   Input/output error - /sys/devices/cpu/power/autosuspend_delay_ms
  
  The issue with this file is, if the power.use_autosuspend flag is not
  set for the device, then it can't be read or written to.  This flag
  changes dynamically with the system state
  (__pm_runtime_use_autosuspend() can change it), so we can't just not
  show the file if the flag is not set properly, sorry.
  
  So the error is correct here, as is the 0644 file value.
 
 hmm... we could create the file at pm_runtime_enable() time and remove
 it on pm_runtime_disable() time, no ? Addin Rafael to Cc

...


  
   No such device - /sys/devices/system/edac/mc/mc0/sdram_scrub_rate
  
  Odd, go ask the edac developers
 
 will do ;-)

Well, the question is missing ;) /me assumes that you want to talk about
suspending/resume and EDAC, right?

In general, memory controllers don't supports suspend, as far as I can tell. 
Still, I've seen a few ones that support, but the current drivers and/or the
EDAC core currently doesn't offer any support to it, as such setup is done
by the BIOS, when it detects the used DIMM banks.

I suspect that, when the OS puts the machine on a suspend state, the BIOS may
also suspend also the memory controller or put it into a low power consumption
mode, but it does it without any help from the EDAC drivers.

For most of what's there at EDAC, I don't think it is worth to add any PM 
support
inside it. There are, however, two cases were we may need to add some support:

1) if user changed the SDRAM scrub rate before suspending, it makes sense to
restore it after resume, on the memory controller drivers that support such
feature (not all supports it);

2) hot-pluggable DIMMs. EDAC currently doesn't support. This could be needed
on some future. In this case, the core may need to re-scan the memory 
controller,
changing the memory properties. I've no idea if is there any real case needing
it, nor what event would trigger the memory-controller re-scan. Resume is likely
one of the candidates for it, on machines that support hot-pluggable memories.

Regards,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SYSFS errors

[Mauro Carvalho Chehab]

Em Mon, 18 Feb 2013 22:05:42 +0200
Felipe Balbi ba...@ti.com escreveu:

 Hi,
 
 On Mon, Feb 18, 2013 at 04:46:38PM -0300, Mauro Carvalho Chehab wrote:
 No such device - /sys/devices/system/edac/mc/mc0/sdram_scrub_rate

Odd, go ask the edac developers
   
   will do ;-)
  
  Well, the question is missing ;) /me assumes that you want to talk about
  suspending/resume and EDAC, right?
 
 oops, my bad. The thing is that file has read permission but reading it
 isn't allowed ;-)

That depends on the driver. See drivers/edac/edac_mc_sysfs.c:

static ssize_t mci_sdram_scrub_rate_show(struct device *dev,
 struct device_attribute *mattr,
 char *data)
{
struct mem_ctl_info *mci = to_mci(dev);
int bandwidth = 0;

if (!mci-get_sdram_scrub_rate)
return -ENODEV;

bandwidth = mci-get_sdram_scrub_rate(mci);
if (bandwidth  0) {
edac_printk(KERN_DEBUG, EDAC_MC, Error reading scrub rate\n);
return bandwidth;
}

return sprintf(data, %d\n, bandwidth);
}

/* memory scrubber attribute file */
DEVICE_ATTR(sdram_scrub_rate, S_IRUGO | S_IWUSR, mci_sdram_scrub_rate_show,
mci_sdram_scrub_rate_store);

static struct attribute *mci_attrs[] = {
dev_attr_reset_counters.attr,
dev_attr_mc_name.attr,
dev_attr_size_mb.attr,
dev_attr_seconds_since_reset.attr,
dev_attr_ue_noinfo_count.attr,
dev_attr_ce_noinfo_count.attr,
dev_attr_ue_count.attr,
dev_attr_ce_count.attr,
dev_attr_sdram_scrub_rate.attr,
dev_attr_max_location.attr,
NULL
};

The capability of get and/or set the scrub rate is edac-driver specific.
Drivers that support it need to fill those callbacks:
mci-get_sdram_scrub_rate
mci-set_sdram_scrub_rate

In any case, the sysfs attribute is created even if the device doesn't
have support for read/set the scrub rate.

On devices where this is not supported, reading (and/or writing) to this
sysfs node will return -ENODEV.

In the past, the sysfs node creation was done using the raw sysfs support.
Doing dynamic creation with the old code were much more complex. I guess
that's the reason why the code was written this way. Now that the code 
uses struct device, it shouldn't be hard to change the code to only 
create this attribute if the device has support for scrub rate setting.

Yet, that would change the userspace API. I'm not sure what
applications would rely on the current behavior. So, changing it
would require some investigation in order to avoid regressions.

Regards,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html