kern/54538: wdc(4) regression after SATA NCQ merge

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>Number:         54538
>Category:       kern
>Synopsis:       wdc(4) regression after SATA NCQ merge
>Confidential:   no
>Severity:       critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 09 12:40:00 +0000 2019
>Originator:     Izumi Tsutsui
>Release:        NetBSD 9.0_BETA updated around 201909091200
>Organization:
>Environment:
System: NetBSD 9.0_BETA (-current is also affected)
Architecture: all
Machine: all
>Description:
On SATA-NCQ (jdolecek-ncq branch) merge,
 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ic/wdc.c#rev1.284
wdcprobe() function in src/sys/dev/ic/wd.c has been changed to take
(struct wdc_regs *wdr) instead of the previous (struct ata_channel *chp):

 http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ic/wdc.c.diff?r1=1.283&r2=1.284
---before---
int
wdcprobe(struct ata_channel *chp)
{
	struct wdc_softc *wdc = CHAN_TO_WDC(chp);
	/* default reset method */
	if (wdc->reset == NULL)
		wdc->reset = wdc_do_reset;

	return (wdcprobe1(chp, 1));
}
---end of quote----

---after---
int
wdcprobe(struct wdc_regs *wdr)
{
	struct wdc_softc wdc;
	struct ata_channel ch;
	int rv;

	memset(&wdc, 0, sizeof(wdc));
	memset(&ch, 0, sizeof(ch));
	ata_channel_init(&ch);
	ch.ch_atac = &wdc.sc_atac;
	ch.ch_queue = ata_queue_alloc(1);
	wdc.regs = wdr;

	/* default reset method */
	if (wdc.reset == NULL)
		wdc.reset = wdc_do_reset;

	rv = wdcprobe1(&ch, 1);

	ata_queue_free(ch.ch_queue);
	ata_channel_destroy(&ch);

	return rv;
}
---end of quote---

As you can see above, we no longer can specify MD wdc reset function
because the struct wdc is prepared in wdcprobe() itself:

>>	memset(&wdc, 0, sizeof(wdc));
 :
>>	/* default reset method */
>>	if (wdc.reset == NULL)
>>		wdc.reset = wdc_do_reset;

so currently there is no way to pass the MD function that was passed
via struct ata_channel.

This breaks at least sys/arch/dreamcast/conf/G1IDE functions.
(though not enabled by default)

Note there are more MD drivers that have not been updated
for the wdcprobe() API changes:

 https://nxr.netbsd.org/source/s?refs=wdcprobe&project=src

  /src/sys/arch/evbppc/mpc85xx/
	wdc_obio.c 	129 rv = wdcprobe(&ch);

  /src/sys/arch/dreamcast/dev/g1/
	wdc_g1.c 	117 result = wdcprobe(&ch);

  /src/sys/arch/mips/adm5120/dev/
	wdc_extio.c 	266 result = wdcprobe(&ch);

  /src/sys/arch/mmeye/dev/
	wdc_mainbus.c 	118 result = wdcprobe(&ch);

>How-To-Repeat:
Code inspection.

>Fix:
1) Change wdcprobe() to take a MD reset function?
   (I'm not sure how many attachments should be updated though)

2) Add a new public probe function (like wdcprobe2())
   that also takes MD reset function?

3) Revert the wdcprobe() API change?

---
Izumi Tsutsui