Re: [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory

On Wed, Aug 21, 2019 at 3:37 PM Anju T Sudhakar [off-list ref] wrote:

Hi,

On 8/21/19 10:16 AM, Oliver O'Halloran wrote:

quoted

Is there a reason why we create the debugfs directory here and not in
opal_imc_counters_probe()? There's logic to remove the debugfs
directory in _probe() already so it seems like a more natural place to
it.

Good point. But we can only create the parent directory,
i.e 'imc' directory in `_probe()` function and the entries can be
created only here.

I know the entries can't be added in the probe function and I'm not
suggesting they should be.

The reason is, this debugfs entries are only for
IMC nest units. So, to get the imc mode and command values from
the nest memory region we need the relevant offsets from the control
block structure.

Since imc_get_mem_addr_nest() function reads this address
for each chip, we invoke the function to create the debugfs
entries after this values are populated(i.e export_imc_mode_and_cmd() in
invoked by imc_get_mem_addr_nest()).

Also, if we create the parent directory in `_probe()` function,
we need to track whether the entries(i.e imc_cmd and imc_mode) are
created or not.

I think you should be tracking this anyway so you can do proper
cleanup if a debugfs entry can't be added. The current approach of
nuking the entire debugfs directory is pretty questionable.

Regards,

Anju