linux-kernel - RE: [char-misc-next 3/3] mei: add WPT second mei interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 27 May 2014 21:42:19 +0000
From:	"Winkler, Tomas" <tomas.winkler@...el.com>
To:	Greg KH <gregkh@...uxfoundation.org>
CC:	"arnd@...db.de" <arnd@...db.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Usyskin, Alexander" <alexander.usyskin@...el.com>
Subject: RE: [char-misc-next 3/3] mei: add WPT second mei interface

> > +/* PCH devices MEI 2 interface */
> > +const struct mei_cfg mei_me_pch_2_cfg = {
> > +	MEI_CFG_PCH_HFS,
> > +	.mei_id = 1
> 
> That's going to be a recipe for disaster.  Have the MEI core allocate
> the id numbers as things are registered, don't have the individual
> drivers create their id.

I'm don't think can ensure the enumeration order.
This is per device not per driver configuration structure.
Each pci device is actually just  another head to one MEI device but heads are not equal the name/id matters
Yes I assume it looks odd at the first glance, anyhow we are open to any reasonable suggestions 
 

> > diff --git a/drivers/misc/mei/main.c b/drivers/misc/mei/main.c
> > index c58e059..cf4ed5a 100644
> > --- a/drivers/misc/mei/main.c
> > +++ b/drivers/misc/mei/main.c
> > @@ -667,7 +667,7 @@ int mei_register(struct mei_device *dev)
> >  {
> >
> >  	int ret, devno;
> > -	int id = 0; /* FIXME: retrieve interface version*/
> > +	int id = dev->cfg->mei_id;
> 
> Why can't you do this in patch 01 of this series?

It was just progress of development we can refactor that, no problem 
> 
> 
> >
> >  	/* Fill in the data structures */
> >  	devno = MKDEV(MAJOR(mei_devt), id);
> > diff --git a/drivers/misc/mei/mei_dev.h b/drivers/misc/mei/mei_dev.h
> > index a418565..a521783 100644
> > --- a/drivers/misc/mei/mei_dev.h
> > +++ b/drivers/misc/mei/mei_dev.h
> > @@ -384,10 +384,12 @@ enum mei_pg_state {
> >   *
> >   * @fw_status - FW status
> >   * @quirk_probe - device exclusion quirk
> > + * @mei_id - id of mei device
> >   */
> >  struct mei_cfg {
> >  	const struct mei_fw_status fw_status;
> >  	bool (*quirk_probe)(struct pci_dev *pdev);
> > +	int mei_id;
> 
> This should be part of the device, not the configuration, for the reason
> specified above.

As stated above this is per devices configuration.



Thanks
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/