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

[Jagan Teki]

On Sat, Nov 5, 2016 at 9:37 PM, Simon Glass  wrote:
> Hi Jagan,
>
> On 30 October 2016 at 11:46, Jagan Teki  wrote:
>> - Proper tabs spaces
>> - Removed unnecessary
>> - Added meaningful comments
>>
>> Cc: Simon Glass 
>> Cc: Bin Meng 
>> Cc: York Sun 
>> Cc: Vignesh R 
>> Cc: Mugunthan V N 
>> Cc: Michal Simek 
>> Cc: Siva Durga Prasad Paladugu 
>> Signed-off-by: Jagan Teki 
>> ---
>>  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!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Jagan,

On 30 October 2016 at 11:46, Jagan Teki  wrote:
> - Proper tabs spaces
> - Removed unnecessary
> - Added meaningful comments
>
> Cc: Simon Glass 
> Cc: Bin Meng 
> Cc: York Sun 
> Cc: Vignesh R 
> Cc: Mugunthan V N 
> Cc: Michal Simek 
> Cc: Siva Durga Prasad Paladugu 
> Signed-off-by: Jagan Teki 
> ---
>  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_4KBIT(0)
>  #define E_FSR  BIT(1)
>  #define SST_WR BIT(2)
> --
> 2.7.4
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot