Sergei Shtylyov wrote:
>
> Hello.
Hi ;-)
>
> Kukjin Kim wrote:
>
> > From: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
>
> > Adds support for the Samsung PATA controller. This driver is based on
the
> > Libata subsystem and references the earlier patches sent for IDE
subsystem.
>
> > Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
> > Signed-off-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
> > Cc: Ben Dooks <ben-linux@xxxxxxxxx>
>
> There's probably still a place for improvement... sorry that I dind't
> notice before. Other than that:
>
Ok..will address comments from you and re-submit.
> Acked-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxx>
>
Will be added above your ack in updated patch.
If any problem, please let me know.
> [...]
>
> > diff --git a/drivers/ata/pata_samsung_cf.c
b/drivers/ata/pata_samsung_cf.c
> > new file mode 100644
> > index 0000000..5718220
> > --- /dev/null
> > +++ b/drivers/ata/pata_samsung_cf.c
> > @@ -0,0 +1,727 @@
> [...]
> > +/*
> > + * pata_s3c_wait_after_reset - wait for devices to become ready after
reset
> > + */
> > +static int pata_s3c_wait_after_reset(struct ata_link *link,
> > + unsigned int devmask, unsigned long deadline)
> > +{
> > + struct ata_port *ap = link->ap;
> > + struct ata_ioports *ioaddr = &ap->ioaddr;
> > + unsigned int dev0 = devmask & (1 << 0);
> > + unsigned int dev1 = devmask & (1 << 1);
> > + int rc, ret = 0;
> > +
> > + msleep(ATA_WAIT_AFTER_RESET);
> > +
> > + /* always check readiness of the master device */
> > + rc = ata_sff_wait_ready(link, deadline);
> > + /* -ENODEV means the odd clown forgot the D7 pulldown resistor
> > + * and TF status is 0xff, bail out on it too.
> > + */
> > + if (rc)
> > + return rc;
> > +
> > + /* if device 1 was found in ata_devchk, wait for register
> > + * access briefly, then wait for BSY to clear.
> > + */
>
> You don't call pata_s3c_devchk() for device 1, so in principle you can
> drop the following block:
>
> > + if (dev1) {
> > + int i;
> > +
> > + pata_s3c_dev_select(ap, 1);
> > +
> > + /* Wait for register access. Some ATAPI devices fail
> > + * to set nsect/lbal after reset, so don't waste too
> > + * much time on it. We're gonna wait for !BSY anyway.
> > + */
> > + for (i = 0; i < 2; i++) {
> > + u8 nsect, lbal;
> > +
> > + nsect = ata_inb(ap->host, ioaddr->nsect_addr);
> > + lbal = ata_inb(ap->host, ioaddr->lbal_addr);
> > + if ((nsect == 1) && (lbal == 1))
> > + break;
> > + msleep(50); /* give drive a breather */
> > + }
> > +
> > + rc = ata_sff_wait_ready(link, deadline);
> > + if (rc) {
> > + if (rc != -ENODEV)
> > + return rc;
> > + ret = rc;
> > + }
> > + }
> > +
> > + /* is all this really necessary? */
> > + pata_s3c_dev_select(ap, 0);
> > + if (dev1)
> > + pata_s3c_dev_select(ap, 1);
>
> This too...
>
> > + if (dev0)
> > + pata_s3c_dev_select(ap, 0);
> > +
> > + return ret;
> > +}
> > +
> [...]
> > +static int pata_s3c_resume(struct device *dev)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct ata_host *host = platform_get_drvdata(pdev);
> > + struct s3c_ide_platdata *pdata = pdev->dev.platform_data;
> > + struct s3c_ide_info *info;
> > +
> > + info = host->private_data;
>
> Could be initializer...
>
> MBR, Sergei
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
