diff mbox

[U-Boot,v5,04/21] sf: Cleanup spi_flash_info{}

Message ID 1477849590-11954-5-git-send-email-jagan@openedev.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Jagan Teki Oct. 30, 2016, 5:46 p.m. UTC
- Proper tabs spaces
- Removed unnecessary
- Added meaningful comments 

Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: York Sun <york.sun@nxp.com>
Cc: Vignesh R <vigneshr@ti.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
Signed-off-by: Jagan Teki <jagan@openedev.com>
---
 drivers/mtd/spi/sf_internal.h | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Comments

Simon Glass Nov. 5, 2016, 4:07 p.m. UTC | #1
Hi Jagan,

On 30 October 2016 at 11:46, Jagan Teki <jagan@openedev.com> wrote:
> - Proper tabs spaces
> - Removed unnecessary
> - Added meaningful comments
>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: York Sun <york.sun@nxp.com>
> Cc: Vignesh R <vigneshr@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Signed-off-by: Jagan Teki <jagan@openedev.com>
> ---
>  drivers/mtd/spi/sf_internal.h | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index a9455ac..71feba9 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -108,17 +108,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
>  #define JEDEC_ID(info)         (((info)->id[1]) << 8 | ((info)->id[2]))
>  #define JEDEC_EXT(info)                (((info)->id[3]) << 8 | ((info)->id[4]))
>
> -/**
> - * struct spi_flash_info - SPI/QSPI flash device params structure
> - *
> - * @name:              Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO])
> - * @sector_size:       Isn't necessarily a sector size from vendor,
> - *                     the size listed here is what works with CMD_ERASE_64K
> - * @nr_sectors:                No.of sectors on this device
> - * @flags:             Important param, for flash specific behaviour

Why remove these comments? For 'flags' it would be good to point to
the enum / #define that describes its values.

> - */
>  struct spi_flash_info {
> -       const char *name;
> +       const char      *name;
>
>         /*
>          * This array stores the ID bytes.
> @@ -128,12 +119,15 @@ struct spi_flash_info {
>         u8              id[5];
>         u8              id_len;
>
> -       u32 sector_size;
> -       u32 nr_sectors;
> +       /* The size listed here is what works with SPINOR_OP_SE, which isn't

/*
 * The size

> +        * necessarily called a "sector" by the vendor.
> +        */
> +       u32             sector_size;
> +       u32             nr_sectors;
>
> -       u16 page_size;
> +       u16             page_size;
>
> -       u16 flags;
> +       u16             flags;
>  #define SECT_4K                        BIT(0)
>  #define E_FSR                  BIT(1)
>  #define SST_WR                 BIT(2)
> --
> 2.7.4
>

Regards,
Simon
Jagan Teki Nov. 15, 2016, 7:45 a.m. UTC | #2
On Sat, Nov 5, 2016 at 9:37 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 30 October 2016 at 11:46, Jagan Teki <jagan@openedev.com> wrote:
>> - Proper tabs spaces
>> - Removed unnecessary
>> - Added meaningful comments
>>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: York Sun <york.sun@nxp.com>
>> Cc: Vignesh R <vigneshr@ti.com>
>> Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Signed-off-by: Jagan Teki <jagan@openedev.com>
>> ---
>>  drivers/mtd/spi/sf_internal.h | 22 ++++++++--------------
>>  1 file changed, 8 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index a9455ac..71feba9 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -108,17 +108,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
>>  #define JEDEC_ID(info)         (((info)->id[1]) << 8 | ((info)->id[2]))
>>  #define JEDEC_EXT(info)                (((info)->id[3]) << 8 | ((info)->id[4]))
>>
>> -/**
>> - * struct spi_flash_info - SPI/QSPI flash device params structure
>> - *
>> - * @name:              Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO])
>> - * @sector_size:       Isn't necessarily a sector size from vendor,
>> - *                     the size listed here is what works with CMD_ERASE_64K
>> - * @nr_sectors:                No.of sectors on this device
>> - * @flags:             Important param, for flash specific behaviour
>
> Why remove these comments? For 'flags' it would be good to point to
> the enum / #define that describes its values.

Added comments inside the structure in-place where members defined and
will update comment for name.
For flags will add the comments for respective macros in future patch.

>
>> - */
>>  struct spi_flash_info {
>> -       const char *name;
>> +       const char      *name;
>>
>>         /*
>>          * This array stores the ID bytes.
>> @@ -128,12 +119,15 @@ struct spi_flash_info {
>>         u8              id[5];
>>         u8              id_len;
>>
>> -       u32 sector_size;
>> -       u32 nr_sectors;
>> +       /* The size listed here is what works with SPINOR_OP_SE, which isn't
>
> /*
>  * The size

Will fix this.

thanks!
diff mbox

Patch

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index a9455ac..71feba9 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -108,17 +108,8 @@  int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
 #define JEDEC_ID(info)		(((info)->id[1]) << 8 | ((info)->id[2]))
 #define JEDEC_EXT(info)		(((info)->id[3]) << 8 | ((info)->id[4]))
 
-/**
- * struct spi_flash_info - SPI/QSPI flash device params structure
- *
- * @name:		Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO])
- * @sector_size:	Isn't necessarily a sector size from vendor,
- *			the size listed here is what works with CMD_ERASE_64K
- * @nr_sectors:		No.of sectors on this device
- * @flags:		Important param, for flash specific behaviour
- */
 struct spi_flash_info {
-	const char *name;
+	const char	*name;
 
 	/*
 	 * This array stores the ID bytes.
@@ -128,12 +119,15 @@  struct spi_flash_info {
 	u8		id[5];
 	u8		id_len;
 
-	u32 sector_size;
-	u32 nr_sectors;
+	/* The size listed here is what works with SPINOR_OP_SE, which isn't
+	 * necessarily called a "sector" by the vendor.
+	 */
+	u32		sector_size;
+	u32		nr_sectors;
 
-	u16 page_size;
+	u16		page_size;
 
-	u16 flags;
+	u16		flags;
 #define SECT_4K			BIT(0)
 #define E_FSR			BIT(1)
 #define SST_WR			BIT(2)