Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[kernel test robot]

Hi Yinbo,

I love your patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
        git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
      79 |         val = readq(loongson2_pll_base + offset);
         |               ^~~~~
         |               readl
   cc1: some warnings being treated as errors


vim +79 drivers/clk/clk-loongson2.c

    73	
    74	static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate)
    75	{
    76		u64 val;
    77		u32 mult = 1, div = 1;
    78	
  > 79		val = readq(loongson2_pll_base + offset);
    80	
    81		mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
    82				clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
    83		div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
    84				clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
    85	
    86		return div_u64((u64)rate * mult, div);
    87	}
    88	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[zhuyinbo]

在 2023/3/8 下午8:16, kernel test robot 写道:
> Hi Yinbo,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on clk/clk-next]
> [also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
> config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
> compiler: mips-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>        79 |         val = readq(loongson2_pll_base + offset);
>           |               ^~~~~
>           |               readl
>     cc1: some warnings being treated as errors

The CONFIG_64BIT not enabled in your config file, I will add a depend on 
"CONFIG_64BIT" in my clock driver to fix this compile error.

>
>
> vim +79 drivers/clk/clk-loongson2.c
>
>      73	
>      74	static unsigned long loongson2_calc_pll_rate(int offset, unsigned long rate)
>      75	{
>      76		u64 val;
>      77		u32 mult = 1, div = 1;
>      78	
>    > 79		val = readq(loongson2_pll_base + offset);
>      80	
>      81		mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
>      82				clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
>      83		div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
>      84				clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
>      85	
>      86		return div_u64((u64)rate * mult, div);
>      87	}
>      88	
>

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[zhuyinbo]

在 2023/3/9 上午10:58, zhuyinbo 写道:
>
> 在 2023/3/8 下午8:16, kernel test robot 写道:
>> Hi Yinbo,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on clk/clk-next]
>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 
>> next-20230308]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url: 
>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
>> clk-next
>> patch link: 
>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock 
>> controller driver support
>> config: mips-allyesconfig 
>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>> compiler: mips-linux-gcc (GCC) 12.1.0
>> reproduce (this is a W=1 build):
>>          wget 
>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>> -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # 
>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>          git remote add linux-review 
>> https://github.com/intel-lab-lkp/linux
>>          git fetch --no-tags linux-review 
>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Link: 
>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of 
>>>> function 'readq'; did you mean 'readl'? 
>>>> [-Werror=implicit-function-declaration]
>>        79 |         val = readq(loongson2_pll_base + offset);
>>           |               ^~~~~
>>           |               readl
>>     cc1: some warnings being treated as errors
>
> The CONFIG_64BIT not enabled in your config file, I will add a depend 
> on "CONFIG_64BIT" in my clock driver to fix this compile error.
My clock is for LoongArch platform, The LOONGARCH had select 
"CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to 
fix this compile error.
>
>>
>>
>> vim +79 drivers/clk/clk-loongson2.c
>>
>>      73
>>      74    static unsigned long loongson2_calc_pll_rate(int offset, 
>> unsigned long rate)
>>      75    {
>>      76        u64 val;
>>      77        u32 mult = 1, div = 1;
>>      78
>>    > 79        val = readq(loongson2_pll_base + offset);
>>      80
>>      81        mult = (val >> LOONGSON2_PLL_MULT_SHIFT) &
>>      82                clk_div_mask(LOONGSON2_PLL_MULT_WIDTH);
>>      83        div = (val >> LOONGSON2_PLL_DIV_SHIFT) &
>>      84                clk_div_mask(LOONGSON2_PLL_DIV_WIDTH);
>>      85
>>      86        return div_u64((u64)rate * mult, div);
>>      87    }
>>      88
>>

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Krzysztof Kozlowski]

On 09/03/2023 03:58, zhuyinbo wrote:
> 
> 在 2023/3/8 下午8:16, kernel test robot 写道:
>> Hi Yinbo,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on clk/clk-next]
>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230308]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
>> patch link:    https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support
>> config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>> compiler: mips-linux-gcc (GCC) 12.1.0
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>>          git fetch --no-tags linux-review Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>
>> If you fix the issue, kindly add following tag where applicable
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Link: https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>        79 |         val = readq(loongson2_pll_base + offset);
>>           |               ^~~~~
>>           |               readl
>>     cc1: some warnings being treated as errors
> 
> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> "CONFIG_64BIT" in my clock driver to fix this compile error.

No, that's not how it should be fixed. Fix your code instead.



Best regards,
Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Krzysztof Kozlowski]

