From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965340AbdCVR70 (ORCPT ); Wed, 22 Mar 2017 13:59:26 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:32882 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965203AbdCVR7R (ORCPT ); Wed, 22 Mar 2017 13:59:17 -0400 X-AuditID: b6c32a2d-f793d6d0000012b6-39-58d2bb716229 From: Bartlomiej Zolnierkiewicz To: Sergei Shtylyov Cc: Tejun Heo , Sekhar Nori , Kevin Hilman , Arnd Bergmann , Russell King , Dmitry Eremin-Solenikov , linux-ide@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] ata: add Palmchip BK3710 PATA controller driver Date: Wed, 22 Mar 2017 18:59:10 +0100 Message-id: <4365895.k4zyj0CytI@amdc3058> User-Agent: KMail/4.13.3 (Linux/3.13.0-96-generic; KDE/4.13.3; x86_64; ; ) In-reply-to: MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=us-ascii X-Brightmail-Tracker: H4sIAAAAAAAAA02Sa0hTYRzGec/Zdo6ryXGa/VFSmBQ51CwLDhWSJnHQpDCC5Qdr5EHFeekc FfVDWWnLW0xFjOEH8dJlmdUczQvO4WoKKpiGecFLJUJpCqWVTi23o+C33/s+z//yvLwkLn8j 9iFT0rNYLl2tUUikore2QGXwrc5hVei3JTG9WWkn6Mq5ZZxe05ow2vh1VEzb275g9EhHrYSe HJHR3eWvMXrAMEHQ60/fo3NSpvB+mYRxrFciZnmsiGBmq/9hTLt+imCMhmIJ09p4h+kdM2PM L6PfZbd46dlEVpOSw3LHwm9Ik+uLuEydT+6qrQEVoA3PEuRGAnUSauY7MIG9YWj6laQESUk5 1YRgoXgWdwpySovB+ErCbsFzi10kmGoRtFubCcG0iqCpi3GyhDoNFVoDcrIXRcPU3XpXAU51 YdBjWRQ7BU8qBtbsha5iEXUYTLZCkZNlVCD8WDG7+AAVDSaL1rWeG3UB2t4tEoLHA/5WTbs8 OOUPlu5qscBBMGhvQc5hQI0S0DhQtx2B3D4cAqMVFxJEgd3xghDYE773mnbYFzabJpDANQjM DhD6tCIwdOp2TGfA1vthZ5g7lDvmMKG/DB4+kAvIQFFXuIARMFjpJbxVDQZ9H9cIHfLX70mg 35NAvydBHcINyJvN5NOSWP5U5okQXp3GZ6cnhdzMSDMi10dSBrehlbqYHkSRSLFfNvtsWCUX q3P4vLQeBCSu8JJtmLevZInqvHyWy7jOZWtYvgf5kiLFQVl8aIVKTiWps9hUls1kuV0VI918 CpDfE0Nz7FiYZmtB8WTGO8X96MLnrNUlzXlj8u0W3e8+3jw4UJYQZS3x1mX/LF2NaPLQWxq4 a9aR8fw8//6W4iuxF1seRVq51Lj4gJft0dqIrf64TXNk9Z/cgMfrQetLc5/CjtybJAqMQx3z qsKq8dKZAYXnpX1JCVeXJMrgsplghYhPVh9X4hyv/g+y0/K2RAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrOIsWRmVeSWpSXmKPExsVy+t9jQd2C3ZciDGZOMrL4O+kYu8WkJ++Z LX62b2Gy2PT4GqvFsR2PmCwu75rDZnH7Mq/F/t4NTBZnVt1it/i1/CijA5dHS3MPm8fvX5MY Pd7faGX3eDD1P5PHzll32T02repk89i8pN7j+I3tTB6fN8kFcEa52WSkJqakFimk5iXnp2Tm pdsqhYa46VooKeQl5qbaKkXo+oYEKSmUJeaUAnlGBmjAwTnAPVhJ3y7BLWNRa1HBBKmKr4cX MzYw/hHuYuTkkBAwkVi57xgLhC0mceHeerYuRi4OIYFZjBLL+9ayQzhfGSXOrp/GDlLFJmAl MbF9FSOILSJgIXG3cRELSBGzwF4miUkL5zOBJIQFvCV+HmsBa2ARUJXYcrgFbAWvgKbE2y/b wWxRAS+JLfvaweo5Bdwkdhx5A7XtMKPEzZYv7BANghI/Jt8Da2AWkJfYt38qK4StJbF+53Gm CYxAhyKUzUJSNgtJ2QJG5lWMEqkFyQXFSem5Rnmp5XrFibnFpXnpesn5uZsYwZH6THoH4+Fd 7ocYBTgYlXh4I2ouRQixJpYVV+YeYpTgYFYS4f2zHSjEm5JYWZValB9fVJqTWnyI0RTow4nM UqLJ+cAkklcSb2hibmJubGBhbmlpYqQkzts4+1m4kEB6YklqdmpqQWoRTB8TB6dUA2Oq6pUD N4v9XhxL4XFYq/W2TpCrYHXTin42/6RZh2qWfJ84bfffvJmLxI7+q7FQ6Z/37X1E+4GXIp+q bSPKXr366LiqN/uR6akC+YfZmjvn8MxYeKJN6WlJz0272Allsk6aRfN3S34vee2zqL9M+Ne5 3qkFXEuvLXjYu5ph8rY3Mmm7PO4/mbVciaU4I9FQi7moOBEAr1ykoeoCAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170322175912epcas5p2dc82f4879de4c440523e7d31c9bf385b X-Msg-Generator: CA X-Sender-IP: 203.254.230.27 X-Local-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRvsgrzshLHsoITsnpAbU2VuaW9yIFNvZnR3YXJlIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRtTYW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBF?= =?UTF-8?B?bmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 105P X-HopCount: 7 X-CMS-RootMailID: 20170322175912epcas5p2dc82f4879de4c440523e7d31c9bf385b X-RootMTR: 20170322175912epcas5p2dc82f4879de4c440523e7d31c9bf385b References: <1489509414-11491-1-git-send-email-b.zolnierkie@samsung.com> <1489509414-11491-2-git-send-email-b.zolnierkie@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Saturday, March 18, 2017 04:52:18 PM Sergei Shtylyov wrote: > Hello! > > On 3/14/2017 7:36 PM, Bartlomiej Zolnierkiewicz wrote: > > > Add Palmchip BK3710 PATA controller driver. > > > > Signed-off-by: Bartlomiej Zolnierkiewicz > [...] > > diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c > > new file mode 100644 > > index 0000000..6d77217 > > --- /dev/null > > +++ b/drivers/ata/pata_bk3710.c > > @@ -0,0 +1,386 @@ > [...] > > +static void pata_bk3710_chipinit(void __iomem *base) > > +{ > [...] > > + /* > > + * IORDYTMP IORDY Timer for Primary Register > > + * (ATA_IORDYTMP_IORDYTMP , 0xffff ) > > + */ > > + iowrite32(0xFFFF, base + BK3710_IORDYTMP); > > As I've already said, this is useless as we don't handle the IORDY timeout > interrupt anyway; writing 0 would be fine. Will fix in v3, in the incremental patch (so it is easier to revert if it turns out to cause problems later or port to palm_bk3710). > > + > > + /* > > + * Configure BMISP Register > > + * (ATA_BMISP_DMAEN1 , DISABLE ) | > > + * (ATA_BMISP_DMAEN0 , DISABLE ) | > > + * (ATA_BMISP_IORDYINT , CLEAR) | > > + * (ATA_BMISP_INTRSTAT , CLEAR) | > > + * (ATA_BMISP_DMAERROR , CLEAR) > > + */ > > + iowrite16(0, base + BK3710_BMISP); > > Bits 0-3 cane only be cleared by writing 1, so this write can't clear The documentation does say this about bits 1-3, bit 0 is handled in a different way. > them, contrary to what the comment says. Might be a material for a follow-up > patch tho... Will fix in the incremental patch in v3. > [...] > > +static int __init pata_bk3710_probe(struct platform_device *pdev) > > +{ > > + struct clk *clk; > > + struct resource *mem; > > + struct ata_host *host; > > + struct ata_port *ap; > > + void __iomem *base; > > + unsigned long rate; > > + int irq; > > + > > + clk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(clk)) > > + return -ENODEV; > > + > > + clk_enable(clk); > > + rate = clk_get_rate(clk); > > + if (!rate) > > + return -EINVAL; > > + > > + /* NOTE: round *down* to meet minimum timings; we count in clocks */ > > + ideclk_period = 1000000000UL / rate; > > + > > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (mem == NULL) { > > + pr_err(DRV_NAME ": failed to get memory region resource\n"); > > + return -ENODEV; > > + } > > NULL check not needed here, devm_ioremap_resource() checks this anyway. Will be fixed in v3. > > + > > + irq = platform_get_irq(pdev, 0); > > + if (irq < 0) { > > + pr_err(DRV_NAME ": failed to get IRQ resource\n"); > > + return irq; > > + } > > + > > + base = devm_ioremap_resource(&pdev->dev, mem); > > + if (IS_ERR(base)) > > + return PTR_ERR(base); > > + > [...] > > +/* work with hotplug and coldplug */ > > +MODULE_ALIAS("platform:palm_bk3710"); > > + > > +static struct platform_driver pata_bk3710_driver = { > > + .driver = { > > + .name = "palm_bk3710", > > Not DRV_NAME? DRV_NAME is "pata_bk3710" and the platform driver name needs to match the old driver name for compatibility reasons (supporting both drivers by the arch specific code). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics