Re: [PATCH 05/11] mtd: nand: denali: remove "Spectra:" prefix from printk strings

[Masahiro Yamada]

Hi Marek,


2016-11-13 6:35 GMT+09:00 Marek Vasut <marek.vasut@xxxxxxxxx>:
> On 11/09/2016 05:35 AM, Masahiro Yamada wrote:
>> As far as I understood from the Kconfig menu deleted by commit
>> be7f39c5ecf5 ("Staging: delete spectra driver"), the "Spectra" is
>> specific to Intel Moorestown Platform.
>>
>> The Denali NAND controller IP is used for various SoCs such as
>> Altera SOCFPGA, Socionext UniPhier, etc.  The platform specific
>> strings are not preferred in this driver.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>
> Reviewed-by: Marek Vasut <marek.vasut@xxxxxxxxx>
>
>> ---
>>
>> As an ARM-SoC developer, I only need denali.c and denali_dt.c.
>>
>> I see some "Spectra:" in drivers/mtd/nand/denali_pci.c as well.
>> I was not quite sure if they are needed or not.
>> If desired, I can update this patch to remove them too.
>
> Is anyone even using Denali on Intel now ?
>
>>  drivers/mtd/nand/denali.c | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
>> index 80d3e26..78d795b 100644
>> --- a/drivers/mtd/nand/denali.c
>> +++ b/drivers/mtd/nand/denali.c
>> @@ -402,7 +402,7 @@ static void get_hynix_nand_para(struct denali_nand_info *denali,
>>               break;
>>       default:
>>               dev_warn(denali->dev,
>> -                      "Spectra: Unknown Hynix NAND (Device ID: 0x%x).\n"
>> +                      "Unknown Hynix NAND (Device ID: 0x%x).\n"
>>                        "Will use default parameter values instead.\n",
>>                        device_id);
>>       }
>> @@ -1458,7 +1458,7 @@ int denali_init(struct denali_nand_info *denali)
>>        */
>>       if (request_irq(denali->irq, denali_isr, IRQF_SHARED,
>>                       DENALI_NAND_NAME, denali)) {
>> -             pr_err("Spectra: Unable to allocate IRQ\n");
>> +             dev_err(denali->dev, "Unable to request IRQ\n");
>>               return -ENODEV;
>>       }
>>
>> @@ -1495,7 +1495,7 @@ int denali_init(struct denali_nand_info *denali)
>>       /* Is 32-bit DMA supported? */
>>       ret = dma_set_mask(denali->dev, DMA_BIT_MASK(32));
>>       if (ret) {
>> -             pr_err("Spectra: no usable DMA configuration\n");
>> +             dev_err(denali->dev, "no usable DMA configuration\n");
>>               goto failed_req_irq;
>>       }
>>
>> @@ -1503,7 +1503,7 @@ int denali_init(struct denali_nand_info *denali)
>>                            mtd->writesize + mtd->oobsize,
>>                            DMA_BIDIRECTIONAL);
>>       if (dma_mapping_error(denali->dev, denali->buf.dma_buf)) {
>> -             dev_err(denali->dev, "Spectra: failed to map DMA buffer\n");
>> +             dev_err(denali->dev, "failed to map DMA buffer\n");
>
> Nit: For consistency's sake, use Failed with capital F . Fix the "No
> usable DMA ..." too.


Even if we fix those two, we still have some more printk strings
that start with a lower case.


line 177:  dev_err(denali->dev, "reset bank failed.\n");

line 699:  pr_err("timeout occurred, status = 0x%x, mask = 0x%x\n",

line 863:  dev_err(denali->dev, "unable to send pipeline command\n");

line 1074:  dev_err(denali->dev, "timeout on write_page (type = %d)\n",

line 1309:  pr_err(": unsupported command received 0x%x\n", cmd);



If you say "consistency's sake" and
you are a big fan of capital letters instead of lower cases,
will you send a patch that touches those globally?


Your comments against this series are just about
"upper cases vs lower cases".

If I get more useful comments, I am happy to send v2.

But, at this moment, I see no good reason for v2
because changing those two lines does not give us any consistency.


-- 
Best Regards
Masahiro Yamada