Add PNP info to PCI attachment of ae, age, ahci, amr, ale, an, bwi, bwn drivers

[lakhanshiva]

The device id table for ahci is

static const struct {
	uint32_t	id;
	uint8_t		rev;
	const char	*name;
	int		quirks;
} ahci_ids[]

Therefore i used "W32:vendor/device" as descriptor string in MODULE_PNP_INFO

The device id table for an is

struct an_type {
	u_int16_t		an_vid;
	u_int16_t		an_did;
	char			*an_name;
};

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for bwn is

struct bwn_pci_device {
	uint16_t	vendor;
	uint16_t	device;
	const char	*desc;
	uint32_t	quirks;
}

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for bwi is

static const struct bwi_dev {
	uint16_t	vid;
	uint16_t	did;
	const char	*desc;
}

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for ale is

static const struct ale_dev {
	uint16_t	ale_vendorid;
	uint16_t	ale_deviceid;
	const char	*ale_name;
}

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for amr is

static struct amr_ident
{
    int		vendor;
    int		device;
    int		flags;
#define AMR_ID_PROBE_SIG	(1<<0)	/* generic i960RD, check signature */
#define AMR_ID_DO_SG64		(1<<1)
#define AMR_ID_QUARTZ		(1<<2)
}

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for age is

static struct age_dev {
	uint16_t	age_vendorid;
	uint16_t	age_deviceid;
	const char	*age_name;
} age_devs[];

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for ae is

static struct ae_dev {
	uint16_t	vendorid;
	uint16_t	deviceid;
	const char	*name;
} ae_devs[];

Therefore i used "U16:vendor;U16:device;D:#" as descriptor string in MODULE_PNP_INFO

[ctuffli]

@lakhanshiva, there seem to be multiple iterations to the AHCI driver. Because they are all related, would you mind squashing them into a single commit to make the review easier?

Also, it would be worth discussing with @bsdimp if a pull request should have a single driver or if submitting changes for multiple drivers is OK.

[ctuffli]

Shouldn't the passed parameter be ahci_ids without the trailing []?

[lakhanshiva]

Hi Chuck, Yes, I will squash commits related to same driver into one. On May 16, 2018 6:40 PM, "Chuck Tuffli" <notifications@github.com> wrote: *@ctuffli* requested changes on this pull request. @lakhanshiva <https://github.com/lakhanshiva>, there seem to be multiple iterations to the AHCI driver. Because they are all related, would you mind squashing them into a single commit to make the review easier? Also, it would be worth discussing with @bsdimp <https://github.com/bsdimp> if a pull request should have a single driver or if submitting changes for multiple drivers is OK. ------------------------------ In sys/dev/ahci/ahci_pci.c <#3 (comment)>:

@@ -684,3 +684,5 @@ static driver_t ahci_ata_driver = {

sizeof(struct ahci_controller) }; DRIVER_MODULE(ahci, atapci, ahci_ata_driver, ahci_devclass, NULL, NULL); +MODULE_PNP_INFO("U32:id;U8:rev;D:#", pci, ahci, ahci_ids[], Shouldn't the passed parameter be ahci_ids without the trailing []? I have taken care of this in another commit. I am going to squash the commits together, so it would be easier to review. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEiqmEAp6nPHqoYC8P-FWkrqCtpnvdSzks5tzCVfgaJpZM4UAw5E> . Thanks, Lakhan

[lakhanshiva]

I have squashed the commits related to same driver into one commit. I will discuss with bsdimp regarding one driver per pull request.

[ctuffli]

LGTM

[ctuffli]

LGTM

[ctuffli]

As to your email question, I don't see an issue with using rev in the description string, but I'll defer to @bsdimp

[bsdimp]

This line needs to be just before the ahci_ata_methods table.

[lakhanshiva]

Moved it.

[bsdimp]

I think this needs to be "W32:vendor/device;" We can leave out the U8:rev; stuff since it won't change which driver is loaded.

[lakhanshiva]

Yeah, I've excluded U8:rev and later portion and changed it to W32

[ctuffli]

For the unique string, I'd be tempted to make this bwn_siba instead of bwn_pci to give a hint this is related to siba(4)

[lakhanshiva]

Yep, changed it as bwn_siba

[ctuffli]

Similar to the above, I'd be tempted to make this bwn_bcma to give a hint this is related to bmca(4)

[lakhanshiva]

Yep, changed it as bwn_bcma

[bsdimp]

I think this string is right...

[bsdimp]

but we need to change vendor and device here to be uint16_t's rather than int's. int's are 32-bits, but a poor fit for this data.

[lakhanshiva]

I have made this change accordingly..

…

On Thu, May 31, 2018 at 8:27 PM Warner Losh ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sys/dev/amr/amr_pci.c <#3 (comment)>: > @@ -116,11 +116,6 @@ static driver_t amr_pci_driver = { sizeof(struct amr_softc) }; -static devclass_t amr_devclass; -DRIVER_MODULE(amr, pci, amr_pci_driver, amr_devclass, 0, 0); -MODULE_DEPEND(amr, pci, 1, 1, 1); -MODULE_DEPEND(amr, cam, 1, 1, 1); - static struct amr_ident { int vendor; but we need to change vendor and device here to be uint16_t's rather than int's. int's are 32-bits, but a poor fit for this data. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEiqmJodEQqm3LqjB15oozyMU6LVIh9kks5t4ATugaJpZM4UAw5E> .