openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] i2c: aspeed: Add newline characters into message printings.
@ 2018-07-02 21:13 Jae Hyun Yoo
  2018-07-11  5:42 ` Brendan Higgins
  2018-07-20 22:25 ` Wolfram Sang
  0 siblings, 2 replies; 7+ messages in thread
From: Jae Hyun Yoo @ 2018-07-02 21:13 UTC (permalink / raw)
  To: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley,
	Andrew Jeffery, linux-i2c, openbmc, linux-arm-kernel,
	linux-aspeed, linux-kernel
  Cc: Jarkko Nikula, James Feist, Vernon Mauery, Jae Hyun Yoo

There are some log printing without a newline character. This
patch adds the missing newline characters.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 60e4d0e939a3..e3007c1c4ac5 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -407,7 +407,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 	 */
 	ret = aspeed_i2c_is_irq_error(irq_status);
 	if (ret < 0) {
-		dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+		dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
 			irq_status);
 		bus->cmd_err = ret;
 		bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
@@ -416,7 +416,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 
 	/* We are in an invalid state; reset bus to a known state. */
 	if (!bus->msgs) {
-		dev_err(bus->dev, "bus in unknown state");
+		dev_err(bus->dev, "bus in unknown state\n");
 		bus->cmd_err = -EIO;
 		if (bus->master_state != ASPEED_I2C_MASTER_STOP)
 			aspeed_i2c_do_stop(bus);
@@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 	 */
 	if (bus->master_state == ASPEED_I2C_MASTER_START) {
 		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
-			pr_devel("no slave present at %02x", msg->addr);
+			pr_devel("no slave present at %02x\n", msg->addr);
 			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
 			bus->cmd_err = -ENXIO;
 			aspeed_i2c_do_stop(bus);
@@ -451,11 +451,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 	switch (bus->master_state) {
 	case ASPEED_I2C_MASTER_TX:
 		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
-			dev_dbg(bus->dev, "slave NACKed TX");
+			dev_dbg(bus->dev, "slave NACKed TX\n");
 			status_ack |= ASPEED_I2CD_INTR_TX_NAK;
 			goto error_and_stop;
 		} else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
-			dev_err(bus->dev, "slave failed to ACK TX");
+			dev_err(bus->dev, "slave failed to ACK TX\n");
 			goto error_and_stop;
 		}
 		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
@@ -478,7 +478,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 		/* fallthrough intended */
 	case ASPEED_I2C_MASTER_RX:
 		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
-			dev_err(bus->dev, "master failed to RX");
+			dev_err(bus->dev, "master failed to RX\n");
 			goto error_and_stop;
 		}
 		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
@@ -509,7 +509,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 		goto out_no_complete;
 	case ASPEED_I2C_MASTER_STOP:
 		if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
-			dev_err(bus->dev, "master failed to STOP");
+			dev_err(bus->dev, "master failed to STOP\n");
 			bus->cmd_err = -EIO;
 			/* Do not STOP as we have already tried. */
 		} else {
@@ -520,7 +520,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
 		goto out_complete;
 	case ASPEED_I2C_MASTER_INACTIVE:
 		dev_err(bus->dev,
-			"master received interrupt 0x%08x, but is inactive",
+			"master received interrupt 0x%08x, but is inactive\n",
 			irq_status);
 		bus->cmd_err = -EIO;
 		/* Do not STOP as we should be inactive. */
@@ -851,7 +851,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
 	if (IS_ERR(bus->rst)) {
 		dev_err(&pdev->dev,
-			"missing or invalid reset controller device tree entry");
+			"missing or invalid reset controller device tree entry\n");
 		return PTR_ERR(bus->rst);
 	}
 	reset_control_deassert(bus->rst);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-02 21:13 [PATCH] i2c: aspeed: Add newline characters into message printings Jae Hyun Yoo
