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  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]
Date:   Wed, 06 Mar 2019 14:44:28 +0100
From:   Sylwester Nawrocki <s.nawrocki@...sung.com>
To:     cwchoi00@...il.com, Lukasz Luba <l.luba@...tner.samsung.com>
Cc:     devicetree <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Kukjin Kim <kgene@...nel.org>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 4/8] drivers: devfreq: add DMC driver for Exynos5422

Hi,

On 2/3/19 13:23, Chanwoo Choi wrote:

> 2019년 2월 2일 (토) 오전 2:42, Lukasz Luba <l.luba@...tner.samsung.com>님이 작성:

>> +/**
>> + * exynos5_dmc_pause_on_switching() - Controls a pause feature in DMC
>> + * @dmc:       device which is used for changing this feature
>> + * @set:       a boolean state passing enable/disable request
>> + *
>> + * There is a need of pausing DREX DMC when divider or MUX in clock tree
>> + * changes its configuration. In such situation access to the memory is blocked
>> + * in DMC automatically. This feature is used when clock frequency change
>> + * request appears and touches clock tree.
>> + */
>> +static int exynos5_dmc_pause_on_switching(struct exynos5_dmc *dmc, bool set)
> 
> Don't need to make it as the separate function. It is only used on
> probe() function.

It seems fine to me to have this functionality in a separate function, 
it's self-contained and it's now pretty well documented.

>> +{
>> +       unsigned int val;
>> +
>> +       val = readl(dmc->base_clk + DMC_PAUSE_CTRL);
>> +       if (set)
>> +               val |= DMC_PAUSE_ENABLE;
>> +       else
>> +               val &= ~DMC_PAUSE_ENABLE;
>> +       writel(val, dmc->base_clk + DMC_PAUSE_CTRL);
> 
> The dt-binding file doesn't explain the 'reg' property for 'base_clk'.
> You are missing.
> When I tried to find what are the base address, it is the register map
> of clock-controller.
> This driver accesed the register of clock controller without any
> functions of CCF
> (common clock framework). It is wrong.
> 
> If you need to get the some information of clock, must have to use the CCF.
We talked a little about this issue with Lukasz in person and it looks
like there are some DMC related registers in the clock controller register 
region, I'd say those registers are better handled by the DMC driver rather
than the clocks controller driver. Moreover, we should avoid abusing clk API
for not strictly clocks related functionality as it appears to be above.
It might be more appropriate to add in the dmc DT node a phandle to a regmap
exposed by the clock-controller node. It seems there will be even no single
register that would be shared between the DMC and the clock controller.

-- 
Regards,
Sylwester

Powered by blists - more mailing lists