On 09/03/2023 04:18, zhuyinbo wrote:
> 
> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>
>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>> Hi Yinbo,
>>>
>>> I love your patch! Yet something to improve:
>>>
>>> [auto build test ERROR on clk/clk-next]
>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1 
>>> next-20230308]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url: 
>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
>>> clk-next
>>> patch link: 
>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock 
>>> controller driver support
>>> config: mips-allyesconfig 
>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>> reproduce (this is a W=1 build):
>>>          wget 
>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
>>> -O ~/bin/make.cross
>>>          chmod +x ~/bin/make.cross
>>>          # 
>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>          git remote add linux-review 
>>> https://github.com/intel-lab-lkp/linux
>>>          git fetch --no-tags linux-review 
>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>          git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>          # save the config file
>>>          mkdir build_dir && cp config build_dir/.config
>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 
>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Link: 
>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of 
>>>>> function 'readq'; did you mean 'readl'? 
>>>>> [-Werror=implicit-function-declaration]
>>>        79 |         val = readq(loongson2_pll_base + offset);
>>>           |               ^~~~~
>>>           |               readl
>>>     cc1: some warnings being treated as errors
>>
>> The CONFIG_64BIT not enabled in your config file, I will add a depend 
>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
> My clock is for LoongArch platform, The LOONGARCH had select 
> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to 
> fix this compile error.

No. Fix your code instead.

Best regards,
Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[zhuyinbo]

在 2023/3/9 下午2:09, Krzysztof Kozlowski 写道:
> On 09/03/2023 04:18, zhuyinbo wrote:
>> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>> Hi Yinbo,
>>>>
>>>> I love your patch! Yet something to improve:
>>>>
>>>> [auto build test ERROR on clk/clk-next]
>>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1
>>>> next-20230308]
>>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>>> And when submitting patch, we suggest to use '--base' as documented in
>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>
>>>> url:
>>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>>>> clk-next
>>>> patch link:
>>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock
>>>> controller driver support
>>>> config: mips-allyesconfig
>>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>>> reproduce (this is a W=1 build):
>>>>           wget
>>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>>>> -O ~/bin/make.cross
>>>>           chmod +x ~/bin/make.cross
>>>>           #
>>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>           git remote add linux-review
>>>> https://github.com/intel-lab-lkp/linux
>>>>           git fetch --no-tags linux-review
>>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>           git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>           # save the config file
>>>>           mkdir build_dir && cp config build_dir/.config
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>> | Link:
>>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of
>>>>>> function 'readq'; did you mean 'readl'?
>>>>>> [-Werror=implicit-function-declaration]
>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>            |               ^~~~~
>>>>            |               readl
>>>>      cc1: some warnings being treated as errors
>>> The CONFIG_64BIT not enabled in your config file, I will add a depend
>>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
>> My clock is for LoongArch platform, The LOONGARCH had select
>> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to
>> fix this compile error.
> No. Fix your code instead.

but the readq that ask CONFIG_64BIT is enabled,  definition in 
include/asm-generic/io.h

so need enable CONFIG_64BIT for my clk driver.


#ifdef CONFIG_64BIT
#ifndef readq
#define readq readq
static inline u64 readq(const volatile void __iomem *addr)
{
         u64 val;

         __io_br();
         val = __le64_to_cpu(__raw_readq(addr));
         __io_ar();
         return val;
}
#endif
#endif /* CONFIG_64BIT */

>
> Best regards,
> Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Krzysztof Kozlowski]

