lkml.org 
[lkml]   [2016]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 05/11] mtd: nand: denali: remove "Spectra:" prefix from printk strings
Hi Marek,


2016-11-13 6:35 GMT+09:00 Marek Vasut <marek.vasut@gmail.com>:
> 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@socionext.com>
>
> Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
>
>> ---
>>
>> 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

\
 
 \ /
  Last update: 2016-11-18 09:43    [W:0.069 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site