@ 2018-07-11  5:42 ` Brendan Higgins
  2018-07-11 16:53   ` Jae Hyun Yoo
  2018-07-20 22:25 ` Wolfram Sang
  1 sibling, 1 reply; 7+ messages in thread
From: Brendan Higgins @ 2018-07-11  5:42 UTC (permalink / raw)
  To: jae.hyun.yoo
  Cc: Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, james.feist,
	vernon.mauery

On Mon, Jul 2, 2018 at 2:14 PM Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
>
> There are some log printing without a newline character. This
> patch adds the missing newline characters.
>
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 60e4d0e939a3..e3007c1c4ac5 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -407,7 +407,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>          */
>         ret = aspeed_i2c_is_irq_error(irq_status);
>         if (ret < 0) {
> -               dev_dbg(bus->dev, "received error interrupt: 0x%08x",
> +               dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
>                         irq_status);
>                 bus->cmd_err = ret;
>                 bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
> @@ -416,7 +416,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>
>         /* We are in an invalid state; reset bus to a known state. */
>         if (!bus->msgs) {
> -               dev_err(bus->dev, "bus in unknown state");
> +               dev_err(bus->dev, "bus in unknown state\n");
>                 bus->cmd_err = -EIO;
>                 if (bus->master_state != ASPEED_I2C_MASTER_STOP)
>                         aspeed_i2c_do_stop(bus);
> @@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>          */
>         if (bus->master_state == ASPEED_I2C_MASTER_START) {
>                 if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> -                       pr_devel("no slave present at %02x", msg->addr);
> +                       pr_devel("no slave present at %02x\n", msg->addr);

Unless something changed in the last couple versions of the kernel, this is the
only line that actually changes anything. dev_* inserts a newline for every
call.

Admittedly, the rest of the file is pretty inconsistent, so if you really want
to make all these changes, I don't feel super strongly about it.

>                         status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>                         bus->cmd_err = -ENXIO;
>                         aspeed_i2c_do_stop(bus);
> @@ -451,11 +451,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>         switch (bus->master_state) {
>         case ASPEED_I2C_MASTER_TX:
>                 if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
> -                       dev_dbg(bus->dev, "slave NACKed TX");
> +                       dev_dbg(bus->dev, "slave NACKed TX\n");
>                         status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>                         goto error_and_stop;
>                 } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> -                       dev_err(bus->dev, "slave failed to ACK TX");
> +                       dev_err(bus->dev, "slave failed to ACK TX\n");
>                         goto error_and_stop;
>                 }
>                 status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> @@ -478,7 +478,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>                 /* fallthrough intended */
>         case ASPEED_I2C_MASTER_RX:
>                 if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
> -                       dev_err(bus->dev, "master failed to RX");
> +                       dev_err(bus->dev, "master failed to RX\n");
>                         goto error_and_stop;
>                 }
>                 status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> @@ -509,7 +509,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>                 goto out_no_complete;
>         case ASPEED_I2C_MASTER_STOP:
>                 if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
> -                       dev_err(bus->dev, "master failed to STOP");
> +                       dev_err(bus->dev, "master failed to STOP\n");
>                         bus->cmd_err = -EIO;
>                         /* Do not STOP as we have already tried. */
>                 } else {
> @@ -520,7 +520,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>                 goto out_complete;
>         case ASPEED_I2C_MASTER_INACTIVE:
>                 dev_err(bus->dev,
> -                       "master received interrupt 0x%08x, but is inactive",
> +                       "master received interrupt 0x%08x, but is inactive\n",
>                         irq_status);
>                 bus->cmd_err = -EIO;
>                 /* Do not STOP as we should be inactive. */
> @@ -851,7 +851,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
>         if (IS_ERR(bus->rst)) {
>                 dev_err(&pdev->dev,
> -                       "missing or invalid reset controller device tree entry");
> +                       "missing or invalid reset controller device tree entry\n");
>                 return PTR_ERR(bus->rst);
>         }
>         reset_control_deassert(bus->rst);
> --
> 2.17.1
>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-11  5:42 ` Brendan Higgins
@ 2018-07-11 16:53   ` Jae Hyun Yoo
  2018-07-11 17:10     ` Joe Perches
  0 siblings, 1 reply; 7+ messages in thread
From: Jae Hyun Yoo @ 2018-07-11 16:53 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, james.feist,
	vernon.mauery

