Re: [PATCH wireless-next 23/35] wifi: mm81x: add sdio.c

[Lachlan Hodges]

> > +{
> > +	int ret;
> > +
> > +	ret = sdio_register_driver(&mm81x_sdio_driver);
> > +	if (ret)
> > +		pr_err("sdio_register_driver() failed: %d\n", ret);
> > +
> > +	return ret;
> 
> And you miss here module description. This patch organized per files,
> not per logical pieces, makes it very difficult to review. I have
> absolutely no clue whether this is module or not, whether this is built
> or not, whether it is complete or not.

This patchset was structured similarly to how ath12k was structured
by Kalle [1], in his first and second series revisions, where each file
is split into a separate patch for review and then it's squashed into
a single commit once accepted. Admittedly, ath12k did not contain
anything pertinent to separate subsystems, where we have DT bindings
and mmc ID that require ACKs from their respective maintainers.

For example, if we include the MMC SDIO ID in our sdio.c patchset
such that it is "used" we would be using the wireless subsystem
commit message format rather then the MMC SDIO format that is
traditionally used when updating the sdio_ids.h file. Noting that
existing commits to sdio_ids.h are individual commits adding the
SDIO ID and then used in later commits.

Ultimately the structure of our commits is up to Johannes since we are
going through the wireless tree, but since we have patches that
touch different subsytems that also needs to be considered. Open to
suggestions :).

[1] https://lore.kernel.org/linux-wireless/20220812161003.27279-1-kvalo@xxxxxxxxxx/

--

Appreciate the review Krzysztof, will address your other comments
in a separate mail.

lachlan