* [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path @ 2016-04-20 9:24 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Javier Martinez Canillas, Bartlomiej Zolnierkiewicz If during probe() the s3c24xx_i2c_init() failed, the clock was left in disabled but prepared state. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index 362a6de54833..e9658af36f2e 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -1200,6 +1200,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) clk_disable(i2c->clk); if (ret != 0) { dev_err(&pdev->dev, "I2C controller init failed\n"); + clk_unprepare(i2c->clk); return ret; } /* find the IRQ for this unit (note, this relies on the init call to -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path @ 2016-04-20 9:24 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: linux-arm-kernel If during probe() the s3c24xx_i2c_init() failed, the clock was left in disabled but prepared state. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index 362a6de54833..e9658af36f2e 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -1200,6 +1200,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) clk_disable(i2c->clk); if (ret != 0) { dev_err(&pdev->dev, "I2C controller init failed\n"); + clk_unprepare(i2c->clk); return ret; } /* find the IRQ for this unit (note, this relies on the init call to -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup 2016-04-20 9:24 ` Krzysztof Kozlowski @ 2016-04-20 9:24 ` Krzysztof Kozlowski -1 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Javier Martinez Canillas, Bartlomiej Zolnierkiewicz Cleanup the weird function-level comments and remove obvious documentatoin for probe/remove. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 110 ++++++++++++--------------------------- 1 file changed, 32 insertions(+), 78 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index e9658af36f2e..fd3695a921f1 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -163,11 +163,9 @@ static const struct of_device_id s3c24xx_i2c_match[] = { MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match); #endif -/* s3c24xx_get_device_quirks - * +/* * Get controller type either from device tree or platform device variant. -*/ - + */ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev) { if (pdev->dev.of_node) { @@ -179,12 +177,10 @@ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *p return platform_get_device_id(pdev)->driver_data; } -/* s3c24xx_i2c_master_complete - * - * complete the message and wake up the caller, using the given return code, +/* + * Complete the message and wake up the caller, using the given return code, * or zero to mean ok. -*/ - + */ static inline void s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret) { dev_dbg(i2c->dev, "master_complete %d\n", ret); @@ -217,7 +213,6 @@ static inline void s3c24xx_i2c_enable_ack(struct s3c24xx_i2c *i2c) } /* irq enable/disable functions */ - static inline void s3c24xx_i2c_disable_irq(struct s3c24xx_i2c *i2c) { unsigned long tmp; @@ -251,11 +246,9 @@ static bool is_ack(struct s3c24xx_i2c *i2c) return false; } -/* s3c24xx_i2c_message_start - * +/* * put the start of a message onto the bus -*/ - + */ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, struct i2c_msg *msg) { @@ -364,21 +357,17 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) /* helper functions to determine the current state in the set of * messages we are sending */ -/* is_lastmsg() - * +/* * returns TRUE if the current message is the last in the set -*/ - + */ static inline int is_lastmsg(struct s3c24xx_i2c *i2c) { return i2c->msg_idx >= (i2c->msg_num - 1); } -/* is_msglast - * +/* * returns TRUE if we this is the last byte in the current message -*/ - + */ static inline int is_msglast(struct s3c24xx_i2c *i2c) { /* msg->len is always 1 for the first byte of smbus block read. @@ -390,21 +379,17 @@ static inline int is_msglast(struct s3c24xx_i2c *i2c) return i2c->msg_ptr == i2c->msg->len-1; } -/* is_msgend - * +/* * returns TRUE if we reached the end of the current message -*/ - + */ static inline int is_msgend(struct s3c24xx_i2c *i2c) { return i2c->msg_ptr >= i2c->msg->len; } -/* i2c_s3c_irq_nextbyte - * +/* * process an interrupt and work out what to do */ - static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) { unsigned long tmp; @@ -568,11 +553,9 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) return ret; } -/* s3c24xx_i2c_irq - * +/* * top level IRQ servicing routine -*/ - + */ static irqreturn_t s3c24xx_i2c_irq(int irqno, void *dev_id) { struct s3c24xx_i2c *i2c = dev_id; @@ -630,11 +613,9 @@ static inline void s3c24xx_i2c_disable_bus(struct s3c24xx_i2c *i2c) } -/* s3c24xx_i2c_set_master - * +/* * get the i2c bus for a master transaction -*/ - + */ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c) { unsigned long iicstat; @@ -652,11 +633,9 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c) return -ETIMEDOUT; } -/* s3c24xx_i2c_wait_idle - * +/* * wait for the i2c bus to become idle. -*/ - + */ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c) { unsigned long iicstat; @@ -706,11 +685,9 @@ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c) dev_warn(i2c->dev, "timeout waiting for bus idle\n"); } -/* s3c24xx_i2c_doxfer - * +/* * this starts an i2c transfer -*/ - + */ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, struct i2c_msg *msgs, int num) { @@ -771,12 +748,10 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, return ret; } -/* s3c24xx_i2c_xfer - * +/* * first port of call from the i2c bus code when an message needs * transferring across the i2c bus. -*/ - + */ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) { @@ -814,17 +789,14 @@ static u32 s3c24xx_i2c_func(struct i2c_adapter *adap) } /* i2c bus registration info */ - static const struct i2c_algorithm s3c24xx_i2c_algorithm = { .master_xfer = s3c24xx_i2c_xfer, .functionality = s3c24xx_i2c_func, }; -/* s3c24xx_i2c_calcdivisor - * +/* * return the divisor settings for a given frequency -*/ - + */ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted, unsigned int *div1, unsigned int *divs) { @@ -850,13 +822,11 @@ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted, return clkin / (calc_divs * calc_div1); } -/* s3c24xx_i2c_clockrate - * +/* * work out a divisor for the user requested frequency setting, * either by the requested frequency, or scanning the acceptable * range of frequencies until something is found -*/ - + */ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got) { struct s3c2410_platform_i2c *pdata = i2c->pdata; @@ -1028,11 +998,9 @@ static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c) } #endif -/* s3c24xx_i2c_init - * +/* * initialise the controller, set the IO lines and frequency -*/ - + */ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) { struct s3c2410_platform_i2c *pdata; @@ -1068,11 +1036,9 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) } #ifdef CONFIG_OF -/* s3c24xx_i2c_parse_dt - * +/* * Parse the device tree node and retreive the platform data. -*/ - + */ static void s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) { @@ -1111,11 +1077,6 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) } #endif -/* s3c24xx_i2c_probe - * - * called by the bus driver when a suitable device is found -*/ - static int s3c24xx_i2c_probe(struct platform_device *pdev) { struct s3c24xx_i2c *i2c; @@ -1258,11 +1219,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return 0; } -/* s3c24xx_i2c_remove - * - * called when device is removed from the bus -*/ - static int s3c24xx_i2c_remove(struct platform_device *pdev) { struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev); @@ -1332,8 +1288,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = { #define S3C24XX_DEV_PM_OPS NULL #endif -/* device driver for platform bus bits */ - static struct platform_driver s3c24xx_i2c_driver = { .probe = s3c24xx_i2c_probe, .remove = s3c24xx_i2c_remove, -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup @ 2016-04-20 9:24 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: linux-arm-kernel Cleanup the weird function-level comments and remove obvious documentatoin for probe/remove. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 110 ++++++++++++--------------------------- 1 file changed, 32 insertions(+), 78 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index e9658af36f2e..fd3695a921f1 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -163,11 +163,9 @@ static const struct of_device_id s3c24xx_i2c_match[] = { MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match); #endif -/* s3c24xx_get_device_quirks - * +/* * Get controller type either from device tree or platform device variant. -*/ - + */ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev) { if (pdev->dev.of_node) { @@ -179,12 +177,10 @@ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *p return platform_get_device_id(pdev)->driver_data; } -/* s3c24xx_i2c_master_complete - * - * complete the message and wake up the caller, using the given return code, +/* + * Complete the message and wake up the caller, using the given return code, * or zero to mean ok. -*/ - + */ static inline void s3c24xx_i2c_master_complete(struct s3c24xx_i2c *i2c, int ret) { dev_dbg(i2c->dev, "master_complete %d\n", ret); @@ -217,7 +213,6 @@ static inline void s3c24xx_i2c_enable_ack(struct s3c24xx_i2c *i2c) } /* irq enable/disable functions */ - static inline void s3c24xx_i2c_disable_irq(struct s3c24xx_i2c *i2c) { unsigned long tmp; @@ -251,11 +246,9 @@ static bool is_ack(struct s3c24xx_i2c *i2c) return false; } -/* s3c24xx_i2c_message_start - * +/* * put the start of a message onto the bus -*/ - + */ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, struct i2c_msg *msg) { @@ -364,21 +357,17 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) /* helper functions to determine the current state in the set of * messages we are sending */ -/* is_lastmsg() - * +/* * returns TRUE if the current message is the last in the set -*/ - + */ static inline int is_lastmsg(struct s3c24xx_i2c *i2c) { return i2c->msg_idx >= (i2c->msg_num - 1); } -/* is_msglast - * +/* * returns TRUE if we this is the last byte in the current message -*/ - + */ static inline int is_msglast(struct s3c24xx_i2c *i2c) { /* msg->len is always 1 for the first byte of smbus block read. @@ -390,21 +379,17 @@ static inline int is_msglast(struct s3c24xx_i2c *i2c) return i2c->msg_ptr == i2c->msg->len-1; } -/* is_msgend - * +/* * returns TRUE if we reached the end of the current message -*/ - + */ static inline int is_msgend(struct s3c24xx_i2c *i2c) { return i2c->msg_ptr >= i2c->msg->len; } -/* i2c_s3c_irq_nextbyte - * +/* * process an interrupt and work out what to do */ - static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) { unsigned long tmp; @@ -568,11 +553,9 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) return ret; } -/* s3c24xx_i2c_irq - * +/* * top level IRQ servicing routine -*/ - + */ static irqreturn_t s3c24xx_i2c_irq(int irqno, void *dev_id) { struct s3c24xx_i2c *i2c = dev_id; @@ -630,11 +613,9 @@ static inline void s3c24xx_i2c_disable_bus(struct s3c24xx_i2c *i2c) } -/* s3c24xx_i2c_set_master - * +/* * get the i2c bus for a master transaction -*/ - + */ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c) { unsigned long iicstat; @@ -652,11 +633,9 @@ static int s3c24xx_i2c_set_master(struct s3c24xx_i2c *i2c) return -ETIMEDOUT; } -/* s3c24xx_i2c_wait_idle - * +/* * wait for the i2c bus to become idle. -*/ - + */ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c) { unsigned long iicstat; @@ -706,11 +685,9 @@ static void s3c24xx_i2c_wait_idle(struct s3c24xx_i2c *i2c) dev_warn(i2c->dev, "timeout waiting for bus idle\n"); } -/* s3c24xx_i2c_doxfer - * +/* * this starts an i2c transfer -*/ - + */ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, struct i2c_msg *msgs, int num) { @@ -771,12 +748,10 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, return ret; } -/* s3c24xx_i2c_xfer - * +/* * first port of call from the i2c bus code when an message needs * transferring across the i2c bus. -*/ - + */ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) { @@ -814,17 +789,14 @@ static u32 s3c24xx_i2c_func(struct i2c_adapter *adap) } /* i2c bus registration info */ - static const struct i2c_algorithm s3c24xx_i2c_algorithm = { .master_xfer = s3c24xx_i2c_xfer, .functionality = s3c24xx_i2c_func, }; -/* s3c24xx_i2c_calcdivisor - * +/* * return the divisor settings for a given frequency -*/ - + */ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted, unsigned int *div1, unsigned int *divs) { @@ -850,13 +822,11 @@ static int s3c24xx_i2c_calcdivisor(unsigned long clkin, unsigned int wanted, return clkin / (calc_divs * calc_div1); } -/* s3c24xx_i2c_clockrate - * +/* * work out a divisor for the user requested frequency setting, * either by the requested frequency, or scanning the acceptable * range of frequencies until something is found -*/ - + */ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got) { struct s3c2410_platform_i2c *pdata = i2c->pdata; @@ -1028,11 +998,9 @@ static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c) } #endif -/* s3c24xx_i2c_init - * +/* * initialise the controller, set the IO lines and frequency -*/ - + */ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) { struct s3c2410_platform_i2c *pdata; @@ -1068,11 +1036,9 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) } #ifdef CONFIG_OF -/* s3c24xx_i2c_parse_dt - * +/* * Parse the device tree node and retreive the platform data. -*/ - + */ static void s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) { @@ -1111,11 +1077,6 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) } #endif -/* s3c24xx_i2c_probe - * - * called by the bus driver when a suitable device is found -*/ - static int s3c24xx_i2c_probe(struct platform_device *pdev) { struct s3c24xx_i2c *i2c; @@ -1258,11 +1219,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return 0; } -/* s3c24xx_i2c_remove - * - * called when device is removed from the bus -*/ - static int s3c24xx_i2c_remove(struct platform_device *pdev) { struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev); @@ -1332,8 +1288,6 @@ static const struct dev_pm_ops s3c24xx_i2c_dev_pm_ops = { #define S3C24XX_DEV_PM_OPS NULL #endif -/* device driver for platform bus bits */ - static struct platform_driver s3c24xx_i2c_driver = { .probe = s3c24xx_i2c_probe, .remove = s3c24xx_i2c_remove, -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup 2016-04-20 9:24 ` Krzysztof Kozlowski @ 2016-04-20 13:59 ` Javier Martinez Canillas -1 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 13:59 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Bartlomiej Zolnierkiewicz Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > Cleanup the weird function-level comments and remove obvious > documentatoin for probe/remove. > s/documentatoin/documentation > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- I wonder if instead we should change to kernel-doc formatted documentation. All the functions are static so kernel-doc is not really required but still Documentation/kernel-doc-nano-HOWTO.txt says that is suggested for source code layout consistency. But I agree that's either kernel-doc format or minimal as your patch does: Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup @ 2016-04-20 13:59 ` Javier Martinez Canillas 0 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 13:59 UTC (permalink / raw) To: linux-arm-kernel Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > Cleanup the weird function-level comments and remove obvious > documentatoin for probe/remove. > s/documentatoin/documentation > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- I wonder if instead we should change to kernel-doc formatted documentation. All the functions are static so kernel-doc is not really required but still Documentation/kernel-doc-nano-HOWTO.txt says that is suggested for source code layout consistency. But I agree that's either kernel-doc format or minimal as your patch does: Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup 2016-04-20 13:59 ` Javier Martinez Canillas @ 2016-04-21 7:01 ` Krzysztof Kozlowski -1 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-21 7:01 UTC (permalink / raw) To: Javier Martinez Canillas, Kukjin Kim, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Bartlomiej Zolnierkiewicz On 04/20/2016 03:59 PM, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: >> Cleanup the weird function-level comments and remove obvious >> documentatoin for probe/remove. >> > > s/documentatoin/documentation > >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> --- > > I wonder if instead we should change to kernel-doc formatted documentation. > > All the functions are static so kernel-doc is not really required but still > Documentation/kernel-doc-nano-HOWTO.txt says that is suggested for source > code layout consistency. > > But I agree that's either kernel-doc format or minimal as your patch does: > > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Thanks for review. Indeed switching to kernel-doc would be the best... but that would require a little bit more rewritting than now. I just wanted to fix some weird looking comments. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup @ 2016-04-21 7:01 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-21 7:01 UTC (permalink / raw) To: linux-arm-kernel On 04/20/2016 03:59 PM, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: >> Cleanup the weird function-level comments and remove obvious >> documentatoin for probe/remove. >> > > s/documentatoin/documentation > >> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> >> --- > > I wonder if instead we should change to kernel-doc formatted documentation. > > All the functions are static so kernel-doc is not really required but still > Documentation/kernel-doc-nano-HOWTO.txt says that is suggested for source > code layout consistency. > > But I agree that's either kernel-doc format or minimal as your patch does: > > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Thanks for review. Indeed switching to kernel-doc would be the best... but that would require a little bit more rewritting than now. I just wanted to fix some weird looking comments. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] i2c: s3c2410: Cleanup indentation and comment style 2016-04-20 9:24 ` Krzysztof Kozlowski @ 2016-04-20 9:24 ` Krzysztof Kozlowski -1 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Javier Martinez Canillas, Bartlomiej Zolnierkiewicz Improve the readability by: - fixing indentation, - switching to proper block comments, - removing spurious blank lines, - checkpatch: void function return statements are not generally useful - checkpatch: braces {} are not necessary for any arm of this statement - checkpatch: missing a blank line after declarations Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 113 ++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index fd3695a921f1..70a9f0c1ebca 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -170,6 +170,7 @@ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *p { if (pdev->dev.of_node) { const struct of_device_id *match; + match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node); return (kernel_ulong_t)match->data; } @@ -277,9 +278,10 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, dev_dbg(i2c->dev, "START: %08lx to IICSTAT, %02x to DS\n", stat, addr); writeb(addr, i2c->regs + S3C2410_IICDS); - /* delay here to ensure the data byte has gotten onto the bus - * before the transaction is started */ - + /* + * delay here to ensure the data byte has gotten onto the bus + * before the transaction is started + */ ndelay(i2c->tx_setup); dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); @@ -354,8 +356,10 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) s3c24xx_i2c_disable_irq(i2c); } -/* helper functions to determine the current state in the set of - * messages we are sending */ +/* + * helper functions to determine the current state in the set of + * messages we are sending + */ /* * returns TRUE if the current message is the last in the set @@ -370,9 +374,11 @@ static inline int is_lastmsg(struct s3c24xx_i2c *i2c) */ static inline int is_msglast(struct s3c24xx_i2c *i2c) { - /* msg->len is always 1 for the first byte of smbus block read. + /* + * msg->len is always 1 for the first byte of smbus block read. * Actual length will be read from slave. More bytes will be - * read according to the length then. */ + * read according to the length then. + */ if (i2c->msg->flags & I2C_M_RECV_LEN && i2c->msg->len == 1) return 0; @@ -408,14 +414,13 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) goto out_ack; case STATE_START: - /* last thing we did was send a start condition on the + /* + * last thing we did was send a start condition on the * bus, or started a new i2c message */ - if (iicstat & S3C2410_IICSTAT_LASTBIT && !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { /* ack was not received... */ - dev_dbg(i2c->dev, "ack was not received\n"); s3c24xx_i2c_stop(i2c, -ENXIO); goto out_ack; @@ -426,9 +431,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) else i2c->state = STATE_WRITE; - /* terminate the transfer if there is nothing to do - * as this is used by the i2c probe to find devices. */ - + /* + * Terminate the transfer if there is nothing to do + * as this is used by the i2c probe to find devices. + */ if (is_lastmsg(i2c) && i2c->msg->len == 0) { s3c24xx_i2c_stop(i2c, 0); goto out_ack; @@ -437,14 +443,16 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) if (i2c->state == STATE_READ) goto prepare_read; - /* fall through to the write state, as we will need to - * send a byte as well */ + /* + * fall through to the write state, as we will need to + * send a byte as well + */ case STATE_WRITE: - /* we are writing data to the device... check for the + /* + * we are writing data to the device... check for the * end of the message, and if so, work out what to do */ - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { if (iicstat & S3C2410_IICSTAT_LASTBIT) { dev_dbg(i2c->dev, "WRITE: No Ack\n"); @@ -460,12 +468,13 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) byte = i2c->msg->buf[i2c->msg_ptr++]; writeb(byte, i2c->regs + S3C2410_IICDS); - /* delay after writing the byte to allow the + /* + * delay after writing the byte to allow the * data setup time on the bus, as writing the * data to the register causes the first bit * to appear on SDA, and SCL will change as - * soon as the interrupt is acknowledged */ - + * soon as the interrupt is acknowledged + */ ndelay(i2c->tx_setup); } else if (!is_lastmsg(i2c)) { @@ -481,10 +490,11 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) if (i2c->msg->flags & I2C_M_NOSTART) { if (i2c->msg->flags & I2C_M_RD) { - /* cannot do this, the controller + /* + * cannot do this, the controller * forces us to send a new START - * when we change direction */ - + * when we change direction + */ s3c24xx_i2c_stop(i2c, -EINVAL); } @@ -497,17 +507,16 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) } else { /* send stop */ - s3c24xx_i2c_stop(i2c, 0); } break; case STATE_READ: - /* we have a byte of data in the data register, do + /* + * we have a byte of data in the data register, do * something with it, and then work out whether we are * going to do any more read/write */ - byte = readb(i2c->regs + S3C2410_IICDS); i2c->msg->buf[i2c->msg_ptr++] = byte; @@ -522,9 +531,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) s3c24xx_i2c_disable_ack(i2c); } else if (is_msgend(i2c)) { - /* ok, we've read the entire buffer, see if there - * is anything else we need to do */ - + /* + * ok, we've read the entire buffer, see if there + * is anything else we need to do + */ if (is_lastmsg(i2c)) { /* last message, send stop and complete */ dev_dbg(i2c->dev, "READ: Send Stop\n"); @@ -578,9 +588,10 @@ static irqreturn_t s3c24xx_i2c_irq(int irqno, void *dev_id) goto out; } - /* pretty much this leaves us with the fact that we've - * transmitted or received whatever byte we last sent */ - + /* + * pretty much this leaves us with the fact that we've + * transmitted or received whatever byte we last sent + */ i2c_s3c_irq_nextbyte(i2c, status); out: @@ -726,9 +737,10 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, ret = i2c->msg_idx; - /* having these next two as dev_err() makes life very - * noisy when doing an i2cdetect */ - + /* + * Having these next two as dev_err() makes life very + * noisy when doing an i2cdetect + */ if (timeout == 0) dev_dbg(i2c->dev, "timeout\n"); else if (ret != num) @@ -1071,10 +1083,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) } #else static void -s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) -{ - return; -} +s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) { } #endif static int s3c24xx_i2c_probe(struct platform_device *pdev) @@ -1117,7 +1126,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) init_waitqueue_head(&i2c->wait); /* find the clock and enable it */ - i2c->dev = &pdev->dev; i2c->clk = devm_clk_get(&pdev->dev, "i2c"); if (IS_ERR(i2c->clk)) { @@ -1127,9 +1135,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "clock source %p\n", i2c->clk); - /* map the registers */ - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); i2c->regs = devm_ioremap_resource(&pdev->dev, res); @@ -1140,22 +1146,17 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) i2c->regs, res); /* setup info block for the i2c core */ - i2c->adap.algo_data = i2c; i2c->adap.dev.parent = &pdev->dev; - i2c->pctrl = devm_pinctrl_get_select_default(i2c->dev); /* inititalise the i2c gpio lines */ - - if (i2c->pdata->cfg_gpio) { + if (i2c->pdata->cfg_gpio) i2c->pdata->cfg_gpio(to_platform_device(i2c->dev)); - } else if (IS_ERR(i2c->pctrl) && s3c24xx_i2c_parse_dt_gpio(i2c)) { + else if (IS_ERR(i2c->pctrl) && s3c24xx_i2c_parse_dt_gpio(i2c)) return -EINVAL; - } /* initialise the i2c controller */ - clk_prepare_enable(i2c->clk); ret = s3c24xx_i2c_init(i2c); clk_disable(i2c->clk); @@ -1164,10 +1165,11 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) clk_unprepare(i2c->clk); return ret; } - /* find the IRQ for this unit (note, this relies on the init call to + + /* + * find the IRQ for this unit (note, this relies on the init call to * ensure no current IRQs pending */ - if (!(i2c->quirks & QUIRK_POLL)) { i2c->irq = ret = platform_get_irq(pdev, 0); if (ret <= 0) { @@ -1176,9 +1178,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return ret; } - ret = devm_request_irq(&pdev->dev, i2c->irq, s3c24xx_i2c_irq, 0, - dev_name(&pdev->dev), i2c); - + ret = devm_request_irq(&pdev->dev, i2c->irq, s3c24xx_i2c_irq, + 0, dev_name(&pdev->dev), i2c); if (ret != 0) { dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq); clk_unprepare(i2c->clk); @@ -1193,12 +1194,12 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return ret; } - /* Note, previous versions of the driver used i2c_add_adapter() + /* + * Note, previous versions of the driver used i2c_add_adapter() * to add the bus at any number. We now pass the bus number via * the platform data, so if unset it will now default to always * being bus 0. */ - i2c->adap.nr = i2c->pdata->bus_num; i2c->adap.dev.of_node = pdev->dev.of_node; -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] i2c: s3c2410: Cleanup indentation and comment style @ 2016-04-20 9:24 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2016-04-20 9:24 UTC (permalink / raw) To: linux-arm-kernel Improve the readability by: - fixing indentation, - switching to proper block comments, - removing spurious blank lines, - checkpatch: void function return statements are not generally useful - checkpatch: braces {} are not necessary for any arm of this statement - checkpatch: missing a blank line after declarations Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 113 ++++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index fd3695a921f1..70a9f0c1ebca 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -170,6 +170,7 @@ static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *p { if (pdev->dev.of_node) { const struct of_device_id *match; + match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node); return (kernel_ulong_t)match->data; } @@ -277,9 +278,10 @@ static void s3c24xx_i2c_message_start(struct s3c24xx_i2c *i2c, dev_dbg(i2c->dev, "START: %08lx to IICSTAT, %02x to DS\n", stat, addr); writeb(addr, i2c->regs + S3C2410_IICDS); - /* delay here to ensure the data byte has gotten onto the bus - * before the transaction is started */ - + /* + * delay here to ensure the data byte has gotten onto the bus + * before the transaction is started + */ ndelay(i2c->tx_setup); dev_dbg(i2c->dev, "iiccon, %08lx\n", iiccon); @@ -354,8 +356,10 @@ static inline void s3c24xx_i2c_stop(struct s3c24xx_i2c *i2c, int ret) s3c24xx_i2c_disable_irq(i2c); } -/* helper functions to determine the current state in the set of - * messages we are sending */ +/* + * helper functions to determine the current state in the set of + * messages we are sending + */ /* * returns TRUE if the current message is the last in the set @@ -370,9 +374,11 @@ static inline int is_lastmsg(struct s3c24xx_i2c *i2c) */ static inline int is_msglast(struct s3c24xx_i2c *i2c) { - /* msg->len is always 1 for the first byte of smbus block read. + /* + * msg->len is always 1 for the first byte of smbus block read. * Actual length will be read from slave. More bytes will be - * read according to the length then. */ + * read according to the length then. + */ if (i2c->msg->flags & I2C_M_RECV_LEN && i2c->msg->len == 1) return 0; @@ -408,14 +414,13 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) goto out_ack; case STATE_START: - /* last thing we did was send a start condition on the + /* + * last thing we did was send a start condition on the * bus, or started a new i2c message */ - if (iicstat & S3C2410_IICSTAT_LASTBIT && !(i2c->msg->flags & I2C_M_IGNORE_NAK)) { /* ack was not received... */ - dev_dbg(i2c->dev, "ack was not received\n"); s3c24xx_i2c_stop(i2c, -ENXIO); goto out_ack; @@ -426,9 +431,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) else i2c->state = STATE_WRITE; - /* terminate the transfer if there is nothing to do - * as this is used by the i2c probe to find devices. */ - + /* + * Terminate the transfer if there is nothing to do + * as this is used by the i2c probe to find devices. + */ if (is_lastmsg(i2c) && i2c->msg->len == 0) { s3c24xx_i2c_stop(i2c, 0); goto out_ack; @@ -437,14 +443,16 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) if (i2c->state == STATE_READ) goto prepare_read; - /* fall through to the write state, as we will need to - * send a byte as well */ + /* + * fall through to the write state, as we will need to + * send a byte as well + */ case STATE_WRITE: - /* we are writing data to the device... check for the + /* + * we are writing data to the device... check for the * end of the message, and if so, work out what to do */ - if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) { if (iicstat & S3C2410_IICSTAT_LASTBIT) { dev_dbg(i2c->dev, "WRITE: No Ack\n"); @@ -460,12 +468,13 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) byte = i2c->msg->buf[i2c->msg_ptr++]; writeb(byte, i2c->regs + S3C2410_IICDS); - /* delay after writing the byte to allow the + /* + * delay after writing the byte to allow the * data setup time on the bus, as writing the * data to the register causes the first bit * to appear on SDA, and SCL will change as - * soon as the interrupt is acknowledged */ - + * soon as the interrupt is acknowledged + */ ndelay(i2c->tx_setup); } else if (!is_lastmsg(i2c)) { @@ -481,10 +490,11 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) if (i2c->msg->flags & I2C_M_NOSTART) { if (i2c->msg->flags & I2C_M_RD) { - /* cannot do this, the controller + /* + * cannot do this, the controller * forces us to send a new START - * when we change direction */ - + * when we change direction + */ s3c24xx_i2c_stop(i2c, -EINVAL); } @@ -497,17 +507,16 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) } else { /* send stop */ - s3c24xx_i2c_stop(i2c, 0); } break; case STATE_READ: - /* we have a byte of data in the data register, do + /* + * we have a byte of data in the data register, do * something with it, and then work out whether we are * going to do any more read/write */ - byte = readb(i2c->regs + S3C2410_IICDS); i2c->msg->buf[i2c->msg_ptr++] = byte; @@ -522,9 +531,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat) s3c24xx_i2c_disable_ack(i2c); } else if (is_msgend(i2c)) { - /* ok, we've read the entire buffer, see if there - * is anything else we need to do */ - + /* + * ok, we've read the entire buffer, see if there + * is anything else we need to do + */ if (is_lastmsg(i2c)) { /* last message, send stop and complete */ dev_dbg(i2c->dev, "READ: Send Stop\n"); @@ -578,9 +588,10 @@ static irqreturn_t s3c24xx_i2c_irq(int irqno, void *dev_id) goto out; } - /* pretty much this leaves us with the fact that we've - * transmitted or received whatever byte we last sent */ - + /* + * pretty much this leaves us with the fact that we've + * transmitted or received whatever byte we last sent + */ i2c_s3c_irq_nextbyte(i2c, status); out: @@ -726,9 +737,10 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, ret = i2c->msg_idx; - /* having these next two as dev_err() makes life very - * noisy when doing an i2cdetect */ - + /* + * Having these next two as dev_err() makes life very + * noisy when doing an i2cdetect + */ if (timeout == 0) dev_dbg(i2c->dev, "timeout\n"); else if (ret != num) @@ -1071,10 +1083,7 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) } #else static void -s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) -{ - return; -} +s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c) { } #endif static int s3c24xx_i2c_probe(struct platform_device *pdev) @@ -1117,7 +1126,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) init_waitqueue_head(&i2c->wait); /* find the clock and enable it */ - i2c->dev = &pdev->dev; i2c->clk = devm_clk_get(&pdev->dev, "i2c"); if (IS_ERR(i2c->clk)) { @@ -1127,9 +1135,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) dev_dbg(&pdev->dev, "clock source %p\n", i2c->clk); - /* map the registers */ - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); i2c->regs = devm_ioremap_resource(&pdev->dev, res); @@ -1140,22 +1146,17 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) i2c->regs, res); /* setup info block for the i2c core */ - i2c->adap.algo_data = i2c; i2c->adap.dev.parent = &pdev->dev; - i2c->pctrl = devm_pinctrl_get_select_default(i2c->dev); /* inititalise the i2c gpio lines */ - - if (i2c->pdata->cfg_gpio) { + if (i2c->pdata->cfg_gpio) i2c->pdata->cfg_gpio(to_platform_device(i2c->dev)); - } else if (IS_ERR(i2c->pctrl) && s3c24xx_i2c_parse_dt_gpio(i2c)) { + else if (IS_ERR(i2c->pctrl) && s3c24xx_i2c_parse_dt_gpio(i2c)) return -EINVAL; - } /* initialise the i2c controller */ - clk_prepare_enable(i2c->clk); ret = s3c24xx_i2c_init(i2c); clk_disable(i2c->clk); @@ -1164,10 +1165,11 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) clk_unprepare(i2c->clk); return ret; } - /* find the IRQ for this unit (note, this relies on the init call to + + /* + * find the IRQ for this unit (note, this relies on the init call to * ensure no current IRQs pending */ - if (!(i2c->quirks & QUIRK_POLL)) { i2c->irq = ret = platform_get_irq(pdev, 0); if (ret <= 0) { @@ -1176,9 +1178,8 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return ret; } - ret = devm_request_irq(&pdev->dev, i2c->irq, s3c24xx_i2c_irq, 0, - dev_name(&pdev->dev), i2c); - + ret = devm_request_irq(&pdev->dev, i2c->irq, s3c24xx_i2c_irq, + 0, dev_name(&pdev->dev), i2c); if (ret != 0) { dev_err(&pdev->dev, "cannot claim IRQ %d\n", i2c->irq); clk_unprepare(i2c->clk); @@ -1193,12 +1194,12 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) return ret; } - /* Note, previous versions of the driver used i2c_add_adapter() + /* + * Note, previous versions of the driver used i2c_add_adapter() * to add the bus at any number. We now pass the bus number via * the platform data, so if unset it will now default to always * being bus 0. */ - i2c->adap.nr = i2c->pdata->bus_num; i2c->adap.dev.of_node = pdev->dev.of_node; -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] i2c: s3c2410: Cleanup indentation and comment style 2016-04-20 9:24 ` Krzysztof Kozlowski @ 2016-04-20 14:02 ` Javier Martinez Canillas -1 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 14:02 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Bartlomiej Zolnierkiewicz Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > Improve the readability by: > - fixing indentation, > - switching to proper block comments, > - removing spurious blank lines, > - checkpatch: void function return statements are not generally useful > - checkpatch: braces {} are not necessary for any arm of this statement > - checkpatch: missing a blank line after declarations > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] i2c: s3c2410: Cleanup indentation and comment style @ 2016-04-20 14:02 ` Javier Martinez Canillas 0 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 14:02 UTC (permalink / raw) To: linux-arm-kernel Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > Improve the readability by: > - fixing indentation, > - switching to proper block comments, > - removing spurious blank lines, > - checkpatch: void function return statements are not generally useful > - checkpatch: braces {} are not necessary for any arm of this statement > - checkpatch: missing a blank line after declarations > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path 2016-04-20 9:24 ` Krzysztof Kozlowski @ 2016-04-20 13:47 ` Javier Martinez Canillas -1 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 13:47 UTC (permalink / raw) To: Krzysztof Kozlowski, Kukjin Kim, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, linux-i2c, linux-kernel Cc: Bartlomiej Zolnierkiewicz Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > If during probe() the s3c24xx_i2c_init() failed, the clock was left in > disabled but prepared state. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/i2c/busses/i2c-s3c2410.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c > index 362a6de54833..e9658af36f2e 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -1200,6 +1200,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) > clk_disable(i2c->clk); > if (ret != 0) { > dev_err(&pdev->dev, "I2C controller init failed\n"); > + clk_unprepare(i2c->clk); > return ret; > } > /* find the IRQ for this unit (note, this relies on the init call to > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path @ 2016-04-20 13:47 ` Javier Martinez Canillas 0 siblings, 0 replies; 14+ messages in thread From: Javier Martinez Canillas @ 2016-04-20 13:47 UTC (permalink / raw) To: linux-arm-kernel Hello Krzysztof, On 04/20/2016 05:24 AM, Krzysztof Kozlowski wrote: > If during probe() the s3c24xx_i2c_init() failed, the clock was left in > disabled but prepared state. > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > --- > drivers/i2c/busses/i2c-s3c2410.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c > index 362a6de54833..e9658af36f2e 100644 > --- a/drivers/i2c/busses/i2c-s3c2410.c > +++ b/drivers/i2c/busses/i2c-s3c2410.c > @@ -1200,6 +1200,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev) > clk_disable(i2c->clk); > if (ret != 0) { > dev_err(&pdev->dev, "I2C controller init failed\n"); > + clk_unprepare(i2c->clk); > return ret; > } > /* find the IRQ for this unit (note, this relies on the init call to > Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> Best regards, -- Javier Martinez Canillas Open Source Group Samsung Research America ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-04-21 7:01 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-20 9:24 [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path Krzysztof Kozlowski 2016-04-20 9:24 ` Krzysztof Kozlowski 2016-04-20 9:24 ` [PATCH 2/3] i2c: s3c2410: Minor function-level comment cleanup Krzysztof Kozlowski 2016-04-20 9:24 ` Krzysztof Kozlowski 2016-04-20 13:59 ` Javier Martinez Canillas 2016-04-20 13:59 ` Javier Martinez Canillas 2016-04-21 7:01 ` Krzysztof Kozlowski 2016-04-21 7:01 ` Krzysztof Kozlowski 2016-04-20 9:24 ` [PATCH 3/3] i2c: s3c2410: Cleanup indentation and comment style Krzysztof Kozlowski 2016-04-20 9:24 ` Krzysztof Kozlowski 2016-04-20 14:02 ` Javier Martinez Canillas 2016-04-20 14:02 ` Javier Martinez Canillas 2016-04-20 13:47 ` [PATCH 1/3] i2c: s3c2410: Add missing clock unprepare on probe() error path Javier Martinez Canillas 2016-04-20 13:47 ` Javier Martinez Canillas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.