On 7/10/2018 10:42 PM, Brendan Higgins wrote:
> On Mon, Jul 2, 2018 at 2:14 PM Jae Hyun Yoo
> <jae.hyun.yoo@linux.intel.com> wrote:
>>
>> There are some log printing without a newline character. This
>> patch adds the missing newline characters.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/i2c/busses/i2c-aspeed.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> index 60e4d0e939a3..e3007c1c4ac5 100644
>> --- a/drivers/i2c/busses/i2c-aspeed.c
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -407,7 +407,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>           */
>>          ret = aspeed_i2c_is_irq_error(irq_status);
>>          if (ret < 0) {
>> -               dev_dbg(bus->dev, "received error interrupt: 0x%08x",
>> +               dev_dbg(bus->dev, "received error interrupt: 0x%08x\n",
>>                          irq_status);
>>                  bus->cmd_err = ret;
>>                  bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
>> @@ -416,7 +416,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>
>>          /* We are in an invalid state; reset bus to a known state. */
>>          if (!bus->msgs) {
>> -               dev_err(bus->dev, "bus in unknown state");
>> +               dev_err(bus->dev, "bus in unknown state\n");
>>                  bus->cmd_err = -EIO;
>>                  if (bus->master_state != ASPEED_I2C_MASTER_STOP)
>>                          aspeed_i2c_do_stop(bus);
>> @@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>           */
>>          if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>                  if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>> -                       pr_devel("no slave present at %02x", msg->addr);
>> +                       pr_devel("no slave present at %02x\n", msg->addr);
> 
> Unless something changed in the last couple versions of the kernel, this is the
> only line that actually changes anything. dev_* inserts a newline for every
> call.
> 
> Admittedly, the rest of the file is pretty inconsistent, so if you really want
> to make all these changes, I don't feel super strongly about it.
> 

Agreed. Will drop this patch.

Thanks,

Jae

>>                          status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>>                          bus->cmd_err = -ENXIO;
>>                          aspeed_i2c_do_stop(bus);
>> @@ -451,11 +451,11 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>          switch (bus->master_state) {
>>          case ASPEED_I2C_MASTER_TX:
>>                  if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
>> -                       dev_dbg(bus->dev, "slave NACKed TX");
>> +                       dev_dbg(bus->dev, "slave NACKed TX\n");
>>                          status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>>                          goto error_and_stop;
>>                  } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>> -                       dev_err(bus->dev, "slave failed to ACK TX");
>> +                       dev_err(bus->dev, "slave failed to ACK TX\n");
>>                          goto error_and_stop;
>>                  }
>>                  status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> @@ -478,7 +478,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>                  /* fallthrough intended */
>>          case ASPEED_I2C_MASTER_RX:
>>                  if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
>> -                       dev_err(bus->dev, "master failed to RX");
>> +                       dev_err(bus->dev, "master failed to RX\n");
>>                          goto error_and_stop;
>>                  }
>>                  status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> @@ -509,7 +509,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>                  goto out_no_complete;
>>          case ASPEED_I2C_MASTER_STOP:
>>                  if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
>> -                       dev_err(bus->dev, "master failed to STOP");
>> +                       dev_err(bus->dev, "master failed to STOP\n");
>>                          bus->cmd_err = -EIO;
>>                          /* Do not STOP as we have already tried. */
>>                  } else {
>> @@ -520,7 +520,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>                  goto out_complete;
>>          case ASPEED_I2C_MASTER_INACTIVE:
>>                  dev_err(bus->dev,
>> -                       "master received interrupt 0x%08x, but is inactive",
>> +                       "master received interrupt 0x%08x, but is inactive\n",
>>                          irq_status);
>>                  bus->cmd_err = -EIO;
>>                  /* Do not STOP as we should be inactive. */
>> @@ -851,7 +851,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>>          bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
>>          if (IS_ERR(bus->rst)) {
>>                  dev_err(&pdev->dev,
>> -                       "missing or invalid reset controller device tree entry");
>> +                       "missing or invalid reset controller device tree entry\n");
>>                  return PTR_ERR(bus->rst);
>>          }
>>          reset_control_deassert(bus->rst);
>> --
>> 2.17.1
>>
> 
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-11 16:53   ` Jae Hyun Yoo
@ 2018-07-11 17:10     ` Joe Perches
  2018-07-12  8:38       ` Brendan Higgins
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Perches @ 2018-07-11 17:10 UTC (permalink / raw)
  To: Jae Hyun Yoo, Brendan Higgins
  Cc: Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, james.feist,
	vernon.mauery

On Wed, 2018-07-11 at 09:53 -0700, Jae Hyun Yoo wrote:
> On 7/10/2018 10:42 PM, Brendan Higgins wrote:
> > On Mon, Jul 2, 2018 at 2:14 PM Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
> > > There are some log printing without a newline character. This
> > > patch adds the missing newline characters.
[]
> > > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
[]
> > > @@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> > >           */
> > >          if (bus->master_state == ASPEED_I2C_MASTER_START) {
> > >                  if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> > > -                       pr_devel("no slave present at %02x", msg->addr);
> > > +                       pr_devel("no slave present at %02x\n", msg->addr);
> > 
> > Unless something changed in the last couple versions of the kernel, this is the
> > only line that actually changes anything. dev_* inserts a newline for every
> > call.

Not true.

Any printk without KERN_CONT inserts a newline
if the last character
emitted is not a newline.

dev_<level> uses can also be followed by pr_cont.

So this patch does reduce the possibility of
interleaved messages from multiple processes.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-11 17:10     ` Joe Perches
@ 2018-07-12  8:38       ` Brendan Higgins
  2018-07-12 17:10         ` Jae Hyun Yoo
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Higgins @ 2018-07-12  8:38 UTC (permalink / raw)
  To: joe
  Cc: jae.hyun.yoo, Benjamin Herrenschmidt, Joel Stanley,
	Andrew Jeffery, linux-i2c, OpenBMC Maillist, Linux ARM,
	linux-aspeed, Linux Kernel Mailing List, jarkko.nikula,
	james.feist, vernon.mauery

