Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Usyskin, Alexander" <alexander.usyskin@intel.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Winkler, Tomas" <tomas.winkler@intel.com>
CC:     "Lubart, Vitaly" <vitaly.lubart@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [char-misc-next] mei: gsc_proxy: add gsc proxy driver
Thread-Topic: [char-misc-next] mei: gsc_proxy: add gsc proxy driver
Thread-Index: AQHZFlEYy41no8BHb0OgaolMrZvbla57EG6AgAUZmkA=
Date:   Wed, 28 Dec 2022 11:46:36 +0000
Message-ID: <CY5PR11MB6366515D2A965EEDEC77AF63EDF29@CY5PR11MB6366.namprd11.prod.outlook.com>
References: <20221222220214.3688774-1-tomas.winkler@intel.com>
 <Y6VV5d/V4MKDz2Te@kroah.com>
In-Reply-To: <Y6VV5d/V4MKDz2Te@kroah.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?10YU7+yw3Gp3Nj3lu4ZqNkKxnfoXp8R7s+3HhrcOsMZQ/GV9LG/hvz0Jjt+A?=
 =?us-ascii?Q?eoKsnbEp6RSeR/S3bAUgi4KQqThsEyMwpqR9XRNx24SpvGbvN6HDTgruqEbp?=
 =?us-ascii?Q?441Hg99LhBtJhT/sXkiHRKaEuW35NDHo654gDkmaK7KureASzQa0LMkn21BX?=
 =?us-ascii?Q?W9IOidLwDlW+SvRmz31iLvBu84XXOJSbvwKG8wWIUhOLDkM7xdXppeOyoWsK?=
 =?us-ascii?Q?X+1C/++qqrw4Bc0SvntJs1SylTGXm/3lu4tmfxzPaklg0wfw0cP4NGialdny?=
 =?us-ascii?Q?jFzObs61cPP+R/vySr30ggQc2Ykq4sbncGLR+6iN1HdYtvsfOGbC8sNHuh5z?=
 =?us-ascii?Q?C2oIpd79vrkySlAYz62KbBB4rdLteldVNZxhETBGugQTrRe7V4rWviST2jDV?=
 =?us-ascii?Q?9LLxDT25u9YLSXUwg9pn7N4MjuC6dR/SpWL0Mzn+knqDqbTFuKrHcNB+a0Fk?=
 =?us-ascii?Q?0cgB1iz6GvO7JpeNne4BKZwa5Mz5mKlnqiNV5szOVz27w6H7DVhbrglzGJzy?=
 =?us-ascii?Q?sVBMtUoO7iFY3VCrp34nz/HAwBRUkW4ebZ7IM6aGMgzbAx7EVYkK60U9F0PX?=
 =?us-ascii?Q?M/YTwQY++8d/XmbxUvNFfj7vTcEMKxoGGHzNunxxWCkXobJvBZ+6suwYkN5k?=
 =?us-ascii?Q?olKjIiwbPBKiqPB8tQzG9QV2AEqXvmODVTZZz8XiLtACd185oaq4DWeR6ZeJ?=
 =?us-ascii?Q?R47ph6jRd8hsW/gpGatpxpu6P7DLAfRNTaDIR5odLCCEuvu07GSQ1E5IBtBS?=
 =?us-ascii?Q?tM6pKC5yKdo4ou/VjHpkV6Lh2JSOiT0za8+q8r4dI11cD8adNhi7/jgeATKW?=
 =?us-ascii?Q?wVNqf42bOPZGwMYDEVVxVyPM3hdgmXm5GFZYb2S6XMTf0q/M2MSucqnWrVlL?=
 =?us-ascii?Q?Cg45eVZQKktc4atsxsv209suEbO8dIVuaBvUpd7WbNKhVQlhMNx5LBO7Gg/5?=
 =?us-ascii?Q?hCz9wTsWM+dIWdW3DH5mI9ngpt7ZGVtYS7EVZ0kgbDEJY/hMNzaairEs5IxE?=
 =?us-ascii?Q?h8fnPKlxl52ZcR5DthRrEWpHg0IEKIPeYT5O3DT0kSHKUjEvxclrWKqm8bPt?=
 =?us-ascii?Q?NwBNrbmgTMRSUKjc8wXEdonr7QCD9nsH7Y06yFygbr7OU6GcQe8q9L8B3iJc?=
 =?us-ascii?Q?XZrXVHOqKomW1mCcHhZtbpfxMn5rHWoufI3dbzbNlP0ye25AkmItleWhM16y?=
 =?us-ascii?Q?jkD5LuD6puecYUsFZBZWZtMA9OrS/aYhHLMSZ6rWEt+KZhcyM2ijypZnTKFo?=
 =?us-ascii?Q?MMckRB8Mu/QIhDHuV7cJre2cNLk54mHP3zC7LFR1wvfiLMeCJrraNhF6f43i?=
 =?us-ascii?Q?FeDB+oczTZaVxkDzqzprvH81uAaYLJBpF1UZUViK0GN0x91i4K2fVIplB4Uo?=
 =?us-ascii?Q?zK0Wkk348/0khYMAQFXRQxvApNDFBUvjEFLPyrn1YyjxfA0AjqXHd5o+QCP4?=
 =?us-ascii?Q?WfqhwVarlMmc5LcexAMuqeGUGiUIER+fAp9VH1bhm+oogmYiHAQL2zIwY0nR?=
 =?us-ascii?Q?gRDoyzM4sfRbljEgMuZr0KqBg5If2wgxYF3sxtxpNHJktk32ZqMrpN4gflxF?=
 =?us-ascii?Q?d/smhUNCazmw0QSr3To6tXudMBm0yTa81zF8JwzyQYRR6glwP4tTk5XqPBb5?=
 =?us-ascii?Q?CA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6366.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1bf572bb-85be-4800-86f6-08dae8c92ccd
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Dec 2022 11:46:36.3927
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4eLs0ONwO6lQwTcNYqDwFAB9VRPL5csrDBDmSoXG4zeIoOGtACm8mzaV3i52f/F+bVmHoU8ZLOAykJafmjieXfXwEgCLVTtkyMvjEeN0bP8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7843
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>=20
> Why a whole new subdirectory for a tiny 200 line file?
>=20
All drivers for devices on mei bus have private subdirectory.
This one just modelled on the existing examples.
If you say that this is not a good thing - can put it in the main mei direc=
tory.

> > +static int mei_gsc_proxy_component_match(struct device *dev, int
> subcomponent, void *data)
> > +{
> > +	struct device *base =3D data;
> > +
> > +	if (!dev || !dev->driver ||
> > +	    strcmp(dev->driver->name, "i915") ||
>=20
> I thought I had objected to this "let's poke around in a driver name for
> a magic value" logic in the past.  How do you know this is always going
> to work?

All components that serve Intel graphics integrated into PCH should check
in their match that calling device is graphic card sitting on the same PCH.
The code below checks that i915 pci device and mei pci device (grandparent =
of our device on mei bus)
are children of the same parent, but there is no way to know if caller
is, indeed, graphic device. Easiest way is to check well-known device river=
 name.
All i915 components doing this comparison.
This is a simplified scheme of relations between devices here:
           /--- MEI PCI --- MEI --- GSC_PROXY
PCH ---
           \--- GRAPHIC PCI --- I915
>=20
> > +	    subcomponent !=3D I915_COMPONENT_GSC_PROXY)
> > +		return 0;
> > +
> > +	base =3D base->parent;
> > +	if (!base) /* mei device */
> > +		return 0;
>=20
> How can a device not have a parent?

This one should be proxy device on mei bus, so parent should be there alway=
s, can drop this check.

>=20
> > +
> > +	base =3D base->parent; /* pci device */
>=20
> You don't know this is a pci device :(

This is more, like, note to explain on what level in above scheme we are no=
w.
It should be mei pci device for match to succeed.

>=20
> If it is, then pass in a REAL pci device structure please.
>=20
> > +
> > +	dev =3D dev->parent;
> > +	return (base && dev && dev =3D=3D base);
>=20
> I do not understand this statement at all, what are you doing here?
>=20
> confused,

Hope that scheme above explains the relations between devices.
>=20
> greg k-h


--=20
Thanks,
Sasha