Alan,
Thanks for your comments:
On Sat, 26 Apr 2008 17:30:12 +0800
"Alan Cox" <alan@xxxxxxxxxxxxxxxxxxx> wrote:
> Re: [PATCH] ata: Add Intel SCH PATA support
>
> > +struct sch_80 {
> > + u16 device;
> > +};
>
> That seems somewhat overkill
I have to consider there would be other PATA controllers work this way.
>
> > + /* Check for specials - Intel SCH chipset */
> > + while (sch->device) {
> > + if (sch->device == pdev->device)
> > + return ATA_CBL_PATA80;
> > + sch++;
>
> PATA_80 means "controller *knows* we are 80 wire". PATA_UNK means
> "controller can't do detect."
I need to force SCH to use PATA 80 pins to enable UDMA5, just like the way ich_laptop force
to use 40 pins.
>
> > + /* Check for specials - Intel SCH chipset */
> > + while (sch->device) {
> > + if (sch->device == pdev->device) {
> > + skip_check = 1;
> > + break;
> > + }
> > + sch++;
>
> This is horrible. If you just defined a new set of methods for your new
> controller (remembering we now have inheritance anyway) you'd just use
> the ata_sff_prereset and remove all the special casing code. At the very
> least write the routine once only;)
>
hmmm... for non-sch chipsets, there is no impact, this will only affect sch. You mean
I should create a ata_sch.c file instead of patching ata_piix.c ?
> > + }
> > + if (!skip_check)
> > + if (!pci_test_config_bits(pdev, &piix_enable_bits[ap->port_no]))
> > + return -ENOENT;
> > return ata_sff_prereset(link, deadline);
> > }
> >
--
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
