Re: [PATCH 1/2] serial: mtk: use ulong for clk_rate

On 7/28/26 10:57 AM, Julien Stephan wrote:
> Le mar. 28 juil. 2026 à 01:25, David Lechner <[email protected]> a écrit :
>>
>> From: Suhrid Subramaniam <[email protected]>
>>
>> Use ulong for return value of clk_get_rate() in the MediaTek serial
>> driver.
>>
>> IS_ERR_VALUE() does a signed 64-bit comparison against the range of
>> possible error codes. If clk_get_rate() returns an error, assigning this
>> to a u32 truncates the top 32 bits making the value smaller, defeating
>> IS_ERR_VALUE() and producing a garbage divisor that hangs the UART.
>>
>> Fixes: 3b17f2e2c2a9 ("serial: mtk: add support for using dynamic baud clock souce")
>> Signed-off-by: Suhrid Subramaniam <[email protected]>
>> Signed-off-by: David Lechner <[email protected]>
>> ---
>>  drivers/serial/serial_mtk.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/serial/serial_mtk.c b/drivers/serial/serial_mtk.c
>> index 01cc415efdd..64b3287f8eb 100644
>> --- a/drivers/serial/serial_mtk.c
>> +++ b/drivers/serial/serial_mtk.c
>> @@ -213,7 +213,7 @@ static int _mtk_serial_pending(struct mtk_serial_priv *priv, bool input)
>>  static int mtk_serial_setbrg(struct udevice *dev, int baudrate)
>>  {
>>         struct mtk_serial_priv *priv = dev_get_priv(dev);
>> -       u32 clk_rate;
>> +       ulong clk_rate;
>>
>>         clk_rate = clk_get_rate(&priv->clk);
>>         if (IS_ERR_VALUE(clk_rate) || clk_rate == 0)
>> @@ -266,6 +266,7 @@ static int mtk_serial_of_to_plat(struct udevice *dev)
>>  {
>>         struct mtk_serial_priv *priv = dev_get_priv(dev);
>>         fdt_addr_t addr;
>> +       ulong clk_rate;
>>         int err;
>>
>>         addr = dev_read_addr(dev);
>> @@ -282,10 +283,10 @@ static int mtk_serial_of_to_plat(struct udevice *dev)
>>                         return -EINVAL;
>>                 }
>>         } else {
>> -               err = clk_get_rate(&priv->clk);
>> -               if (IS_ERR_VALUE(err)) {
>> +               clk_rate = clk_get_rate(&priv->clk);
>> +               if (IS_ERR_VALUE(clk_rate)) {
>>                         dev_err(dev, "invalid baud clock\n");
>> -                       return -EINVAL;
>> +                       return (int)clk_rate;
> 
> Hello David,
> 
> This is a functional changes, maybe deserves a separate patch?

I checked all users and nothing check for specific return values.
Everything just logs the error or only cares about pass/fail. So
I don't consider it that significant of a change. Maybe I should
have mentioned it in the commit message though.

> 
> Cheers
> Julien
> 
>>                 }
>>         }
>>
>>
>> --
>> 2.43.0
>>