Re: [PATCH v2 2/9] ASoC: tegra: add support for CIF programming

[Dmitry Osipenko]

06.02.2020 14:56, Sameer Pujar ÐÐÑÐÑ:
> 
> 
> On 2/5/2020 10:32 PM, Dmitry Osipenko wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 30.01.2020 13:33, Sameer Pujar ÐÐÑÐÑ:
>> ...
>>> +#include <linux/module.h>
>>> +#include <linux/regmap.h>
>>> +#include "tegra_cif.h"
>>> +
>>> +void tegra_set_cif(struct regmap *regmap, unsigned int reg,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct tegra_cif_conf *conf)
>>> +{
>>> +ÂÂÂÂ unsigned int value;
>>> +
>>> +ÂÂÂÂ value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ ((conf->client_ch - 1) <<
>>> TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) |
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂ (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT);
>>> +
>>> +ÂÂÂÂ regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value);
>>> +}
>>> +EXPORT_SYMBOL_GPL(tegra_set_cif);
>> Are you going to add more stuff into this source file later on?
> 
> Yes I plan to add Tegra30 and Tegra124 CIF functions in this. Anything
> related to CIF can be moved here.
>>
>> If not, then it's too much to have a separate driver module just for a
>> single tiny function, you should move it into the header file (make it
>> inline).
> 

You should consider whether it's worth to move anything else to this
module first, because if the functions are not reusable by the drivers,
then the movement won't bring any benefits and final result could be
negative in regards to the code's organization.

I suggest to start clean and easy, without the driver module. You will
be able to factor code into module later on, once there will a real need
to do that.