linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: JosephChan@via.com.tw
Cc: linux-ide@vger.kernel.org, HaraldWelte@viatech.com,
	BruceChang@via.com.tw
Subject: Re: [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571.
Date: Thu, 15 Jan 2009 22:27:32 +0900	[thread overview]
Message-ID: <496F39C4.8050603@kernel.org> (raw)
In-Reply-To: <C80EF34A3D2E494DBAF9AC29C7AE4EB8087EC568@exchtp03.taipei.via.com.tw>

JosephChan@via.com.tw wrote:
>>> +	if (ap->port_no == 0 && pdev->device == 0xC409) {
>>> +		pci_read_config_dword(pdev, 0x52, &ata66);
>>> +		return (ata66 & 0x10)  ? ATA_CBL_PATA80 : 
>> ATA_CBL_PATA40;
>>> +	} else if (ap->port_no == 1 && pdev->device == 0xC409) {
>>> +		DPRINTK("C409 only has one pata channel\n");
>>> +		return ATA_CBL_PATA_UNK;
>>> +	}
>> Eh.. can you add 0xC409 entry like other controllers and put 
>> necessary restrictions there?
>>
> 
> It's a pity that we don't have corresponding registers for
> identifying the single port controller. So, we just can do this by
> checking the device ID at present.

I feel a bit confused so please correct me if I'm babbling.  Can't you
add { PCI_VDEVICE(VIA, 0xC409), FLAGS_OR_IDENTIFIER } to pci_device_id
table and use different port_info on it?  If that's too different from
how other devices are beign identified, you can just add the
corresponding logic to init_one() and use, say, via_single_port_info
there instead of doing it deep in the reset sequence.

>>>  static void via_tf_load(struct ata_port *ap, const struct 
>>> ata_taskfile *tf)  {
>>> -	struct ata_taskfile tmp_tf;
>>> +	struct ata_ioports *ioaddr = &ap->ioaddr;
>>>  
>>> -	if (ap->ctl != ap->last_ctl && !(tf->flags & 
>> ATA_TFLAG_DEVICE)) {
>>> -		tmp_tf = *tf;
>>> -		tmp_tf.flags |= ATA_TFLAG_DEVICE;
>>> -		tf = &tmp_tf;
>>> +	if (tf->ctl != ap->last_ctl) {
>>> +		iowrite8(tf->ctl, ioaddr->ctl_addr);
>>> +		iowrite8(tf->device, ioaddr->device_addr);
>>>  	}
>>> +
>>>  	ata_sff_tf_load(ap, tf);
>> Can you please explain why this change is necessary?  And if 
>> it's necessary, please put it in a separate patch.
>>
> 
> Yes,  I will separate it.

Thanks.

-- 
tejun

      reply	other threads:[~2009-01-15 13:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-09 10:09 [PATCH 1/1] pata_via.c: Support VX855 and future chips whose IDE controller use 0x0571 JosephChan
2009-01-12 10:58 ` Sergei Shtylyov
2009-01-12 11:02   ` Alan Cox
2009-01-14  7:37     ` JosephChan
2009-01-14  7:39   ` JosephChan
2009-01-13  3:58 ` Tejun Heo
2009-01-15 13:01   ` JosephChan
2009-01-15 13:27     ` Tejun Heo [this message]

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=496F39C4.8050603@kernel.org \
    --to=tj@kernel.org \
    --cc=BruceChang@via.com.tw \
    --cc=HaraldWelte@viatech.com \
    --cc=JosephChan@via.com.tw \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).