[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190507144411.GQ16052@vkoul-mobl>
Date: Tue, 7 May 2019 20:14:11 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc: alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
tiwai@...e.de, broonie@...nel.org, gregkh@...uxfoundation.org,
liam.r.girdwood@...ux.intel.com, jank@...ence.com, joe@...ches.com,
srinivas.kandagatla@...aro.org,
Sanyog Kale <sanyog.r.kale@...el.com>
Subject: Re: [PATCH 7/8] soundwire: fix master properties
On 03-05-19, 19:29, Pierre-Louis Bossart wrote:
> The master-count is only defined for a controller or a Slave in the
> MIPI DisCo for SoundWire document.
... so remove it
> rename all fields with 'freq' as 'clk_freq' to follow the MIPI
> specification and avoid confusion between bus clock and audio clocks.
That sounds good to me.
> fix support for clock_stop_mode0 and 1. The existing code uses a
> bitmask between enums, one of which being zero. Or'ing with zero is
> not very useful in general...Fix by or-ing with a BIT dependent on the
> enum value.
>
> Fix additional comments to align with MIPI spec
Ideally these should be different patches...
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> ---
> drivers/soundwire/bus.c | 4 ++--
> drivers/soundwire/intel.c | 11 ++++++-----
> drivers/soundwire/mipi_disco.c | 27 ++++++++++++++-------------
> drivers/soundwire/stream.c | 2 +-
> include/linux/soundwire/sdw.h | 20 +++++++++-----------
> 5 files changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index aac35fc3cf22..96e42df8f458 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -87,7 +87,7 @@ int sdw_add_bus_master(struct sdw_bus *bus)
>
> /*
> * Initialize clock values based on Master properties. The max
> - * frequency is read from max_freq property. Current assumption
> + * frequency is read from max_clk_freq property. Current assumption
> * is that the bus will start at highest clock frequency when
> * powered on.
> *
> @@ -95,7 +95,7 @@ int sdw_add_bus_master(struct sdw_bus *bus)
> * to start with bank 0 (Table 40 of Spec)
> */
> prop = &bus->prop;
> - bus->params.max_dr_freq = prop->max_freq * SDW_DOUBLE_RATE_FACTOR;
> + bus->params.max_dr_freq = prop->max_clk_freq * SDW_DOUBLE_RATE_FACTOR;
> bus->params.curr_dr_freq = bus->params.max_dr_freq;
> bus->params.curr_bank = SDW_BANK0;
> bus->params.next_bank = SDW_BANK1;
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index 31336b0271b0..4ac141730b13 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -796,13 +796,14 @@ static int intel_prop_read(struct sdw_bus *bus)
> sdw_master_read_prop(bus);
>
> /* BIOS is not giving some values correctly. So, lets override them */
> - bus->prop.num_freq = 1;
> - bus->prop.freq = devm_kcalloc(bus->dev, bus->prop.num_freq,
> - sizeof(*bus->prop.freq), GFP_KERNEL);
> - if (!bus->prop.freq)
> + bus->prop.num_clk_freq = 1;
> + bus->prop.clk_freq = devm_kcalloc(bus->dev, bus->prop.num_clk_freq,
> + sizeof(*bus->prop.clk_freq),
> + GFP_KERNEL);
> + if (!bus->prop.clk_freq)
> return -ENOMEM;
>
> - bus->prop.freq[0] = bus->prop.max_freq;
> + bus->prop.clk_freq[0] = bus->prop.max_clk_freq;
> bus->prop.err_threshold = 5;
>
> return 0;
> diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
> index f6b1be920a19..7db816691393 100644
> --- a/drivers/soundwire/mipi_disco.c
> +++ b/drivers/soundwire/mipi_disco.c
> @@ -50,39 +50,40 @@ int sdw_master_read_prop(struct sdw_bus *bus)
>
> if (fwnode_property_read_bool(link,
> "mipi-sdw-clock-stop-mode0-supported"))
> - prop->clk_stop_mode = SDW_CLK_STOP_MODE0;
> + prop->clk_stop_modes |= BIT(SDW_CLK_STOP_MODE0);
>
> if (fwnode_property_read_bool(link,
> "mipi-sdw-clock-stop-mode1-supported"))
> - prop->clk_stop_mode |= SDW_CLK_STOP_MODE1;
> + prop->clk_stop_modes |= BIT(SDW_CLK_STOP_MODE1);
>
> fwnode_property_read_u32(link,
> "mipi-sdw-max-clock-frequency",
> - &prop->max_freq);
> + &prop->max_clk_freq);
>
> nval = fwnode_property_read_u32_array(link,
> "mipi-sdw-clock-frequencies-supported", NULL, 0);
> if (nval > 0) {
> - prop->num_freq = nval;
> - prop->freq = devm_kcalloc(bus->dev, prop->num_freq,
> - sizeof(*prop->freq), GFP_KERNEL);
> - if (!prop->freq)
> + prop->num_clk_freq = nval;
> + prop->clk_freq = devm_kcalloc(bus->dev, prop->num_clk_freq,
> + sizeof(*prop->clk_freq),
> + GFP_KERNEL);
> + if (!prop->clk_freq)
> return -ENOMEM;
>
> fwnode_property_read_u32_array(link,
> "mipi-sdw-clock-frequencies-supported",
> - prop->freq, prop->num_freq);
> + prop->clk_freq, prop->num_clk_freq);
> }
>
> /*
> * Check the frequencies supported. If FW doesn't provide max
> * freq, then populate here by checking values.
> */
> - if (!prop->max_freq && prop->freq) {
> - prop->max_freq = prop->freq[0];
> - for (i = 1; i < prop->num_freq; i++) {
> - if (prop->freq[i] > prop->max_freq)
> - prop->max_freq = prop->freq[i];
> + if (!prop->max_clk_freq && prop->clk_freq) {
> + prop->max_clk_freq = prop->clk_freq[0];
> + for (i = 1; i < prop->num_clk_freq; i++) {
> + if (prop->clk_freq[i] > prop->max_clk_freq)
> + prop->max_clk_freq = prop->clk_freq[i];
> }
> }
>
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index d01060dbee96..89edc897b8eb 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1474,7 +1474,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
> memcpy(¶ms, &bus->params, sizeof(params));
>
> /* TODO: Support Asynchronous mode */
> - if ((prop->max_freq % stream->params.rate) != 0) {
> + if ((prop->max_clk_freq % stream->params.rate) != 0) {
> dev_err(bus->dev, "Async mode not supported\n");
> return -EINVAL;
> }
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index 80584e9d5970..89c51838b7ec 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -364,29 +364,27 @@ struct sdw_slave_prop {
> /**
> * struct sdw_master_prop - Master properties
> * @revision: MIPI spec version of the implementation
> - * @master_count: Number of masters
> - * @clk_stop_mode: Bitmap for Clock Stop modes supported
> - * @max_freq: Maximum Bus clock frequency, in Hz
> + * @clk_stop_modes: Bitmap, bit N set when clock-stop-modeN supported
> + * @max_clk_freq: Maximum Bus clock frequency, in Hz
> * @num_clk_gears: Number of clock gears supported
> * @clk_gears: Clock gears supported
> - * @num_freq: Number of clock frequencies supported, in Hz
> - * @freq: Clock frequencies supported, in Hz
> + * @num_clk_freq: Number of clock frequencies supported, in Hz
> + * @clk_freq: Clock frequencies supported, in Hz
> * @default_frame_rate: Controller default Frame rate, in Hz
> * @default_row: Number of rows
> * @default_col: Number of columns
> - * @dynamic_frame: Dynamic frame supported
> + * @dynamic_frame: Dynamic frame shape supported
> * @err_threshold: Number of times that software may retry sending a single
> * command
> */
> struct sdw_master_prop {
> u32 revision;
> - u32 master_count;
> - enum sdw_clk_stop_mode clk_stop_mode;
> - u32 max_freq;
> + u32 clk_stop_modes;
> + u32 max_clk_freq;
> u32 num_clk_gears;
> u32 *clk_gears;
> - u32 num_freq;
> - u32 *freq;
> + u32 num_clk_freq;
> + u32 *clk_freq;
> u32 default_frame_rate;
> u32 default_row;
> u32 default_col;
> --
> 2.17.1
--
~Vinod
Powered by blists - more mailing lists