On 09/03/2023 07:27, zhuyinbo wrote:
> 
> 在 2023/3/9 下午2:09, Krzysztof Kozlowski 写道:
>> On 09/03/2023 04:18, zhuyinbo wrote:
>>> 在 2023/3/9 上午10:58, zhuyinbo 写道:
>>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>>> Hi Yinbo,
>>>>>
>>>>> I love your patch! Yet something to improve:
>>>>>
>>>>> [auto build test ERROR on clk/clk-next]
>>>>> [also build test ERROR on robh/for-next linus/master v6.3-rc1
>>>>> next-20230308]
>>>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>>>> And when submitting patch, we suggest to use '--base' as documented in
>>>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>>>
>>>>> url:
>>>>> https://github.com/intel-lab-lkp/linux/commits/Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
>>>>> clk-next
>>>>> patch link:
>>>>> https://lore.kernel.org/r/20230307115022.12846-2-zhuyinbo%40loongson.cn
>>>>> patch subject: [PATCH v13 2/2] clk: clk-loongson2: add clock
>>>>> controller driver support
>>>>> config: mips-allyesconfig
>>>>> (https://download.01.org/0day-ci/archive/20230308/202303082037.QPfBP64A-lkp@intel.com/config)
>>>>> compiler: mips-linux-gcc (GCC) 12.1.0
>>>>> reproduce (this is a W=1 build):
>>>>>           wget
>>>>> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
>>>>> -O ~/bin/make.cross
>>>>>           chmod +x ~/bin/make.cross
>>>>>           #
>>>>> https://github.com/intel-lab-lkp/linux/commit/391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>>           git remote add linux-review
>>>>> https://github.com/intel-lab-lkp/linux
>>>>>           git fetch --no-tags linux-review
>>>>> Yinbo-Zhu/clk-clk-loongson2-add-clock-controller-driver-support/20230307-195252
>>>>>           git checkout 391d6fc63ac65f5456e4755c9dd85232a6296285
>>>>>           # save the config file
>>>>>           mkdir build_dir && cp config build_dir/.config
>>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>>> make.cross W=1 O=build_dir ARCH=mips olddefconfig
>>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
>>>>> make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/
>>>>>
>>>>> If you fix the issue, kindly add following tag where applicable
>>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>>> | Link:
>>>>> https://lore.kernel.org/oe-kbuild-all/202303082037.QPfBP64A-lkp@intel.com/
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of
>>>>>>> function 'readq'; did you mean 'readl'?
>>>>>>> [-Werror=implicit-function-declaration]
>>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>>            |               ^~~~~
>>>>>            |               readl
>>>>>      cc1: some warnings being treated as errors
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend
>>>> on "CONFIG_64BIT" in my clock driver to fix this compile error.
>>> My clock is for LoongArch platform, The LOONGARCH had select
>>> "CONFIG_64BIT", I will add a depend on "LOONGARCH" in my clock driver to
>>> fix this compile error.
>> No. Fix your code instead.
> 
> but the readq that ask CONFIG_64BIT is enabled,  definition in 
> include/asm-generic/io.h
> 
> so need enable CONFIG_64BIT for my clk driver.

Ah, you are right, the error was not about the cast but missing readq.
Yeah, you should depend on 64BIT. Not on Loongarch because the code
should compile test on other archs.

Best regards,
Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Stephen Boyd]

Quoting zhuyinbo (2023-03-08 18:58:02)
> 
> 在 2023/3/8 下午8:16, kernel test robot 写道:
> > Hi Yinbo,
> >
[...]
> >
> >     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
> >>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
> >        79 |         val = readq(loongson2_pll_base + offset);
> >           |               ^~~~~
> >           |               readl
> >     cc1: some warnings being treated as errors
> 
> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> "CONFIG_64BIT" in my clock driver to fix this compile error.

Do you need to use readq() here? Can you read two 32-bit registers with
readl() and put them together for a 64-bit number?

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Krzysztof Kozlowski]

On 10/03/2023 00:47, Stephen Boyd wrote:
> Quoting zhuyinbo (2023-03-08 18:58:02)
>>
>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>> Hi Yinbo,
>>>
> [...]
>>>
>>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>>        79 |         val = readq(loongson2_pll_base + offset);
>>>           |               ^~~~~
>>>           |               readl
>>>     cc1: some warnings being treated as errors
>>
>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
>> "CONFIG_64BIT" in my clock driver to fix this compile error.
> 
> Do you need to use readq() here? Can you read two 32-bit registers with
> readl() and put them together for a 64-bit number?

If the platform supports 64-bit reads and these are actually one
register, then readq makes sense - code is more readable, smaller, more
efficient.

Best regards,
Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Stephen Boyd]

Quoting Krzysztof Kozlowski (2023-03-10 00:42:47)
> On 10/03/2023 00:47, Stephen Boyd wrote:
> > Quoting zhuyinbo (2023-03-08 18:58:02)
> >>
> >> 在 2023/3/8 下午8:16, kernel test robot 写道:
> >>> Hi Yinbo,
> >>>
> > [...]
> >>>
> >>>     drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
> >>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
> >>>        79 |         val = readq(loongson2_pll_base + offset);
> >>>           |               ^~~~~
> >>>           |               readl
> >>>     cc1: some warnings being treated as errors
> >>
> >> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> >> "CONFIG_64BIT" in my clock driver to fix this compile error.
> > 
> > Do you need to use readq() here? Can you read two 32-bit registers with
> > readl() and put them together for a 64-bit number?
> 
> If the platform supports 64-bit reads and these are actually one
> register, then readq makes sense - code is more readable, smaller, more
> efficient.
> 

