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

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: <fb0a96bd-ac3c-0916-0337-0c86de196527@gmail.com>
Date:   Thu, 6 Feb 2020 19:49:55 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sameer Pujar <spujar@...dia.com>
Cc:     perex@...ex.cz, tiwai@...e.com, robh+dt@...nel.org,
        broonie@...nel.org, lgirdwood@...il.com, thierry.reding@...il.com,
        jonathanh@...dia.com, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, sharadg@...dia.com,
        mkumard@...dia.com, viswanathl@...dia.com, rlokhande@...dia.com,
        dramesh@...dia.com, atalambedu@...dia.com
Subject: Re: [PATCH v2 2/9] ASoC: tegra: add support for CIF programming

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.