NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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

>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
System: NetBSD 9.0_BETA (-current is also affected)
Architecture: all
Machine: all
On SATA-NCQ (jdolecek-ncq branch) merge,
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):
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----

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));
	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);


	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:

	wdc_obio.c 	129 rv = wdcprobe(&ch);

	wdc_g1.c 	117 result = wdcprobe(&ch);

	wdc_extio.c 	266 result = wdcprobe(&ch);

	wdc_mainbus.c 	118 result = wdcprobe(&ch);

Code inspection.

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

Home | Main Index | Thread Index | Old Index