Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2 2/9] ASoC: tegra: add support for CIF programming
To:     Sameer Pujar <spujar@nvidia.com>
Cc:     perex@perex.cz, tiwai@suse.com, robh+dt@kernel.org,
        broonie@kernel.org, lgirdwood@gmail.com, thierry.reding@gmail.com,
        jonathanh@nvidia.com, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org, sharadg@nvidia.com,
        mkumard@nvidia.com, viswanathl@nvidia.com, rlokhande@nvidia.com,
        dramesh@nvidia.com, atalambedu@nvidia.com
References: <1580380422-3431-1-git-send-email-spujar@nvidia.com>
 <1580380422-3431-3-git-send-email-spujar@nvidia.com>
 <7239e858-16b7-609f-c4e3-8135bee8450b@gmail.com>
 <1305a6db-a492-eec2-111e-ddc801d58d86@nvidia.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <fb0a96bd-ac3c-0916-0337-0c86de196527@gmail.com>
Date:   Thu, 6 Feb 2020 19:49:55 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.3.0
MIME-Version: 1.0
In-Reply-To: <1305a6db-a492-eec2-111e-ddc801d58d86@nvidia.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.