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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Nov 2016 17:42:55 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Marek Vasut <marek.vasut@...il.com>
Cc:     linux-mtd@...ts.infradead.org, Alan Cox <alan@...ux.intel.com>,
        David Woodhouse <David.Woodhouse@...el.com>,
        Jason Roberts <jason.e.roberts@...el.com>,
        Chuanxiao Dong <chuanxiao.dong@...el.com>,
        Dinh Nguyen <dinguyen@...era.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Brian Norris <computersforpeace@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>
Subject: Re: [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@...il.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@...ionext.com>
>
> Reviewed-by: Marek Vasut <marek.vasut@...il.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