On 11/18/2016 09:42 AM, Masahiro Yamada wrote:
> Hi Marek,
>
>
> 2016-11-13 6:35 GMT+09:00 Marek Vasut <marek.va...@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.masah...@socionext.com>
>>
>> Reviewed-by: Marek Vasut <marek.va...@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?
It's not really _that_ important.
> 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.
Well we have to start somewhere, but I can fix those two when applying.
--
Best regards,
Marek Vasut
