Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only

From: Alison Schofield
Date: Tue Mar 14 2023 - 11:15:22 EST

Next message: Rob Herring: "Re: [PATCH] arm64: dts: qcom: sa8775p: add symbols to dtb"
Previous message: Konrad Dybcio: "Re: [PATCH net-next 05/11] clk: qcom: gcc-sc8280xp: Add EMAC GDSCs"
In reply to: Nathan Chancellor: "Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only"
Next in thread: Fabio M. De Francesco: "Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Mar 14, 2023 at 07:42:07AM -0700, Nathan Chancellor wrote:
> Hi Khadija,
>
> On Tue, Mar 14, 2023 at 07:08:31PM +0500, Khadija Kamran wrote:
> > On Tue, Mar 14, 2023 at 11:45:51AM +0800, kernel test robot wrote:
> > > Hi Khadija,
> > >
> > > Thank you for the patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on staging/staging-testing]
> > >
> > > url: https://github.com/intel-lab-lkp/linux/commits/Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230314-020827
> > > patch link: https://lore.kernel.org/r/ZA9mThZ7NyRrQAMX%40khadija-virtual-machine
> > > patch subject: [PATCH] staging: axis-fifo: initialize timeouts in probe only
> > > config: arm64-randconfig-r012-20230313 (https://download.01.org/0day-ci/archive/20230314/202303141159.6wN9HNP9-lkp@xxxxxxxxx/config)
> > > compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
> > > 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
> > > # install arm64 cross compiling tool for clang build
> > > # apt-get install binutils-aarch64-linux-gnu
> > > # https://github.com/intel-lab-lkp/linux/commit/9d186f6c9f9bf467b48da3e28b0e9aa31fc3faf3
> > > git remote add linux-review https://github.com/intel-lab-lkp/linux
> > > git fetch --no-tags linux-review Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230314-020827
> > > git checkout 9d186f6c9f9bf467b48da3e28b0e9aa31fc3faf3
> > > # save the config file
> > > mkdir build_dir && cp config build_dir/.config
> > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
> > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/staging/axis-fifo/
> > >
> > > If you fix the issue, kindly add following tag where applicable
> > > | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > | Link: https://lore.kernel.org/oe-kbuild-all/202303141159.6wN9HNP9-lkp@xxxxxxxxx/
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > >> drivers/staging/axis-fifo/axis-fifo.c:817:18: warning: implicit conversion from 'long' to 'int' changes value from 9223372036854775807 to -1 [-Wconstant-conversion]
> > > read_timeout = MAX_SCHEDULE_TIMEOUT;
> > > ~ ^~~~~~~~~~~~~~~~~~~~
> > > include/linux/sched.h:296:31: note: expanded from macro 'MAX_SCHEDULE_TIMEOUT'
> > > #define MAX_SCHEDULE_TIMEOUT LONG_MAX
> > > ^~~~~~~~
> > > include/vdso/limits.h:11:19: note: expanded from macro 'LONG_MAX'
> > > #define LONG_MAX ((long)(~0UL >> 1))
> > > ^~~~~~~~~~~~~~~~~
> > > drivers/staging/axis-fifo/axis-fifo.c:822:19: warning: implicit conversion from 'long' to 'int' changes value from 9223372036854775807 to -1 [-Wconstant-conversion]
> > > write_timeout = MAX_SCHEDULE_TIMEOUT;
> > > ~ ^~~~~~~~~~~~~~~~~~~~
> > > include/linux/sched.h:296:31: note: expanded from macro 'MAX_SCHEDULE_TIMEOUT'
> > > #define MAX_SCHEDULE_TIMEOUT LONG_MAX
> > > ^~~~~~~~
> > > include/vdso/limits.h:11:19: note: expanded from macro 'LONG_MAX'
> > > #define LONG_MAX ((long)(~0UL >> 1))
> > > ^~~~~~~~~~~~~~~~~
> > > 2 warnings generated.
> > >
> >
> > Hi everyone!
> > Kindly let me know if I should look into these warnings.
> > Thank you!
>
> You should always avoid introducing new warnings whenever possible. In
> this case, it appears that read_timeout and write_timeout should be
> changed from 'int' to 'long' to account for the fact that
> MAX_SCHEDULE_TIMEOUT is being assigned to it directly now, versus being
> passed as a parameter to wait_event_interruptible_timeout(), which
> assigned it to 'long' anyways.
>
> If you have any other questions or need further help, let me know :)
>
> Cheers,
> Nathan

Hi Khadaji,

Echoing Nathan's feedback - yes, you need to correct these.

Do you see these warnings when you compiled? If yes, then your path to
fixing them is easier. Make sure it recompiles with no warnings.

If you don't see these warnings, then you need to follow the steps to
reproduce in the lkp email. You have to be able to 'see the warning'
make a change and 'see it go away'.

Alison