On Wed, Jul 11, 2018 at 10:10 AM Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2018-07-11 at 09:53 -0700, Jae Hyun Yoo wrote:
> > On 7/10/2018 10:42 PM, Brendan Higgins wrote:
> > > On Mon, Jul 2, 2018 at 2:14 PM Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
> > > > There are some log printing without a newline character. This
> > > > patch adds the missing newline characters.
> []
> > > > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> []
> > > > @@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> > > >           */
> > > >          if (bus->master_state == ASPEED_I2C_MASTER_START) {
> > > >                  if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
> > > > -                       pr_devel("no slave present at %02x", msg->addr);
> > > > +                       pr_devel("no slave present at %02x\n", msg->addr);
> > >
> > > Unless something changed in the last couple versions of the kernel, this is the
> > > only line that actually changes anything. dev_* inserts a newline for every
> > > call.
>
> Not true.
>
> Any printk without KERN_CONT inserts a newline
> if the last character
> emitted is not a newline.
>
> dev_<level> uses can also be followed by pr_cont.
>
> So this patch does reduce the possibility of
> interleaved messages from multiple processes.

My mistake. Thanks for pointing that out.

Jae, forget what I said earlier. This looks good to me.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-12  8:38       ` Brendan Higgins
@ 2018-07-12 17:10         ` Jae Hyun Yoo
  0 siblings, 0 replies; 7+ messages in thread
From: Jae Hyun Yoo @ 2018-07-12 17:10 UTC (permalink / raw)
  To: Brendan Higgins, joe
  Cc: Benjamin Herrenschmidt, Joel Stanley, Andrew Jeffery, linux-i2c,
	OpenBMC Maillist, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, jarkko.nikula, james.feist,
	vernon.mauery

On 7/12/2018 1:38 AM, Brendan Higgins wrote:
> On Wed, Jul 11, 2018 at 10:10 AM Joe Perches <joe@perches.com> wrote:
>>
>> On Wed, 2018-07-11 at 09:53 -0700, Jae Hyun Yoo wrote:
>>> On 7/10/2018 10:42 PM, Brendan Higgins wrote:
>>>> On Mon, Jul 2, 2018 at 2:14 PM Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> wrote:
>>>>> There are some log printing without a newline character. This
>>>>> patch adds the missing newline characters.
>> []
>>>>> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> []
>>>>> @@ -431,7 +431,7 @@ static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>>>>>            */
>>>>>           if (bus->master_state == ASPEED_I2C_MASTER_START) {
>>>>>                   if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
>>>>> -                       pr_devel("no slave present at %02x", msg->addr);
>>>>> +                       pr_devel("no slave present at %02x\n", msg->addr);
>>>>
>>>> Unless something changed in the last couple versions of the kernel, this is the
>>>> only line that actually changes anything. dev_* inserts a newline for every
>>>> call.
>>
>> Not true.
>>
>> Any printk without KERN_CONT inserts a newline
>> if the last character
>> emitted is not a newline.
>>
>> dev_<level> uses can also be followed by pr_cont.
>>
>> So this patch does reduce the possibility of
>> interleaved messages from multiple processes.
> 
> My mistake. Thanks for pointing that out.
> 
> Jae, forget what I said earlier. This looks good to me.
> 

Okay. I'll keep this patch as is.
Thanks Joe for your clarification!
Thanks Brendan for your 'Reviewed-by' tag!

Jae

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i2c: aspeed: Add newline characters into message printings.
  2018-07-02 21:13 [PATCH] i2c: aspeed: Add newline characters into message printings Jae Hyun Yoo
  2018-07-11  5:42 ` Brendan Higgins
@ 2018-07-20 22:25 ` Wolfram Sang
  1 sibling, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2018-07-20 22:25 UTC (permalink / raw)
  To: Jae Hyun Yoo
  Cc: Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley,
	Andrew Jeffery, linux-i2c, openbmc, linux-arm-kernel,
	linux-aspeed, linux-kernel, Jarkko Nikula, James Feist,
	Vernon Mauery

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Mon, Jul 02, 2018 at 02:13:59PM -0700, Jae Hyun Yoo wrote:
> There are some log printing without a newline character. This
> patch adds the missing newline characters.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-07-21  3:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02 21:13 [PATCH] i2c: aspeed: Add newline characters into message printings Jae Hyun Yoo
2018-07-11  5:42 ` Brendan Higgins
2018-07-11 16:53   ` Jae Hyun Yoo
2018-07-11 17:10     ` Joe Perches
2018-07-12  8:38       ` Brendan Higgins
2018-07-12 17:10         ` Jae Hyun Yoo
2018-07-20 22:25 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).