Please read the section in Documentation/driver-api/device-io.rst about
hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
restrict the driver to CONFIG_64BIT. Instead, include one of these
header files to get the IO access primitives.

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[zhuyinbo]

在 2023/3/14 上午2:20, Stephen Boyd 写道:
> Quoting Krzysztof Kozlowski (2023-03-10 00:42:47)
>> On 10/03/2023 00:47, Stephen Boyd wrote:
>>> Quoting zhuyinbo (2023-03-08 18:58:02)
>>>> 在 2023/3/8 下午8:16, kernel test robot 写道:
>>>>> Hi Yinbo,
>>>>>
>>> [...]
>>>>>      drivers/clk/clk-loongson2.c: In function 'loongson2_calc_pll_rate':
>>>>>>> drivers/clk/clk-loongson2.c:79:15: error: implicit declaration of function 'readq'; did you mean 'readl'? [-Werror=implicit-function-declaration]
>>>>>         79 |         val = readq(loongson2_pll_base + offset);
>>>>>            |               ^~~~~
>>>>>            |               readl
>>>>>      cc1: some warnings being treated as errors
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on
>>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
>>> Do you need to use readq() here? Can you read two 32-bit registers with
>>> readl() and put them together for a 64-bit number?
>> If the platform supports 64-bit reads and these are actually one
>> register, then readq makes sense - code is more readable, smaller, more
>> efficient.
>>
> Please read the section in Documentation/driver-api/device-io.rst about
> hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> restrict the driver to CONFIG_64BIT. Instead, include one of these
> header files to get the IO access primitives.

okay, I got it.

Thanks your advice!

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Krzysztof Kozlowski]

On 13/03/2023 19:20, Stephen Boyd wrote:
>>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
>>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
>>>
>>> Do you need to use readq() here? Can you read two 32-bit registers with
>>> readl() and put them together for a 64-bit number?
>>
>> If the platform supports 64-bit reads and these are actually one
>> register, then readq makes sense - code is more readable, smaller, more
>> efficient.
>>
> 
> Please read the section in Documentation/driver-api/device-io.rst about
> hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> restrict the driver to CONFIG_64BIT. Instead, include one of these
> header files to get the IO access primitives.

These primitives are for 32bit access. Quoting: "on 32-bit
architectures". What's the point of them if the code *will never* run on
32-bit? It will be a fake choice of linux/io-64-nonatomic-lo-hi.h or
linux/io-64-nonatomic-hi-lo.h misleading users to think this was tested
on 32-bit.

Best regards,
Krzysztof

Re: [PATCH v13 2/2] clk: clk-loongson2: add clock controller driver support

[Stephen Boyd]

Quoting Krzysztof Kozlowski (2023-03-13 23:49:40)
> On 13/03/2023 19:20, Stephen Boyd wrote:
> >>>> The CONFIG_64BIT not enabled in your config file, I will add a depend on 
> >>>> "CONFIG_64BIT" in my clock driver to fix this compile error.
> >>>
> >>> Do you need to use readq() here? Can you read two 32-bit registers with
> >>> readl() and put them together for a 64-bit number?
> >>
> >> If the platform supports 64-bit reads and these are actually one
> >> register, then readq makes sense - code is more readable, smaller, more
> >> efficient.
> >>
> > 
> > Please read the section in Documentation/driver-api/device-io.rst about
> > hi_lo_readq() and <linux/io-64-nonatomic-lo-hi.h>. We shouldn't need to
> > restrict the driver to CONFIG_64BIT. Instead, include one of these
> > header files to get the IO access primitives.
> 
> These primitives are for 32bit access. Quoting: "on 32-bit
> architectures". What's the point of them if the code *will never* run on
> 32-bit?

They're there to make drivers portable.

> It will be a fake choice of linux/io-64-nonatomic-lo-hi.h or
> linux/io-64-nonatomic-hi-lo.h misleading users to think this was tested
> on 32-bit.
> 

I don't think anyone is really going to care that it hasn't been tested.
It's not like the Linux kernel driver is the source of truth for
integrating IP blocks into different architectures. If it's wrong
someone will fix it when they try to use the hardware on 32-bit systems.

Can the register handle being read/written with two 32-bit accesses? I
still don't think we've had any answer to that question. If so, pick the
one that makes the most sense and move on.

In Linux, we try to write portable drivers. This way anyone can compile
the driver on any host architecture with whatever compiler they're
using. Otherwise, they have to download a cross compiler for the target
architecture to simply build test the code. Also, the Linux kernel is
fairly portable. We try to limit architecture specific code to arch/ and
so anything in drivers/ is ideally portable code.