| Message ID | 20250410174010.31588-8-quic_ptalari@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable QUPs and Serial on SA8255p Qualcomm platforms | expand |
Hi Praveen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on tty/tty-next tty/tty-linus robh/for-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.15-rc1 next-20250411]
[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/Praveen-Talari/opp-add-new-helper-API-dev_pm_opp_set_level/20250411-015310
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link: https://lore.kernel.org/r/20250410174010.31588-8-quic_ptalari%40quicinc.com
patch subject: [PATCH v1 7/9] serial: qcom-geni: move clock-rate logic to separate function
config: arm-randconfig-002-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120237.YIWM0gvQ-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120237.YIWM0gvQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504120237.YIWM0gvQ-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15:0,
from include/linux/node.h:18,
from include/linux/cpu.h:17,
from arch/arm/include/asm/cpu.h:11,
from arch/arm/include/asm/smp_plat.h:12,
from arch/arm/include/asm/irq_work.h:5,
from include/linux/irq_work.h:64,
from include/linux/console.h:19,
from drivers/tty/serial/qcom_geni_serial.c:8:
drivers/tty/serial/qcom_geni_serial.c: In function 'geni_serial_set_rate':
>> drivers/tty/serial/qcom_geni_serial.c:1289:4: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
"Couldn't find suitable clock rate for %u\n",
^
include/linux/dev_printk.h:110:16: note: in definition of macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:154:49: note: in expansion of macro 'dev_fmt'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1288:3: note: in expansion of macro 'dev_err'
dev_err(port->se.dev,
^~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1294:24: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'long unsigned int' [-Wformat=]
dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n",
^
include/linux/dev_printk.h:139:28: note: in definition of macro 'dev_no_printk'
_dev_printk(level, dev, fmt, ##__VA_ARGS__); \
^~~
include/linux/dev_printk.h:171:33: note: in expansion of macro 'dev_fmt'
dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1294:2: note: in expansion of macro 'dev_dbg'
dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n",
^~~~~~~
drivers/tty/serial/qcom_geni_serial.c: In function 'qcom_geni_serial_pm':
drivers/tty/serial/qcom_geni_serial.c:1666:32: warning: unused variable 'port' [-Wunused-variable]
struct qcom_geni_serial_port *port = to_dev_port(uport);
^~~~
vim +1289 drivers/tty/serial/qcom_geni_serial.c
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1269
68765ed6fdd109 Praveen Talari 2025-04-10 1270 static int geni_serial_set_rate(struct uart_port *uport, unsigned long baud)
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1271 {
00ce7c6e86b5d1 Bartosz Golaszewski 2022-12-29 1272 struct qcom_geni_serial_port *port = to_dev_port(uport);
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1273 unsigned long clk_rate;
7cf563b2c84624 Akash Asthana 2020-06-23 1274 unsigned int avg_bw_core;
68765ed6fdd109 Praveen Talari 2025-04-10 1275 unsigned int clk_div;
68765ed6fdd109 Praveen Talari 2025-04-10 1276 u32 ver, sampling_rate;
68765ed6fdd109 Praveen Talari 2025-04-10 1277 u32 ser_clk_cfg;
ce734600545fc7 Vivek Gautam 2019-08-01 1278
ce734600545fc7 Vivek Gautam 2019-08-01 1279 sampling_rate = UART_OVERSAMPLING;
ce734600545fc7 Vivek Gautam 2019-08-01 1280 /* Sampling rate is halved for IP versions >= 2.5 */
ce734600545fc7 Vivek Gautam 2019-08-01 1281 ver = geni_se_get_qup_hw_version(&port->se);
c9ca43d42ed8d5 Paras Sharma 2020-09-30 1282 if (ver >= QUP_SE_VERSION_2_5)
ce734600545fc7 Vivek Gautam 2019-08-01 1283 sampling_rate /= 2;
ce734600545fc7 Vivek Gautam 2019-08-01 1284
c2194bc999d41e Vijaya Krishna Nivarthi 2022-05-16 1285 clk_rate = get_clk_div_rate(port->se.clk, baud,
c2194bc999d41e Vijaya Krishna Nivarthi 2022-05-16 1286 sampling_rate, &clk_div);
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1287 if (!clk_rate) {
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1288 dev_err(port->se.dev,
0fec518018cc5c Douglas Anderson 2022-08-02 @1289 "Couldn't find suitable clock rate for %u\n",
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1290 baud * sampling_rate);
68765ed6fdd109 Praveen Talari 2025-04-10 1291 return -EINVAL;
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1292 }
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1293
18536cc8fab81f Johan Hovold 2023-07-14 1294 dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n",
c474c775716edd Vijaya Krishna Nivarthi 2022-07-16 1295 baud * sampling_rate, clk_rate, clk_div);
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1296
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1297 uport->uartclk = clk_rate;
8ece7b754bc34f Johan Hovold 2023-07-14 1298 port->clk_rate = clk_rate;
a5819b548af0cc Rajendra Nayak 2020-06-15 1299 dev_pm_opp_set_rate(uport->dev, clk_rate);
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1300 ser_clk_cfg = SER_CLK_EN;
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1301 ser_clk_cfg |= clk_div << CLK_DIV_SHFT;
c4f528795d1add Karthikeyan Ramasubramanian 2018-03-14 1302
7cf563b2c84624 Akash Asthana 2020-06-23 1303 /*
7cf563b2c84624 Akash Asthana 2020-06-23 1304 * Bump up BW vote on CPU and CORE path as driver supports FIFO mode
7cf563b2c84624 Akash Asthana 2020-06-23 1305 * only.
7cf563b2c84624 Akash Asthana 2020-06-23 1306 */
7cf563b2c84624 Akash Asthana 2020-06-23 1307 avg_bw_core = (baud > 115200) ? Bps_to_icc(CORE_2X_50_MHZ)
7cf563b2c84624 Akash Asthana 2020-06-23 1308 : GENI_DEFAULT_BW;
7cf563b2c84624 Akash Asthana 2020-06-23 1309 port->se.icc_paths[GENI_TO_CORE].avg_bw = avg_bw_core;
7cf563b2c84624 Akash Asthana 2020-06-23 1310 port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud);
7cf563b2c84624 Akash Asthana 2020-06-23 1311 geni_icc_set_bw(&port->se);
7cf563b2c84624 Akash Asthana 2020-06-23 1312
68765ed6fdd109 Praveen Talari 2025-04-10 1313 writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
68765ed6fdd109 Praveen Talari 2025-04-10 1314 writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
68765ed6fdd109 Praveen Talari 2025-04-10 1315 return 0;
68765ed6fdd109 Praveen Talari 2025-04-10 1316 }
68765ed6fdd109 Praveen Talari 2025-04-10 1317
On 10. 04. 25, 19:40, Praveen Talari wrote: > Facilitates future modifications within the new function, > leading to better readability and maintainability of the code. > > Move the code that handles the actual logic of clock-rate > calculations to a separate function geni_serial_set_rate() > which enhances code readability. > > Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com> > --- > drivers/tty/serial/qcom_geni_serial.c | 56 +++++++++++++++++---------- > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index e341f5090ecc..25d16ac3f406 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c ... > @@ -1323,6 +1310,37 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, > port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud); > geni_icc_set_bw(&port->se); > > + writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG); > + writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG); > + return 0; Did this pass checkpatch? > +} > + > +static void qcom_geni_serial_set_termios(struct uart_port *uport, > + struct ktermios *termios, > + const struct ktermios *old) > +{ > + struct qcom_geni_serial_port *port = to_dev_port(uport); > + unsigned int baud; > + unsigned long timeout; > + u32 bits_per_char; > + u32 tx_trans_cfg; > + u32 tx_parity_cfg; > + u32 rx_trans_cfg; > + u32 rx_parity_cfg; > + u32 stop_bit_len; > + int ret = 0; > + > + /* baud rate */ > + baud = uart_get_baud_rate(uport, termios, old, 300, 4000000); > + > + ret = geni_serial_set_rate(uport, baud); > + if (ret) { > + dev_err(port->se.dev, > + "%s: Failed to set baud: %u ret: %d\n", Why the doubled spaces? > + __func__, baud, ret); > + return; > + } > + > /* parity */ > tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG); > tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG); thanks,
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index e341f5090ecc..25d16ac3f406 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1267,27 +1267,14 @@ static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, return ser_clk; } -static void qcom_geni_serial_set_termios(struct uart_port *uport, - struct ktermios *termios, - const struct ktermios *old) +static int geni_serial_set_rate(struct uart_port *uport, unsigned long baud) { - unsigned int baud; - u32 bits_per_char; - u32 tx_trans_cfg; - u32 tx_parity_cfg; - u32 rx_trans_cfg; - u32 rx_parity_cfg; - u32 stop_bit_len; - unsigned int clk_div; - u32 ser_clk_cfg; struct qcom_geni_serial_port *port = to_dev_port(uport); unsigned long clk_rate; - u32 ver, sampling_rate; unsigned int avg_bw_core; - unsigned long timeout; - - /* baud rate */ - baud = uart_get_baud_rate(uport, termios, old, 300, 4000000); + unsigned int clk_div; + u32 ver, sampling_rate; + u32 ser_clk_cfg; sampling_rate = UART_OVERSAMPLING; /* Sampling rate is halved for IP versions >= 2.5 */ @@ -1301,7 +1288,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, dev_err(port->se.dev, "Couldn't find suitable clock rate for %u\n", baud * sampling_rate); - return; + return -EINVAL; } dev_dbg(port->se.dev, "desired_rate = %u, clk_rate = %lu, clk_div = %u\n", @@ -1323,6 +1310,37 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, port->se.icc_paths[CPU_TO_GENI].avg_bw = Bps_to_icc(baud); geni_icc_set_bw(&port->se); + writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG); + writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG); + return 0; +} + +static void qcom_geni_serial_set_termios(struct uart_port *uport, + struct ktermios *termios, + const struct ktermios *old) +{ + struct qcom_geni_serial_port *port = to_dev_port(uport); + unsigned int baud; + unsigned long timeout; + u32 bits_per_char; + u32 tx_trans_cfg; + u32 tx_parity_cfg; + u32 rx_trans_cfg; + u32 rx_parity_cfg; + u32 stop_bit_len; + int ret = 0; + + /* baud rate */ + baud = uart_get_baud_rate(uport, termios, old, 300, 4000000); + + ret = geni_serial_set_rate(uport, baud); + if (ret) { + dev_err(port->se.dev, + "%s: Failed to set baud: %u ret: %d\n", + __func__, baud, ret); + return; + } + /* parity */ tx_trans_cfg = readl(uport->membase + SE_UART_TX_TRANS_CFG); tx_parity_cfg = readl(uport->membase + SE_UART_TX_PARITY_CFG); @@ -1390,8 +1408,6 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, writel(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN); writel(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN); writel(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN); - writel(ser_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG); - writel(ser_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG); } #ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
Facilitates future modifications within the new function, leading to better readability and maintainability of the code. Move the code that handles the actual logic of clock-rate calculations to a separate function geni_serial_set_rate() which enhances code readability. Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com> --- drivers/tty/serial/qcom_geni_serial.c | 56 +++++++++++++++++---------- 1 file changed, 36 insertions(+), 20 deletions(-)