Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <50e8f1e8-806a-3599-7cbe-0c7d4bec1c51@i2se.com>
Date:   Wed, 14 Sep 2022 19:45:48 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v1 2/7] clk: bcm: rpi: Add a function to retrieve the
 maximum
Content-Language: en-US
To:     Stephen Boyd <sboyd@kernel.org>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>, Emma Anholt <emma@anholt.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Maxime Ripard <maxime@cerno.tech>,
        Maxime Ripard <mripard@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Ray Jui <rjui@broadcom.com>,
        Scott Branden <sbranden@broadcom.com>
Cc:     linux-arm-kernel@lists.infradead.org,
        linux-rpi-kernel@lists.infradead.org,
        dri-devel@lists.freedesktop.org, Dom Cobley <popcornmix@gmail.com>,
        linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220815-rpi-fix-4k-60-v1-0-c52bd642f7c6@cerno.tech>
 <20220815-rpi-fix-4k-60-v1-2-c52bd642f7c6@cerno.tech>
 <20220914155035.88E45C433C1@smtp.kernel.org>
From:   Stefan Wahren <stefan.wahren@i2se.com>
In-Reply-To: <20220914155035.88E45C433C1@smtp.kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

Am 14.09.22 um 17:50 schrieb Stephen Boyd:
> Quoting Maxime Ripard (2022-08-15 08:31:24)
>> @@ -254,6 +255,33 @@ static int raspberrypi_fw_dumb_determine_rate(struct clk_hw *hw,
>>          return 0;
>>   }
>>   
>> +unsigned long rpi_firmware_clk_get_max_rate(struct clk *clk)
>> +{
>> +       const struct raspberrypi_clk_data *data;
>> +       struct raspberrypi_clk *rpi;
>> +       struct clk_hw *hw;
>> +       u32 max_rate;
>> +       int ret;
>> +
>> +       if (!clk)
>> +               return 0;
>> +
>> +       hw =  __clk_get_hw(clk);
> Ideally we don't add more users of this API. I should document that :/
>
> It begs the question though, why do we need this API to take a 'struct
> clk'?  Can it simply hardcode the data->id value for the clk you care
> about and call rpi_firmware_property() directly (or some wrapper of it)?
>
> Furthermore, I wonder if even that part needs to be implemented.  Why
> not make a direct call to rpi_firmware_property() and get the max rate?
> All of that can live in the drm driver. Making it a generic API that
> takes a 'struct clk' means that it looks like any clk can be passed,
> when that isn't true. It would be better to restrict it to the one use
> case so that the scope of the problem doesn't grow. I understand that it
> duplicates a few lines of code, but that looks like a fair tradeoff vs.
> exposing an API that can be used for other clks in the future.
it would be nice to keep all the Rpi specific stuff out of the DRM 
driver, since there more users of it.
>
>> +       if (!hw)
>> +               return 0;
>> +
>> +       data = clk_hw_to_data(hw);
>> +       rpi = data->rpi;
>> +       ret = raspberrypi_clock_property(rpi->firmware, data,
>> +                                        RPI_FIRMWARE_GET_MAX_CLOCK_RATE,
>> +                                        &max_rate);
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel