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

[Sameer Pujar]

On 2/6/2020 10:19 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 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.

Tegra124 can reuse above CIF function. Tegra30 will continue to use the 
same function. For consistency all CIF related helpers can be grouped at 
one place. But this is for later. I will start with inline function.