Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: bBnkCfE1yclmjedQp0f8WQTr5UuAFnfXrJ50zlb6CRW3gHGlVMlcyBGgs2q+sQg/bl/pJQv015
 qCXFJPO7o0jA==
IronPort-SDR: wgydlkxHzdbPkAHlRcc5xIYsrqEQGcXfiPbo3gYZKX/SSCCRRDZfU8PMfwqret09vHOqkz9tbN
 EGbMBolbKFVA==
From:   "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com>
To:     Greg KH <gregkh@linuxfoundation.org>
CC:     "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "luca@coelho.fi" <luca@coelho.fi>,
        "Beker, Ayala" <ayala.beker@intel.com>,
        "Coelho, Luciano" <luciano.coelho@intel.com>
Subject: RE: [PATCH RESEND 2/3] iwlwifi: mei: add the driver to allow
 cooperation with CSME
Thread-Topic: [PATCH RESEND 2/3] iwlwifi: mei: add the driver to allow
 cooperation with CSME
Thread-Index: AQHXL5l8AAlmpHtWLk6IhaLef8ZD26qw2g6AgAAEIoCAABAHAIAAAg7ggAACuoCAAAF48A==
Date:   Mon, 12 Apr 2021 14:46:53 +0000
Message-ID: <SA0PR11MB47522D6E66CAD30CF0A63651F2709@SA0PR11MB4752.namprd11.prod.outlook.com>
References: <20210412124328.24472-1-emmanuel.grumbach@intel.com>
 <20210412124328.24472-2-emmanuel.grumbach@intel.com>
 <YHRFy3aq/gB7Vde6@kroah.com>
 <SA0PR11MB475215678ADCCE657C6574D6F2709@SA0PR11MB4752.namprd11.prod.outlook.com>
 <YHRWtJqbuFGmR2Sw@kroah.com>
 <SA0PR11MB475228709B9B0B6D74CE07A2F2709@SA0PR11MB4752.namprd11.prod.outlook.com>
 <YHRat4egd/LM8DNJ@kroah.com>
In-Reply-To: <YHRat4egd/LM8DNJ@kroah.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.5.1.3
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 32693546-879e-4cc6-ee5f-08d8fdc1d052
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Apr 2021 14:46:53.7321
 (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: 8c1esVgXtYbjQXKcmkusqK/Exa376MinKAezj5uny9oOXH2Zx16uyuG4T8UeLYqbnAnGejoIO3zHWeaAFaU8vSU7FXmuwIRlxfeG0m4vsyA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4702
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

> On Mon, Apr 12, 2021 at 02:29:45PM +0000, Grumbach, Emmanuel wrote:
> > >
> > > On Mon, Apr 12, 2021 at 01:44:58PM +0000, Grumbach, Emmanuel wrote:
> > > > > > +#define IWL_MEI_DEBUG(c, f, a...)		\
> > > > > > +	do {					\
> > > > > > +		CHECK_FOR_NEWLINE(f);		\
> > > > >
> > > > > Huh?
> > > > >
> > > > > > +		dev_dbg(&(c)->dev, f, ## a);	\
> > > > >
> > > > > Just use dev_dbg(), don't be special for a single driver, it
> > > > > hurts when trying to read different drivers.
> > > >
> > > > I took this from iwlwifi. I can change if needed, not a big deal.
> > >
> > > Please do.
> > >
> > > > > > +module_param_named(defer_start_message,
> defer_start_message,
> > > > > bool,
> > > > > > +0644); MODULE_PARM_DESC(defer_start_message,
> > > > > > +		 "Defer the start message Tx to CSME (default
> false)");
> > > > >
> > > > > Why do you need this?  Who is going to set it to anything else,
> > > > > and why would they?  This isn't the 1990's anymore, please do
> > > > > not add new module parameters.
> > > >
> > > > For testing. I need this to be able to force a certain order of
> > > > initialization
> > > which is possible (and hence must be tested) but not likely to happen=
.
> > > > Another point is tracing. This allows me to load the module but
> > > > prevent any
> > > real operation. Then, start tracing. This way, I can see the whole
> > > flow in tracing, even the very beginning.
> > >
> > > Then call this something obvious,
> > >
> "kernel_hacker_debuging_testing_only_use_if_you_know_what_you_are_
> > > doing".
> > >
> > > Or better yet, just put it in debugfs to turn it on/off and no
> > > module parameter is needed at all.
> > >
> >
> > Debugfs is not a replacement for module parameters. Debugfs can be
> > used only after the driver already ran quite a bit of its
> > initialization code path. Here I want to be able to catch the very
> > first messages with tracing.
>=20
> Then use the proper trace functionality of the kernel, which is not modul=
e
> parameters :(

I am sorry if I drive you nuts but I don't know any "proper trace functiona=
lity" in the kernel that the user could enable / disable and that would be =
available immediately at init. The user needs to "activate" the trace point=
s using trace-cmd or whatever other tool. By the time the user does so, the=
 driver has already run the code path I wish to debug. I can use debug prin=
ts, but you didn't seem happy about it. So I am happy to use tracing, but t=
hen we need to make sure it cover all the cases. The way I make it cover al=
l the cases is with this module parameter. If you know a better way, I'll b=
e happy to use it.