>
> > > vim +817 drivers/staging/axis-fifo/axis-fifo.c
> > >
> > > 805
> > > 806 static int axis_fifo_probe(struct platform_device *pdev)
> > > 807 {
> > > 808 struct resource *r_mem; /* IO mem resources */
> > > 809 struct device *dev = &pdev->dev; /* OS device (from device tree) */
> > > 810 struct axis_fifo *fifo = NULL;
> > > 811 char *device_name;
> > > 812 int rc = 0; /* error return value */
> > > 813
> > > 814 if (read_timeout >= 0)
> > > 815 read_timeout = msecs_to_jiffies(read_timeout);
> > > 816 else
> > > > 817 read_timeout = MAX_SCHEDULE_TIMEOUT;
> > > 818
> > > 819 if (write_timeout >= 0)
> > > 820 write_timeout = msecs_to_jiffies(write_timeout);
> > > 821 else
> > > 822 write_timeout = MAX_SCHEDULE_TIMEOUT;
> > > 823
> > > 824 /* ----------------------------
> > > 825 * init wrapper device
> > > 826 * ----------------------------
> > > 827 */
> > > 828
> > > 829 device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> > > 830 if (!device_name)
> > > 831 return -ENOMEM;
> > > 832
> > > 833 /* allocate device wrapper memory */
> > > 834 fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
> > > 835 if (!fifo)
> > > 836 return -ENOMEM;
> > > 837
> > > 838 dev_set_drvdata(dev, fifo);
> > > 839 fifo->dt_device = dev;
> > > 840
> > > 841 init_waitqueue_head(&fifo->read_queue);
> > > 842 init_waitqueue_head(&fifo->write_queue);
> > > 843
> > > 844 mutex_init(&fifo->read_lock);
> > > 845 mutex_init(&fifo->write_lock);
> > > 846
> > > 847 /* ----------------------------
> > > 848 * init device memory space
> > > 849 * ----------------------------
> > > 850 */
> > > 851
> > > 852 /* get iospace for the device */
> > > 853 r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > 854 if (!r_mem) {
> > > 855 dev_err(fifo->dt_device, "invalid address\n");
> > > 856 rc = -ENODEV;
> > > 857 goto err_initial;
> > > 858 }
> > > 859
> > > 860 /* request physical memory */
> > > 861 fifo->base_addr = devm_ioremap_resource(fifo->dt_device, r_mem);
> > > 862 if (IS_ERR(fifo->base_addr)) {
> > > 863 rc = PTR_ERR(fifo->base_addr);
> > > 864 goto err_initial;
> > > 865 }
> > > 866
> > > 867 dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
> > > 868
> > > 869 /* create unique device name */
> > > 870 snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
> > > 871 dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
> > > 872
> > > 873 /* ----------------------------
> > > 874 * init IP
> > > 875 * ----------------------------
> > > 876 */
> > > 877
> > > 878 rc = axis_fifo_parse_dt(fifo);
> > > 879 if (rc)
> > > 880 goto err_initial;
> > > 881
> > > 882 reset_ip_core(fifo);
> > > 883
> > > 884 /* ----------------------------
> > > 885 * init device interrupts
> > > 886 * ----------------------------
> > > 887 */
> > > 888
> > > 889 /* get IRQ resource */
> > > 890 rc = platform_get_irq(pdev, 0);
> > > 891 if (rc < 0)
> > > 892 goto err_initial;
> > > 893
> > > 894 /* request IRQ */
> > > 895 fifo->irq = rc;
> > > 896 rc = devm_request_irq(fifo->dt_device, fifo->irq, &axis_fifo_irq, 0,
> > > 897 DRIVER_NAME, fifo);
> > > 898 if (rc) {
> > > 899 dev_err(fifo->dt_device, "couldn't allocate interrupt %i\n",
> > > 900 fifo->irq);
> > > 901 goto err_initial;
> > > 902 }
> > > 903
> > > 904 /* ----------------------------
> > > 905 * init char device
> > > 906 * ----------------------------
> > > 907 */
> > > 908
> > > 909 /* create character device */
> > > 910 fifo->miscdev.fops = &fops;
> > > 911 fifo->miscdev.minor = MISC_DYNAMIC_MINOR;
> > > 912 fifo->miscdev.name = device_name;
> > > 913 fifo->miscdev.groups = axis_fifo_attrs_groups;
> > > 914 fifo->miscdev.parent = dev;
> > > 915 rc = misc_register(&fifo->miscdev);
> > > 916 if (rc < 0)
> > > 917 goto err_initial;
> > > 918
> > > 919 dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
> > > 920 &r_mem->start, &fifo->base_addr, fifo->irq);
> > > 921
> > > 922 return 0;
> > > 923
> > > 924 err_initial:
> > > 925 dev_set_drvdata(dev, NULL);
> > > 926 return rc;
> > > 927 }
> > > 928
> > >
> > > --
> > > 0-DAY CI Kernel Test Service
> > > https://github.com/intel/lkp-tests
> >
>

Next message: Rob Herring: "Re: [PATCH] arm64: dts: qcom: sa8775p: add symbols to dtb"
Previous message: Konrad Dybcio: "Re: [PATCH net-next 05/11] clk: qcom: gcc-sc8280xp: Add EMAC GDSCs"
In reply to: Nathan Chancellor: "Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only"
Next in thread: Fabio M. De Francesco: "Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]