lkml.org 
[lkml]   [2022]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 3/4] soc: starfive: Add StarFive JH71XX pmu driver
Hey Walker,

Apologies for my formatting here.

On 1 December 2022 03:56:27 GMT, Walker Chen <walker.chen@starfivetech.com> wrote:
>On 2022/11/25 19:17, Conor Dooley wrote:
>> On Fri, Nov 25, 2022 at 06:04:59PM +0800, Walker Chen wrote:
>>> On 2022/11/19 8:24, Conor Dooley wrote:
>>> > On Fri, Nov 18, 2022 at 09:32:15PM +0800, Walker Chen wrote:
>>
>>> >> +void starfive_pmu_hw_event_turn_off(u32 mask)
>>> >> +{
>>> >> + pmu_writel(mask, HW_EVENT_TURN_OFF_MASK);
>>> >> +}
>>> >> +EXPORT_SYMBOL(starfive_pmu_hw_event_turn_off);
>>> >
>>> > Where are the users for these exports? Also, should they be exported as
>>> > GPL?
>>> >
>>> > Either way, what is the point of the extra layer of abstraction here
>>> > around the writel()?
>>>
>>> The two export functions are only prepared for GPU module. But accordint to
>>> the latest information, it seems that there is no open source plan for GPU.
>>> So the two functions will be drop in next version of patch.
>>
>> That's a shame!
>
>Need to comply with certain commercial terms.
>
>>
>>> >> +static int starfive_pmu_get_state(struct starfive_power_dev *pmd, bool *is_on)
>>> >> +{
>>> >> + struct starfive_pmu *pmu = pmd->power;
>>> >> +
>>> >> + if (!pmd->mask) {
>>> >> + *is_on = false;
>>> >> + return -EINVAL;
>>> >> + }
>>> >> +
>>> >> + *is_on = __raw_readl(pmu->base + CURR_POWER_MODE) & pmd->mask;
>>> >
>>> > Is there a specific reason that you are using the __raw variants here
>>> > (and elsewhere) in the driver?
>>>
>>> Will use unified function '__raw_readl' and '__raw_writel'
>>
>> No no, I want to know *why* you are using the __raw accessors here. My
>> understanding was that __raw variants are unbarriered & unordered with
>> respect to other io accesses.
>>
>> I do notice that the bcm driver you mentioned uses the __raw variants,
>> but only __raw variants - whereas you use readl() which is ordered and
>> barriered & __raw_readl().
>>
>> Is there a reason why you would not use readl() or readl_relaxed()?
>
>Your question led me to deeply understand the usage of these io accessors.
>__raw_readl / __raw_writel denotes native byte order, no memory barrier.
>readl / writel do guarantee the byte order with barrier, ensure that previous writes are done.
>Seem that non-raw accessors are more safe.

Yeah, if there's no good reason to use these "raw" versions then please use readl/readl_relaxed.


>> No worries, looking forward to getting my board :)
>>
>Have you purchased a VisionFive 2 board online?

I have :)

\
 
 \ /
  Last update: 2022-12-01 07:22    [W:0.049 / U:1.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site