linux-kernel - Re: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization

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]

Message-ID: <20200520135425.GX374218@vkoul-mobl.Dlink>
Date:   Wed, 20 May 2020 19:24:25 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Bard Liao <yung-chuan.liao@...ux.intel.com>
Cc:     alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        tiwai@...e.de, broonie@...nel.org, gregkh@...uxfoundation.org,
        jank@...ence.com, srinivas.kandagatla@...aro.org,
        rander.wang@...ux.intel.com, ranjani.sridharan@...ux.intel.com,
        hui.wang@...onical.com, pierre-louis.bossart@...ux.intel.com,
        sanyog.r.kale@...el.com, slawomir.blauciak@...el.com,
        mengdong.lin@...el.com, bard.liao@...el.com
Subject: Re: [PATCH 2/2] soundwire: intel: transition to 3 steps
 initialization

On 20-05-20, 03:19, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> 
> Rather than a plain-vanilla init/exit, this patch provides 3 steps in
> the initialization (ACPI scan, probe, startup) which makes it easier to
> detect platform support for SoundWire, allocate required resources as
> early as possible, and conversely help make the startup() callback
> lighter-weight with only hardware register setup.

Okay but can you add details in changelog on what each step would do?

> @@ -1134,25 +1142,15 @@ static int intel_probe(struct platform_device *pdev)
>  
>  	intel_pdi_ch_update(sdw);
>  
> -	/* Acquire IRQ */
> -	ret = request_threaded_irq(sdw->link_res->irq,
> -				   sdw_cdns_irq, sdw_cdns_thread,
> -				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);

This is removed here but not added anywhere else, do we have no irq
after this patch?

> @@ -1205,5 +1201,5 @@ static struct platform_driver sdw_intel_drv = {
>  module_platform_driver(sdw_intel_drv);
>  
>  MODULE_LICENSE("Dual BSD/GPL");
> -MODULE_ALIAS("platform:int-sdw");
> +MODULE_ALIAS("sdw:intel-sdw");

it is still a platform device, so does sdw: tag make sense?
This is used by modprobe to load the driver!

> +/**
> + * sdw_intel_probe() - SoundWire Intel probe routine
> + * @res: resource data
> + *
> + * This creates SoundWire Master and Slave devices below the controller.

I dont think the comment is correct, this is done in intel_master_probe
which is platform device probe...

> + * All the information necessary is stored in the context, and the res
> + * argument pointer can be freed after this step.
> + */
> +struct sdw_intel_ctx
> +*sdw_intel_probe(struct sdw_intel_res *res)
> +{
> +	return sdw_intel_probe_controller(res);
> +}
> +EXPORT_SYMBOL(sdw_intel_probe);

I guess this would be called by SOF driver, question is when..?

> +/**
> + * sdw_intel_startup() - SoundWire Intel startup
> + * @ctx: SoundWire context allocated in the probe
> + *
> + */
> +int sdw_intel_startup(struct sdw_intel_ctx *ctx)
> +{
> +	return sdw_intel_startup_controller(ctx);
> +}
> +EXPORT_SYMBOL(sdw_intel_startup);

when is this called, pls do document that

-- 
~Vinod