public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: sshtylyov@mvista•com (Sergei Shtylyov)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 4/4] pata_samsung: Add Samsung PATA controller driver
Date: Wed, 02 Jun 2010 13:51:51 +0400	[thread overview]
Message-ID: <4C0629B7.4060300@ru.mvista.com> (raw)
In-Reply-To: <002a01cb01fd$4cf10a50$e6d31ef0$%kim@samsung.com>

Hello.

Kukjin Kim wrote:

>>> From: Abhilash Kesavan <a.kesavan@samsung•com>
>>> Adds support for the Samsung PATA controller. This driver is based on the
>>> Libata subsystem and references the earlier patches sent for IDE subsystem.

    Where I those I wonder? :-)
    It's a pity they didn't get accepted.

>>> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung•com>
>>> Signed-off-by: Kukjin Kim <kgene.kim@samsung•com>

[...]

>>> +		for (i = 0; i < words; i++, temp_addr++) {
>>> +			wait_for_host_ready(info);
>>> +			writel(*temp_addr, data_addr);
>>> +		}
>>> +	}

>>     Well, if this is pere CF case, 'buflen' can't be odd, but otherwise

    I meant to type "pure". :-)

>> you should handle that case...

>>> +	uint pio_teoc[5] = { 240, 43, 10, 70, 25 };

>>     What timing is this? :-O

> Should have been t2i rec.

    If it's t2i timing, it is incorrect for the first 3 PIO modes.

>>> +	if (cpu_type == TYPE_S3C6400) {
>>> +		ap->ops = &pata_s3c_port_ops;
>>> +		info->sfr_addr = info->ide_addr + 0x1800;
>>> +		info->ide_addr = info->ide_addr + 0x1900;
>>> +		info->fifo_status_reg = 0x94;
>>> +	} else if (cpu_type == TYPE_S5PC100) {
>>> +		ap->ops = &pata_s5p_port_ops;
>>> +		info->sfr_addr = info->ide_addr + 0x1800;
>>> +		info->ide_addr = info->ide_addr + 0x1900;

>>     Does make sense to assign those before *if*.

> Offsets not required for S5PV210/S5PC110.

    pata_s3c_tune_chipset() certainly requires them.

>>> +		info->fifo_status_reg = 0x84;
>>> +	} else {
>>> +		ap->ops = &pata_s5p_port_ops;

>>     You don't assign 'info->ide_addr' here but you'll need it in
>> pata_s3c_tune_chipset() which will be called thru pata_s5p_port_ops!

> The address received at ioremap() is enough for S5PV210/S5PC110..no offset
> needed.

    I only have to restate that pata_s3c_tune_chipset() does use 
'info->ide_addr'. Maybe you shouldn't install this method for S5PV210/S5PC110? 
Or do you mean thgat offset of 0 will work?

>>> +release_mem:
>>> +	release_mem_region(res->start, res->end - res->start + 1);

>>     But you didn't call request_mem_region()!

> I didn't..will remove.

    But you should call request_mem_region() in one or another form...

MBR, Sergei

  reply	other threads:[~2010-06-02  9:51 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-27  8:22 [PATCH] Add Compact Flash driver for Samsung SoCs Kukjin Kim
2010-05-27  8:22 ` [PATCH 1/4] ARM: S3C64XX: Add support for Compact Flash driver on SMDK6410 Kukjin Kim
2010-05-27  8:52   ` Ben Dooks
2010-05-28  5:44     ` Marek Szyprowski
2010-06-02  2:31     ` Kukjin Kim
2010-05-27 19:45   ` Russell King - ARM Linux
2010-06-02  2:33     ` Kukjin Kim
2010-05-27  8:22 ` [PATCH 2/4] ARM: S5PV210: Add support for Compact Flash driver on SMDKV210/SMDKC110 Kukjin Kim
2010-05-27 19:46   ` Russell King - ARM Linux
2010-05-27  8:22 ` [PATCH 3/4] ARM: S5PC100: Add support for Compact Flash driver on SMDKC100 Kukjin Kim
2010-05-27 19:47   ` Russell King - ARM Linux
2010-06-02  2:34     ` Kukjin Kim
2010-05-27  8:22 ` [PATCH 4/4] pata_samsung: Add Samsung PATA controller driver Kukjin Kim
2010-05-27  8:43   ` Ben Dooks
2010-06-02  2:37     ` Kukjin Kim
2010-05-27  8:43   ` Jassi Brar
2010-05-27  8:57     ` Ben Dooks
2010-05-27  9:18       ` Jassi Brar
2010-05-27 10:17   ` Sergei Shtylyov
2010-06-02  2:42     ` Kukjin Kim
2010-06-02  9:51       ` Sergei Shtylyov [this message]
2010-06-03  0:25         ` Kukjin Kim
2010-05-27 21:52   ` Tejun Heo
2010-06-02  2:46     ` Kukjin Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C0629B7.4060300@ru.mvista.com \
    --to=sshtylyov@mvista$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox