Re: [RESEND PATCH] drm/hisilicon: Ensure LDI regs are properly configured.

[Greg KH]

On Thu, Jun 29, 2017 at 09:54:38PM +0100, Peter Griffin wrote:
> This patch fixes the following soft lockup:
>   BUG: soft lockup - CPU#0 stuck for 23s! [weston:307]
> 
> On weston idle-timeout the IP is powered down and reset
> asserted. On weston resume we get a massive vblank
> IRQ storm due to the LDI registers having lost some state.
> 
> This state loss is caused by ade_crtc_atomic_begin() not
> calling ade_ldi_set_mode(). With this patch applied
> resuming from Weston idle-timeout works well.
> 
> Signed-off-by: Peter Griffin 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 3 +++
>  1 file changed, 3 insertions(+)
> 



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Re: [RESEND PATCH] drm/hisilicon: Ensure LDI regs are properly configured.

[Greg KH]

On Thu, Jun 29, 2017 at 09:54:38PM +0100, Peter Griffin wrote:
> This patch fixes the following soft lockup:
>   BUG: soft lockup - CPU#0 stuck for 23s! [weston:307]
> 
> On weston idle-timeout the IP is powered down and reset
> asserted. On weston resume we get a massive vblank
> IRQ storm due to the LDI registers having lost some state.
> 
> This state loss is caused by ade_crtc_atomic_begin() not
> calling ade_ldi_set_mode(). With this patch applied
> resuming from Weston idle-timeout works well.
> 
> Signed-off-by: Peter Griffin 
> ---
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 3 +++
>  1 file changed, 3 insertions(+)
> 



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Re: [PATCH] staging: ks7010: fix styling WARNINGs

[Frans Klaver]

On Fri, Jun 30, 2017 at 6:52 AM, Mark Rogers  wrote:
> Trivial style changes. There are still 3 "line over 80 characters"
> checkpatch.pl warnings, but I think they are best left alone as
> breaking the first two warning lines could hurt readability. The third
> warning is a message that should not be broken for the sake of grep.
>
> All but one of the changes fix lines that exceed 80 characters. An
> embedded function name was replaced by __func__ as well.
>
> Signed-off-by: Mark Rogers 
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
> b/drivers/staging/ks7010/ks7010_sdio.c
> index c325f48..6c0c6b2 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -548,7 +548,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
> if (atomic_read(>psstatus.status) == PS_SNOOZE) 
> {
> if (cnt_txqbody(priv)) {
> ks_wlan_hw_wakeup_request(priv);
> -   queue_delayed_work(priv->wq, 
> >rw_dwork, 1);
> +   queue_delayed_work(priv->wq,
> +  >rw_dwork, 
> 1);
> return;
> }
> } else {
> @@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card 
> *card)
> memcpy(rom_buf, fw_entry->data + n, size);
>
> offset = n;
> -   ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + 
> offset);
> +   ret = ks7010_sdio_update_index(priv,
> +  KS7010_IRAM_ADDRESS + offset);
> if (ret)
> goto release_firmware;
>
> -   ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
> +   ret = ks7010_sdio_write(priv,
> +   DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> -   ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, 
> size);
> +   ret = ks7010_sdio_data_compare(priv,
> +  DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> @@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> priv = netdev_priv(netdev);
>
> card->priv = priv;
> -   SET_NETDEV_DEV(netdev, >func->dev);   /* for create sysfs 
> symlinks */
> +   SET_NETDEV_DEV(netdev, >func->dev);/* for create sysfs symlinks 
> */

I don't think this is much of an improvement for readability. Should
we move the comment about a bit?

>
> /* private memory initialize */
> priv->ks_sdio_card = card;
> @@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> }
>
> /* interrupt setting */
> -   /* clear Interrupt status write (ARMtoSD_InterruptPending 
> FN1:00_0024) */
> +   /* clear Interrupt status write (ARMtoSD_InterruptPending 
> FN1:00_0024)*/

This is a bit of a pointless change, isn't it? It also makes the comment uglier.

> sdio_claim_host(func);
> ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
> sdio_release_host(func);
> @@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
> struct ks_sdio_card *card;
> struct ks_wlan_private *priv;
>
> -   DPRINTK(1, "ks7010_sdio_remove()\n");
> +   DPRINTK(1, "%s()\n", __func__);

You might get a "one thing per patch please" for this. You wouldn't
have had to change this line if you'd strictly stuck to that.

Frans

Re: [PATCH] staging: ks7010: fix styling WARNINGs

[Frans Klaver]

On Fri, Jun 30, 2017 at 6:52 AM, Mark Rogers  wrote:
> Trivial style changes. There are still 3 "line over 80 characters"
> checkpatch.pl warnings, but I think they are best left alone as
> breaking the first two warning lines could hurt readability. The third
> warning is a message that should not be broken for the sake of grep.
>
> All but one of the changes fix lines that exceed 80 characters. An
> embedded function name was replaced by __func__ as well.
>
> Signed-off-by: Mark Rogers 
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
> b/drivers/staging/ks7010/ks7010_sdio.c
> index c325f48..6c0c6b2 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -548,7 +548,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
> if (atomic_read(>psstatus.status) == PS_SNOOZE) 
> {
> if (cnt_txqbody(priv)) {
> ks_wlan_hw_wakeup_request(priv);
> -   queue_delayed_work(priv->wq, 
> >rw_dwork, 1);
> +   queue_delayed_work(priv->wq,
> +  >rw_dwork, 
> 1);
> return;
> }
> } else {
> @@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card 
> *card)
> memcpy(rom_buf, fw_entry->data + n, size);
>
> offset = n;
> -   ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + 
> offset);
> +   ret = ks7010_sdio_update_index(priv,
> +  KS7010_IRAM_ADDRESS + offset);
> if (ret)
> goto release_firmware;
>
> -   ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
> +   ret = ks7010_sdio_write(priv,
> +   DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> -   ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, 
> size);
> +   ret = ks7010_sdio_data_compare(priv,
> +  DATA_WINDOW, rom_buf, size);
> if (ret)
> goto release_firmware;
>
> @@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> priv = netdev_priv(netdev);
>
> card->priv = priv;
> -   SET_NETDEV_DEV(netdev, >func->dev);   /* for create sysfs 
> symlinks */
> +   SET_NETDEV_DEV(netdev, >func->dev);/* for create sysfs symlinks 
> */

I don't think this is much of an improvement for readability. Should
we move the comment about a bit?

>
> /* private memory initialize */
> priv->ks_sdio_card = card;
> @@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
> }
>
> /* interrupt setting */
> -   /* clear Interrupt status write (ARMtoSD_InterruptPending 
> FN1:00_0024) */
> +   /* clear Interrupt status write (ARMtoSD_InterruptPending 
> FN1:00_0024)*/

This is a bit of a pointless change, isn't it? It also makes the comment uglier.

> sdio_claim_host(func);
> ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
> sdio_release_host(func);
> @@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
> struct ks_sdio_card *card;
> struct ks_wlan_private *priv;
>
> -   DPRINTK(1, "ks7010_sdio_remove()\n");
> +   DPRINTK(1, "%s()\n", __func__);

You might get a "one thing per patch please" for this. You wouldn't
have had to change this line if you'd strictly stuck to that.

Frans

Re: [BISECTED, REGRESSION] v4.12-rc: omapdrm fails to probe on Nokia N900

[Tony Lindgren]

* Aaro Koskinen  [170629 11:50]:
> Is it just me or do other OMAP users fail to see omapdrm changes being
> posted to linux-omap for testing or review purposes?

Yeah Cc:ing linux-omap in addition to the drm list is a good idea. Hopefully
we get few more people to review changes that way.

What also should help preventing regressions is getting the changes into
Linux next early on during the -rc cycle. And switching the defconfigs to
use omapdrm instead of omapfb. Maybe we should do that right after v4.13-rc1
is tagged.

Cheers,

Tony

Re: [PATCH 1/1] selftests: gpio: use pkg-config to determine libmount CFLAGS/LDLIBS

[Fathi Boudra]

On 30 June 2017 at 05:20, Bamvor Zhang Jian  wrote:
> On 29 June 2017 at 19:39, Fathi Boudra  wrote:
>> On 29 June 2017 at 12:01, Michael Ellerman  wrote:
>>> Fathi Boudra  writes:
>>>
 Fix hardcoded and misplaced libmount headers. Use pkg-config instead to
 figure out CFLAGS/LDLIBS, fixing also their value for cross-compilation.
>>>
>>> Can you explain how it fixes the values for cross-compilation?
>>
>> When pkg-config is invoked in a cross-build environment, it will
>> return your sysroot headers path instead of the hardcoded
>> -I/usr/include/libmount
>>
>> For example in my oe environment, when I invoke "pkg-config --cflags
>> mount", it returns:
>> -I/build/tmp-rpb-glibc/sysroots/hikey/usr/include/libmount
> I suppose we could also pass the extra CFLAGS and LDFLAGS directly to
> make it work. correct?

or use the appropriate tool for getting CFLAGS/LDFLAGS instead of:
1. hardcoding it and potentially get them wrong
2. expecting the user will have to take extra steps, while he can just run make

> Regards
>
> Bamvor
>>
>>> cheers
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kselftest" 
>>> in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_isert: prevent NULL pointer dereference in isert_login_recv_done()

[Nicholas A. Bellinger]

On Thu, 2017-06-29 at 11:28 +0300, Sagi Grimberg wrote:
> >> Can you test just the one liner fix below?
> >>
>  @@ -1452,7 +1452,7 @@
>    isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>    {
>   struct isert_conn *isert_conn = wc->qp->qp_context;
>  -struct ib_device *ib_dev = isert_conn->cm_id->device;
>  +struct ib_device *ib_dev = isert_conn->device->ib_device;
>   if (unlikely(wc->status != IB_WC_SUCCESS)) {
>   isert_print_wc(wc, "login recv");
> > 
> > I'll test also this one-liner fix as soon as I can.
> > 
> > But I can say that I'm pretty sure it will work as well, because all the
> > previous NULL pointer dereferences that we've got in the past happened
> > all 100% in isert_login_recv_done(). The other cases are probably a safe
> > precaution, but they can't really happen.
> 
> Agree, I'd prefer to start with a surgical fix before moving forward.

Sounds fine.  The single line fix above in isert_login_recv_done() has
been queued up in target-pending/for-next with Andrea's Tested-by and
your Reviewed-by:

https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?h=for-next=702c0f17403765cc5aa1c18f6ea6eb549c1bac9b

Please let me know if there are any other issues wrt the surgical fix.

Re: [BISECTED, REGRESSION] v4.12-rc: omapdrm fails to probe on Nokia N900

[Tony Lindgren]

* Aaro Koskinen  [170629 11:50]:
> Is it just me or do other OMAP users fail to see omapdrm changes being
> posted to linux-omap for testing or review purposes?

Yeah Cc:ing linux-omap in addition to the drm list is a good idea. Hopefully
we get few more people to review changes that way.

What also should help preventing regressions is getting the changes into
Linux next early on during the -rc cycle. And switching the defconfigs to
use omapdrm instead of omapfb. Maybe we should do that right after v4.13-rc1
is tagged.

Cheers,

Tony

Re: [PATCH 1/1] selftests: gpio: use pkg-config to determine libmount CFLAGS/LDLIBS

[Fathi Boudra]

On 30 June 2017 at 05:20, Bamvor Zhang Jian  wrote:
> On 29 June 2017 at 19:39, Fathi Boudra  wrote:
>> On 29 June 2017 at 12:01, Michael Ellerman  wrote:
>>> Fathi Boudra  writes:
>>>
 Fix hardcoded and misplaced libmount headers. Use pkg-config instead to
 figure out CFLAGS/LDLIBS, fixing also their value for cross-compilation.
>>>
>>> Can you explain how it fixes the values for cross-compilation?
>>
>> When pkg-config is invoked in a cross-build environment, it will
>> return your sysroot headers path instead of the hardcoded
>> -I/usr/include/libmount
>>
>> For example in my oe environment, when I invoke "pkg-config --cflags
>> mount", it returns:
>> -I/build/tmp-rpb-glibc/sysroots/hikey/usr/include/libmount
> I suppose we could also pass the extra CFLAGS and LDFLAGS directly to
> make it work. correct?

or use the appropriate tool for getting CFLAGS/LDFLAGS instead of:
1. hardcoding it and potentially get them wrong
2. expecting the user will have to take extra steps, while he can just run make

> Regards
>
> Bamvor
>>
>>> cheers
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kselftest" 
>>> in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_isert: prevent NULL pointer dereference in isert_login_recv_done()

[Nicholas A. Bellinger]

On Thu, 2017-06-29 at 11:28 +0300, Sagi Grimberg wrote:
> >> Can you test just the one liner fix below?
> >>
>  @@ -1452,7 +1452,7 @@
>    isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc)
>    {
>   struct isert_conn *isert_conn = wc->qp->qp_context;
>  -struct ib_device *ib_dev = isert_conn->cm_id->device;
>  +struct ib_device *ib_dev = isert_conn->device->ib_device;
>   if (unlikely(wc->status != IB_WC_SUCCESS)) {
>   isert_print_wc(wc, "login recv");
> > 
> > I'll test also this one-liner fix as soon as I can.
> > 
> > But I can say that I'm pretty sure it will work as well, because all the
> > previous NULL pointer dereferences that we've got in the past happened
> > all 100% in isert_login_recv_done(). The other cases are probably a safe
> > precaution, but they can't really happen.
> 
> Agree, I'd prefer to start with a surgical fix before moving forward.

Sounds fine.  The single line fix above in isert_login_recv_done() has
been queued up in target-pending/for-next with Andrea's Tested-by and
your Reviewed-by:

https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?h=for-next=702c0f17403765cc5aa1c18f6ea6eb549c1bac9b

Please let me know if there are any other issues wrt the surgical fix.

Re: [PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Gustavo A. R. Silva]

Hi Frans,

Quoting Frans Klaver :


On Fri, Jun 30, 2017 at 6:42 AM, Gustavo A. R. Silva
 wrote:

Propagate the return values of platform_get_irq and
devm_request_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/clocksource/em_sti.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index bc48cbf..c4818dd 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "failed to get irq\n");
-   return -EINVAL;
+   return irq;
}

/* map memory, let base point to the STI instance */
@@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
if (IS_ERR(p->base))
return PTR_ERR(p->base);

-   if (devm_request_irq(>dev, irq, em_sti_interrupt,
+   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
 IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-dev_name(>dev), p)) {
+dev_name(>dev), p);
+   if (irq) {
dev_err(>dev, "failed to request low IRQ\n");
-   return -ENOENT;
+   return irq;
}


This works. Yet I think that 'ret' would be a better candidate for
taking the result of devm_request_irq, since it doesn't return the irq
number on success. Should someone decide to reference irq at a later
point in the code, this has to be changed.


Good point. I'll change it and send a new patch shortly.

Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Gustavo A. R. Silva]

Hi Frans,

Quoting Frans Klaver :


On Fri, Jun 30, 2017 at 6:42 AM, Gustavo A. R. Silva
 wrote:

Propagate the return values of platform_get_irq and
devm_request_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/clocksource/em_sti.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index bc48cbf..c4818dd 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "failed to get irq\n");
-   return -EINVAL;
+   return irq;
}

/* map memory, let base point to the STI instance */
@@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
if (IS_ERR(p->base))
return PTR_ERR(p->base);

-   if (devm_request_irq(>dev, irq, em_sti_interrupt,
+   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
 IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-dev_name(>dev), p)) {
+dev_name(>dev), p);
+   if (irq) {
dev_err(>dev, "failed to request low IRQ\n");
-   return -ENOENT;
+   return irq;
}


This works. Yet I think that 'ret' would be a better candidate for
taking the result of devm_request_irq, since it doesn't return the irq
number on success. Should someone decide to reference irq at a later
point in the code, this has to be changed.


Good point. I'll change it and send a new patch shortly.

Thanks!
--
Gustavo A. R. Silva

[PATCH v4] staging: lustre: lnet: remove dead code and crc32_le() wrapper

[Dmitriy Cherkasov]

After removing code which was permanently disabled with ifdefs, the
function ksocknal_csum() becomes just a wrapper for crc32_le(). Remove
this useless wrapper and instead call crc32_le() directly.

This also resolves the following checkpatch warning which was
triggered by the dead code:

WARNING: space prohibited before semicolon

Signed-off-by: Dmitriy Cherkasov 
---

Changes since v3:
* fixed spacing issues
* fixed typo in commit message

 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h  | 11 ---
 .../staging/lustre/lnet/klnds/socklnd/socklnd_lib.c  | 20 ++--
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h 
b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
index 5540de6..9eb169d 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
@@ -519,17 +519,6 @@ struct ksock_proto {
 #define CPU_MASK_NONE   0UL
 #endif
 
-static inline __u32 ksocknal_csum(__u32 crc, unsigned char const *p, size_t 
len)
-{
-#if 1
-   return crc32_le(crc, p, len);
-#else
-   while (len-- > 0)
-   crc = ((crc + 0x100) & ~0xff) | ((crc + *p++) & 0xff) ;
-   return crc;
-#endif
-}
-
 static inline int
 ksocknal_route_mask(void)
 {
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c 
b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
index 8a036f4..41715cd 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
@@ -201,9 +201,9 @@
if (fragnob > sum)
fragnob = sum;
 
-   conn->ksnc_rx_csum = ksocknal_csum(conn->ksnc_rx_csum,
-  iov[i].iov_base,
-  fragnob);
+   conn->ksnc_rx_csum = crc32_le(conn->ksnc_rx_csum,
+   iov[i].iov_base,
+   fragnob);
}
conn->ksnc_msg.ksm_csum = saved_csum;
}
@@ -243,8 +243,8 @@
if (fragnob > sum)
fragnob = sum;
 
-   conn->ksnc_rx_csum = ksocknal_csum(conn->ksnc_rx_csum,
-  base, fragnob);
+   conn->ksnc_rx_csum = crc32_le(conn->ksnc_rx_csum,
+   base, fragnob);
 
kunmap(kiov[i].bv_page);
}
@@ -265,22 +265,22 @@
 
tx->tx_msg.ksm_csum = 0;
 
-   csum = ksocknal_csum(~0, tx->tx_iov[0].iov_base,
-tx->tx_iov[0].iov_len);
+   csum = crc32_le(~0, tx->tx_iov[0].iov_base,
+   tx->tx_iov[0].iov_len);
 
if (tx->tx_kiov) {
for (i = 0; i < tx->tx_nkiov; i++) {
base = kmap(tx->tx_kiov[i].bv_page) +
   tx->tx_kiov[i].bv_offset;
 
-   csum = ksocknal_csum(csum, base, tx->tx_kiov[i].bv_len);
+   csum = crc32_le(csum, base, tx->tx_kiov[i].bv_len);
 
kunmap(tx->tx_kiov[i].bv_page);
}
} else {
for (i = 1; i < tx->tx_niov; i++)
-   csum = ksocknal_csum(csum, tx->tx_iov[i].iov_base,
-tx->tx_iov[i].iov_len);
+   csum = crc32_le(csum, tx->tx_iov[i].iov_base,
+   tx->tx_iov[i].iov_len);
}
 
if (*ksocknal_tunables.ksnd_inject_csum_error) {
-- 
1.9.1

[PATCH v4] staging: lustre: lnet: remove dead code and crc32_le() wrapper

[Dmitriy Cherkasov]

After removing code which was permanently disabled with ifdefs, the
function ksocknal_csum() becomes just a wrapper for crc32_le(). Remove
this useless wrapper and instead call crc32_le() directly.

This also resolves the following checkpatch warning which was
triggered by the dead code:

WARNING: space prohibited before semicolon

Signed-off-by: Dmitriy Cherkasov 
---

Changes since v3:
* fixed spacing issues
* fixed typo in commit message

 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h  | 11 ---
 .../staging/lustre/lnet/klnds/socklnd/socklnd_lib.c  | 20 ++--
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h 
b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
index 5540de6..9eb169d 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.h
@@ -519,17 +519,6 @@ struct ksock_proto {
 #define CPU_MASK_NONE   0UL
 #endif
 
-static inline __u32 ksocknal_csum(__u32 crc, unsigned char const *p, size_t 
len)
-{
-#if 1
-   return crc32_le(crc, p, len);
-#else
-   while (len-- > 0)
-   crc = ((crc + 0x100) & ~0xff) | ((crc + *p++) & 0xff) ;
-   return crc;
-#endif
-}
-
 static inline int
 ksocknal_route_mask(void)
 {
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c 
b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
index 8a036f4..41715cd 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib.c
@@ -201,9 +201,9 @@
if (fragnob > sum)
fragnob = sum;
 
-   conn->ksnc_rx_csum = ksocknal_csum(conn->ksnc_rx_csum,
-  iov[i].iov_base,
-  fragnob);
+   conn->ksnc_rx_csum = crc32_le(conn->ksnc_rx_csum,
+   iov[i].iov_base,
+   fragnob);
}
conn->ksnc_msg.ksm_csum = saved_csum;
}
@@ -243,8 +243,8 @@
if (fragnob > sum)
fragnob = sum;
 
-   conn->ksnc_rx_csum = ksocknal_csum(conn->ksnc_rx_csum,
-  base, fragnob);
+   conn->ksnc_rx_csum = crc32_le(conn->ksnc_rx_csum,
+   base, fragnob);
 
kunmap(kiov[i].bv_page);
}
@@ -265,22 +265,22 @@
 
tx->tx_msg.ksm_csum = 0;
 
-   csum = ksocknal_csum(~0, tx->tx_iov[0].iov_base,
-tx->tx_iov[0].iov_len);
+   csum = crc32_le(~0, tx->tx_iov[0].iov_base,
+   tx->tx_iov[0].iov_len);
 
if (tx->tx_kiov) {
for (i = 0; i < tx->tx_nkiov; i++) {
base = kmap(tx->tx_kiov[i].bv_page) +
   tx->tx_kiov[i].bv_offset;
 
-   csum = ksocknal_csum(csum, base, tx->tx_kiov[i].bv_len);
+   csum = crc32_le(csum, base, tx->tx_kiov[i].bv_len);
 
kunmap(tx->tx_kiov[i].bv_page);
}
} else {
for (i = 1; i < tx->tx_niov; i++)
-   csum = ksocknal_csum(csum, tx->tx_iov[i].iov_base,
-tx->tx_iov[i].iov_len);
+   csum = crc32_le(csum, tx->tx_iov[i].iov_base,
+   tx->tx_iov[i].iov_len);
}
 
if (*ksocknal_tunables.ksnd_inject_csum_error) {
-- 
1.9.1

Re: [RFC PATCH v3] acpi: indicate to platform when hot remove returns busy

[joeyli]

Hi Rafael,

On Thu, Jun 29, 2017 at 12:13:18AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 21, 2017 03:45:44 PM Lee, Chun-Yi wrote:
> > In hotplug logic, it always indicates non-specific failure to
> > platform through _OST when handing acpi hot-remove event failed. Then
> > platform terminates the hot-remove process but it can not identify
> > the reason.
> > 
> > Base on current hot-remove code, there have two situations that it
> > returns busy:
> >  - OSPM try to offline an individual device, but the device offline
> >function returns busy.
> >  - When the ejection event is applied to an "not offlined yet" container.
> >OSPM send kobject change event to userspace and returns busy.
> > 
> > Both of them will returns -EBUSY to acpi device hotplug function then
> > hotplug function indicates non-specific failure to platform just like
> > any other error, e.g. -ENODEV or -EIO.
> > 
> > The benefit to platform for identifying the OS busy state is that
> > platform can be applied different approach to handle the busy but
> > not just terminate the hot-remove process by unknown reason. For
> > example, platform can wait for a while then triggers hot-remove
> > again.
> > 
> > This RFC patch adds one more parameter to the handler function of
> > acpi generic hotplug event to give the function a chance to propose
> > the return code of _OST. In this case, it sets ost return code to
> > ACPI_OST_SC_DEVICE_BUSY when the acpi hot remove function returns
> > -EBUSY.
> > 
> > v3:
> > Removed redundant 'else' in acpi_ost_status_code(). (Andy Shevchenko)
> > 
> > v2:
> > Do not overwrite ost code in acpi_generic_hotplug_event(). Move
> > the "error code to ost code" logic to a help function. (Andy Shevchenko)
> > 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Len Brown 
> > Cc: Andy Shevchenko 
> > Cc: Michal Hocko 
> > Signed-off-by: "Lee, Chun-Yi" 
> > ---
> >  drivers/acpi/scan.c | 21 +++--
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 3a10d7573477..41f5065190c7 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -384,6 +384,20 @@ static int acpi_generic_hotplug_event(struct 
> > acpi_device *adev, u32 type)
> > return -EINVAL;
> >  }
> >  
> > +static int acpi_ost_status_code(u32 src, int error)
> > +{
> > +   switch (src) {
> > +   case ACPI_NOTIFY_EJECT_REQUEST:
> > +   case ACPI_OST_EC_OSPM_EJECT:
> > +   if (error == -EPERM)
> > +   return ACPI_OST_SC_EJECT_NOT_SUPPORTED;
> > +   if (error == -EBUSY)
> > +   return ACPI_OST_SC_DEVICE_BUSY;
> > +   break;
> > +   }
> > +   return error ? ACPI_OST_SC_NON_SPECIFIC_FAILURE : ACPI_OST_SC_SUCCESS;
> > +}
> 
> I'm not sure why you need the function above?
> 
> It only has one user anyway.
>

Because different hotplug source event may has different
OST code definition. So I want to use a function to handle
the the complexity. The originial function like this: 

static int acpi_ost_status_code(u32 src, int error)
{
switch (src) {
case ACPI_NOTIFY_EJECT_REQUEST:
case ACPI_OST_EC_OSPM_EJECT:
if (error == -EPERM)
return ACPI_OST_SC_EJECT_NOT_SUPPORTED;
if (error == -EBUSY)
return ACPI_OST_SC_DEVICE_BUSY;
break;
case ACPI_OST_EC_OSPM_INSERTION:
if (error == -EPERM)
return ACPI_OST_SC_INSERT_NOT_SUPPORTED;
if (error == -EBUSY)
return ACPI_OST_SC_INSERT_IN_PROGRESS;
break;
}
return error ? ACPI_OST_SC_NON_SPECIFIC_FAILURE : ACPI_OST_SC_SUCCESS;
}

The ACPI_OST_EC_OSPM_INSERTION represents "Insertion Processing
(Source Event: 0x200)" that it has different ost code definition.
Because the machine supports "Insertion" that has not yet appeared.
So I removed the ACPI_OST_EC_OSPM_INSERTION case block.  
 
> > +
> >  void acpi_device_hotplug(struct acpi_device *adev, u32 src)
> >  {
> > u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
> > @@ -404,10 +418,6 @@ void acpi_device_hotplug(struct acpi_device *adev, u32 
> > src)
> > error = dock_notify(adev, src);
> > } else if (adev->flags.hotplug_notify) {
> > error = acpi_generic_hotplug_event(adev, src);
> > -   if (error == -EPERM) {
> > -   ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
> > -   goto err_out;
> > -   }
> > } else {
> > int (*notify)(struct acpi_device *, u32);
> >  
> > @@ -423,9 +433,8 @@ void acpi_device_hotplug(struct acpi_device *adev, u32 
> > src)
> > else
> > goto out;
> > }
> > -   if (!error)
> > -   ost_code = ACPI_OST_SC_SUCCESS;
> 
> I just would do:
> 
> 

Re: [RFC PATCH v3] acpi: indicate to platform when hot remove returns busy

[joeyli]

Hi Rafael,

On Thu, Jun 29, 2017 at 12:13:18AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 21, 2017 03:45:44 PM Lee, Chun-Yi wrote:
> > In hotplug logic, it always indicates non-specific failure to
> > platform through _OST when handing acpi hot-remove event failed. Then
> > platform terminates the hot-remove process but it can not identify
> > the reason.
> > 
> > Base on current hot-remove code, there have two situations that it
> > returns busy:
> >  - OSPM try to offline an individual device, but the device offline
> >function returns busy.
> >  - When the ejection event is applied to an "not offlined yet" container.
> >OSPM send kobject change event to userspace and returns busy.
> > 
> > Both of them will returns -EBUSY to acpi device hotplug function then
> > hotplug function indicates non-specific failure to platform just like
> > any other error, e.g. -ENODEV or -EIO.
> > 
> > The benefit to platform for identifying the OS busy state is that
> > platform can be applied different approach to handle the busy but
> > not just terminate the hot-remove process by unknown reason. For
> > example, platform can wait for a while then triggers hot-remove
> > again.
> > 
> > This RFC patch adds one more parameter to the handler function of
> > acpi generic hotplug event to give the function a chance to propose
> > the return code of _OST. In this case, it sets ost return code to
> > ACPI_OST_SC_DEVICE_BUSY when the acpi hot remove function returns
> > -EBUSY.
> > 
> > v3:
> > Removed redundant 'else' in acpi_ost_status_code(). (Andy Shevchenko)
> > 
> > v2:
> > Do not overwrite ost code in acpi_generic_hotplug_event(). Move
> > the "error code to ost code" logic to a help function. (Andy Shevchenko)
> > 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Len Brown 
> > Cc: Andy Shevchenko 
> > Cc: Michal Hocko 
> > Signed-off-by: "Lee, Chun-Yi" 
> > ---
> >  drivers/acpi/scan.c | 21 +++--
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 3a10d7573477..41f5065190c7 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -384,6 +384,20 @@ static int acpi_generic_hotplug_event(struct 
> > acpi_device *adev, u32 type)
> > return -EINVAL;
> >  }
> >  
> > +static int acpi_ost_status_code(u32 src, int error)
> > +{
> > +   switch (src) {
> > +   case ACPI_NOTIFY_EJECT_REQUEST:
> > +   case ACPI_OST_EC_OSPM_EJECT:
> > +   if (error == -EPERM)
> > +   return ACPI_OST_SC_EJECT_NOT_SUPPORTED;
> > +   if (error == -EBUSY)
> > +   return ACPI_OST_SC_DEVICE_BUSY;
> > +   break;
> > +   }
> > +   return error ? ACPI_OST_SC_NON_SPECIFIC_FAILURE : ACPI_OST_SC_SUCCESS;
> > +}
> 
> I'm not sure why you need the function above?
> 
> It only has one user anyway.
>

Because different hotplug source event may has different
OST code definition. So I want to use a function to handle
the the complexity. The originial function like this: 

static int acpi_ost_status_code(u32 src, int error)
{
switch (src) {
case ACPI_NOTIFY_EJECT_REQUEST:
case ACPI_OST_EC_OSPM_EJECT:
if (error == -EPERM)
return ACPI_OST_SC_EJECT_NOT_SUPPORTED;
if (error == -EBUSY)
return ACPI_OST_SC_DEVICE_BUSY;
break;
case ACPI_OST_EC_OSPM_INSERTION:
if (error == -EPERM)
return ACPI_OST_SC_INSERT_NOT_SUPPORTED;
if (error == -EBUSY)
return ACPI_OST_SC_INSERT_IN_PROGRESS;
break;
}
return error ? ACPI_OST_SC_NON_SPECIFIC_FAILURE : ACPI_OST_SC_SUCCESS;
}

The ACPI_OST_EC_OSPM_INSERTION represents "Insertion Processing
(Source Event: 0x200)" that it has different ost code definition.
Because the machine supports "Insertion" that has not yet appeared.
So I removed the ACPI_OST_EC_OSPM_INSERTION case block.  
 
> > +
> >  void acpi_device_hotplug(struct acpi_device *adev, u32 src)
> >  {
> > u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
> > @@ -404,10 +418,6 @@ void acpi_device_hotplug(struct acpi_device *adev, u32 
> > src)
> > error = dock_notify(adev, src);
> > } else if (adev->flags.hotplug_notify) {
> > error = acpi_generic_hotplug_event(adev, src);
> > -   if (error == -EPERM) {
> > -   ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
> > -   goto err_out;
> > -   }
> > } else {
> > int (*notify)(struct acpi_device *, u32);
> >  
> > @@ -423,9 +433,8 @@ void acpi_device_hotplug(struct acpi_device *adev, u32 
> > src)
> > else
> > goto out;
> > }
> > -   if (!error)
> > -   ost_code = ACPI_OST_SC_SUCCESS;
> 
> I just would do:
> 
> switch (error) {
> case 0:
> ost_code = ACPI_OST_SC_SUCCESS;
> break;

[PATCH for-4.4 08/16] MIPS: ralink: fix MT7628 pinmux typos

[Amit Pundir]

From: Álvaro Fernández Rojas 

commit d7146829c9da24e285cb1b1f2156b5b3e2d40c07 upstream.

Signed-off-by: Álvaro Fernández Rojas 
Cc: j...@phrozen.org
Cc: linux-m...@linux-mips.org
Cc: linux-kernel@vger.kernel.org
Patchwork: https://patchwork.linux-mips.org/patch/13306/
Signed-off-by: Ralf Baechle 
Signed-off-by: Amit Pundir 
---
 arch/mips/ralink/mt7620.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index 4c17dc6e8ae9..37cfc7d3c185 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -223,9 +223,9 @@ static struct rt2880_pmx_func wled_an_grp_mt7628[] = {
 #define MT7628_GPIO_MODE_GPIO  0
 
 static struct rt2880_pmx_group mt7628an_pinmux_data[] = {
-   GRP_G("pmw1", pwm1_grp_mt7628, MT7628_GPIO_MODE_MASK,
+   GRP_G("pwm1", pwm1_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_PWM1),
-   GRP_G("pmw0", pwm0_grp_mt7628, MT7628_GPIO_MODE_MASK,
+   GRP_G("pwm0", pwm0_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_PWM0),
GRP_G("uart2", uart2_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_UART2),
-- 
2.7.4

[PATCH for-4.4 08/16] MIPS: ralink: fix MT7628 pinmux typos

[Amit Pundir]

From: Álvaro Fernández Rojas 

commit d7146829c9da24e285cb1b1f2156b5b3e2d40c07 upstream.

Signed-off-by: Álvaro Fernández Rojas 
Cc: j...@phrozen.org
Cc: linux-m...@linux-mips.org
Cc: linux-kernel@vger.kernel.org
Patchwork: https://patchwork.linux-mips.org/patch/13306/
Signed-off-by: Ralf Baechle 
Signed-off-by: Amit Pundir 
---
 arch/mips/ralink/mt7620.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index 4c17dc6e8ae9..37cfc7d3c185 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -223,9 +223,9 @@ static struct rt2880_pmx_func wled_an_grp_mt7628[] = {
 #define MT7628_GPIO_MODE_GPIO  0
 
 static struct rt2880_pmx_group mt7628an_pinmux_data[] = {
-   GRP_G("pmw1", pwm1_grp_mt7628, MT7628_GPIO_MODE_MASK,
+   GRP_G("pwm1", pwm1_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_PWM1),
-   GRP_G("pmw0", pwm0_grp_mt7628, MT7628_GPIO_MODE_MASK,
+   GRP_G("pwm0", pwm0_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_PWM0),
GRP_G("uart2", uart2_grp_mt7628, MT7628_GPIO_MODE_MASK,
1, MT7628_GPIO_MODE_UART2),
-- 
2.7.4

[PATCH] sata_rcar: fix error return code in sata_rcar_probe()

[Gustavo A. R. Silva]

Print error message and propagate the return value of
platform_get_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/sata_rcar.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index ee98447..c936b2a 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -872,8 +872,10 @@ static int sata_rcar_probe(struct platform_device *pdev)
int ret = 0;
 
irq = platform_get_irq(pdev, 0);
-   if (irq <= 0)
-   return -EINVAL;
+   if (irq < 0) {
+   dev_err(>dev, "failed to get IRQ\n");
+   return irq;
+   }
 
priv = devm_kzalloc(>dev, sizeof(struct sata_rcar_priv),
   GFP_KERNEL);
-- 
2.5.0

[PATCH] sata_rcar: fix error return code in sata_rcar_probe()

[Gustavo A. R. Silva]

Print error message and propagate the return value of
platform_get_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/sata_rcar.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index ee98447..c936b2a 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -872,8 +872,10 @@ static int sata_rcar_probe(struct platform_device *pdev)
int ret = 0;
 
irq = platform_get_irq(pdev, 0);
-   if (irq <= 0)
-   return -EINVAL;
+   if (irq < 0) {
+   dev_err(>dev, "failed to get IRQ\n");
+   return irq;
+   }
 
priv = devm_kzalloc(>dev, sizeof(struct sata_rcar_priv),
   GFP_KERNEL);
-- 
2.5.0

[PATCH for-4.4 09/16] MIPS: ralink: fix MT7628 wled_an pinmux gpio

[Amit Pundir]

From: Álvaro Fernández Rojas 

commit 07b50db6e685172a41b9978aebffb2438166d9b6 upstream.

Signed-off-by: Álvaro Fernández Rojas 
Cc: j...@phrozen.org
Cc: linux-m...@linux-mips.org
Cc: linux-kernel@vger.kernel.org
Patchwork: https://patchwork.linux-mips.org/patch/13307/
Signed-off-by: Ralf Baechle 
Signed-off-by: Amit Pundir 
---
 arch/mips/ralink/mt7620.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index 37cfc7d3c185..48d6349fd9d7 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -196,10 +196,10 @@ static struct rt2880_pmx_func wled_kn_grp_mt7628[] = {
 };
 
 static struct rt2880_pmx_func wled_an_grp_mt7628[] = {
-   FUNC("rsvd", 3, 35, 1),
-   FUNC("rsvd", 2, 35, 1),
-   FUNC("gpio", 1, 35, 1),
-   FUNC("wled_an", 0, 35, 1),
+   FUNC("rsvd", 3, 44, 1),
+   FUNC("rsvd", 2, 44, 1),
+   FUNC("gpio", 1, 44, 1),
+   FUNC("wled_an", 0, 44, 1),
 };
 
 #define MT7628_GPIO_MODE_MASK  0x3
-- 
2.7.4

[PATCH for-4.4 09/16] MIPS: ralink: fix MT7628 wled_an pinmux gpio

[Amit Pundir]

From: Álvaro Fernández Rojas 

commit 07b50db6e685172a41b9978aebffb2438166d9b6 upstream.

Signed-off-by: Álvaro Fernández Rojas 
Cc: j...@phrozen.org
Cc: linux-m...@linux-mips.org
Cc: linux-kernel@vger.kernel.org
Patchwork: https://patchwork.linux-mips.org/patch/13307/
Signed-off-by: Ralf Baechle 
Signed-off-by: Amit Pundir 
---
 arch/mips/ralink/mt7620.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/ralink/mt7620.c b/arch/mips/ralink/mt7620.c
index 37cfc7d3c185..48d6349fd9d7 100644
--- a/arch/mips/ralink/mt7620.c
+++ b/arch/mips/ralink/mt7620.c
@@ -196,10 +196,10 @@ static struct rt2880_pmx_func wled_kn_grp_mt7628[] = {
 };
 
 static struct rt2880_pmx_func wled_an_grp_mt7628[] = {
-   FUNC("rsvd", 3, 35, 1),
-   FUNC("rsvd", 2, 35, 1),
-   FUNC("gpio", 1, 35, 1),
-   FUNC("wled_an", 0, 35, 1),
+   FUNC("rsvd", 3, 44, 1),
+   FUNC("rsvd", 2, 44, 1),
+   FUNC("gpio", 1, 44, 1),
+   FUNC("wled_an", 0, 44, 1),
 };
 
 #define MT7628_GPIO_MODE_MASK  0x3
-- 
2.7.4

Re: [PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Frans Klaver]

On Fri, Jun 30, 2017 at 6:42 AM, Gustavo A. R. Silva
 wrote:
> Propagate the return values of platform_get_irq and
> devm_request_irq on failure.
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/clocksource/em_sti.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
> index bc48cbf..c4818dd 100644
> --- a/drivers/clocksource/em_sti.c
> +++ b/drivers/clocksource/em_sti.c
> @@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> dev_err(>dev, "failed to get irq\n");
> -   return -EINVAL;
> +   return irq;
> }
>
> /* map memory, let base point to the STI instance */
> @@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
> if (IS_ERR(p->base))
> return PTR_ERR(p->base);
>
> -   if (devm_request_irq(>dev, irq, em_sti_interrupt,
> +   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
>  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
> -dev_name(>dev), p)) {
> +dev_name(>dev), p);
> +   if (irq) {
> dev_err(>dev, "failed to request low IRQ\n");
> -   return -ENOENT;
> +   return irq;
> }

This works. Yet I think that 'ret' would be a better candidate for
taking the result of devm_request_irq, since it doesn't return the irq
number on success. Should someone decide to reference irq at a later
point in the code, this has to be changed.

Re: [PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Frans Klaver]

On Fri, Jun 30, 2017 at 6:42 AM, Gustavo A. R. Silva
 wrote:
> Propagate the return values of platform_get_irq and
> devm_request_irq on failure.
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/clocksource/em_sti.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
> index bc48cbf..c4818dd 100644
> --- a/drivers/clocksource/em_sti.c
> +++ b/drivers/clocksource/em_sti.c
> @@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> dev_err(>dev, "failed to get irq\n");
> -   return -EINVAL;
> +   return irq;
> }
>
> /* map memory, let base point to the STI instance */
> @@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
> if (IS_ERR(p->base))
> return PTR_ERR(p->base);
>
> -   if (devm_request_irq(>dev, irq, em_sti_interrupt,
> +   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
>  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
> -dev_name(>dev), p)) {
> +dev_name(>dev), p);
> +   if (irq) {
> dev_err(>dev, "failed to request low IRQ\n");
> -   return -ENOENT;
> +   return irq;
> }

This works. Yet I think that 'ret' would be a better candidate for
taking the result of devm_request_irq, since it doesn't return the irq
number on success. Should someone decide to reference irq at a later
point in the code, this has to be changed.

Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

[Andy Lutomirski]

On Thu, Jun 29, 2017 at 10:05 PM, Andy Lutomirski  wrote:
> Hmm.  There's another option that might be considerably nicer, though:
> put the IRQ stack at a known (at link time) position *in percpu
> space*.  (Presumably it already is -- I haven't checked.)  Then we do:
>
> .macro ENTER_IRQ_STACK old_rsp
> DEBUG_ENTRY_ASSERT_IRQS_OFF
> movq%rsp, \old_rsp
> inclPER_CPU_VAR(irq_count)
>
> /*
>  * Right now, if we just incremented irq_count to zero, we've
>  * claimed the IRQ stack but we haven't switched to it yet.
>  * Anything that can interrupt us here without using IST
>  * must be *extremely* careful to limit its stack usage.
>  */
> jnz .Lpush_old_rsp_\@
> movq\old_rsp, PER_CPU_VAR(top_word_in_irq_stack)
> movqPER_CPU_VAR(irq_stack_ptr), %rsp
> .Lpush_old_rsp_\@:
> pushq\old_rsp
> .endm
>

How about the two commits here (well, soon to be there once gitweb catches up):

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_irq_stack=0f56a55bb133cd53ccb78ca51378086296618322

If you like them, want to add them to your series?

Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

[Andy Lutomirski]

On Thu, Jun 29, 2017 at 10:05 PM, Andy Lutomirski  wrote:
> Hmm.  There's another option that might be considerably nicer, though:
> put the IRQ stack at a known (at link time) position *in percpu
> space*.  (Presumably it already is -- I haven't checked.)  Then we do:
>
> .macro ENTER_IRQ_STACK old_rsp
> DEBUG_ENTRY_ASSERT_IRQS_OFF
> movq%rsp, \old_rsp
> inclPER_CPU_VAR(irq_count)
>
> /*
>  * Right now, if we just incremented irq_count to zero, we've
>  * claimed the IRQ stack but we haven't switched to it yet.
>  * Anything that can interrupt us here without using IST
>  * must be *extremely* careful to limit its stack usage.
>  */
> jnz .Lpush_old_rsp_\@
> movq\old_rsp, PER_CPU_VAR(top_word_in_irq_stack)
> movqPER_CPU_VAR(irq_stack_ptr), %rsp
> .Lpush_old_rsp_\@:
> pushq\old_rsp
> .endm
>

How about the two commits here (well, soon to be there once gitweb catches up):

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_irq_stack=0f56a55bb133cd53ccb78ca51378086296618322

If you like them, want to add them to your series?

Re: [PATCH 3/6] cpufreq: governor: Drop min_sampling_rate

[Viresh Kumar]

On 30-06-17, 06:53, Dominik Brodowski wrote:
> On Fri, Jun 30, 2017 at 09:04:25AM +0530, Viresh Kumar wrote:
> > On 29-06-17, 20:01, Dominik Brodowski wrote:
> > > On Thu, Jun 29, 2017 at 04:29:06PM +0530, Viresh Kumar wrote:
> > > > The cpufreq core and governors aren't supposed to set a limit on how
> > > > fast we want to try changing the frequency. This is currently done for
> > > > the legacy governors with help of min_sampling_rate.
> > > > 
> > > > At worst, we may end up setting the sampling rate to a value lower than
> > > > the rate at which frequency can be changed and then one of the CPUs in
> > > > the policy will be only changing frequency for ever.
> > > 
> > > Is it safe to issue requests to change the CPU frequency so frequently,
> > 
> > Well, I assumed so. I am not sure the hardware would break though.
> > Overheating ?
> > 
> > > even
> > > on historic hardware such as speedstep-{ich,smi,centrino}? In the past,

speedstep-smi is the only one which sets transition_latency to
CPUFREQ_ETERNAL and the others are putting some meaningful values. So
yes, they should be doing DVFS dynamically.

> > > these checks more or less disallowed the running of dynamic frequency
> > > scaling at least on speedstep-smi[*],
> > 
> > We must by doing dynamic freq scaling even without this patch. I don't
> > see why you say the above then.
> > 
> > All we do here is that we get rid of the limit on how soon we can
> > change the freq again.
> 
> Well, as I understand it, first generation "speedstep" was designed more or
> less to switch frequencies only when AC power was lost or restored.
> 
> The Linux implementation merely said: "no on-the-fly changes", but switch
> frequencies whenever a user explicitly requested such a change (presumably
> only every once in an unspecified while).
> 
> This same reasoning may be present in other drivers using CPUFREQ_ETERNAL.

Thanks for the explanation here and I am convinced that this series
has at least done one thing wrong. And that is removal of
max_transition_latency from governors and allowing ondemand to run on
such platforms (which may end up breaking them).

So I will actually modify that patch and set max_transition_latency to
CPUFREQ_ETERNAL for ondemand/conservative instead of 10ms. Also we
should do the same for schedutil as well, so that will also use the
max_transition_latency field.

But I hope, this patch will still be fine. Right ?

> I am not *sure* either, I am just worried of the consequences of doing
> things out-of-spec...

Thanks for your inputs Dominik.

-- 
viresh

Re: [PATCH 3/6] cpufreq: governor: Drop min_sampling_rate

[Viresh Kumar]

On 30-06-17, 06:53, Dominik Brodowski wrote:
> On Fri, Jun 30, 2017 at 09:04:25AM +0530, Viresh Kumar wrote:
> > On 29-06-17, 20:01, Dominik Brodowski wrote:
> > > On Thu, Jun 29, 2017 at 04:29:06PM +0530, Viresh Kumar wrote:
> > > > The cpufreq core and governors aren't supposed to set a limit on how
> > > > fast we want to try changing the frequency. This is currently done for
> > > > the legacy governors with help of min_sampling_rate.
> > > > 
> > > > At worst, we may end up setting the sampling rate to a value lower than
> > > > the rate at which frequency can be changed and then one of the CPUs in
> > > > the policy will be only changing frequency for ever.
> > > 
> > > Is it safe to issue requests to change the CPU frequency so frequently,
> > 
> > Well, I assumed so. I am not sure the hardware would break though.
> > Overheating ?
> > 
> > > even
> > > on historic hardware such as speedstep-{ich,smi,centrino}? In the past,

speedstep-smi is the only one which sets transition_latency to
CPUFREQ_ETERNAL and the others are putting some meaningful values. So
yes, they should be doing DVFS dynamically.

> > > these checks more or less disallowed the running of dynamic frequency
> > > scaling at least on speedstep-smi[*],
> > 
> > We must by doing dynamic freq scaling even without this patch. I don't
> > see why you say the above then.
> > 
> > All we do here is that we get rid of the limit on how soon we can
> > change the freq again.
> 
> Well, as I understand it, first generation "speedstep" was designed more or
> less to switch frequencies only when AC power was lost or restored.
> 
> The Linux implementation merely said: "no on-the-fly changes", but switch
> frequencies whenever a user explicitly requested such a change (presumably
> only every once in an unspecified while).
> 
> This same reasoning may be present in other drivers using CPUFREQ_ETERNAL.

Thanks for the explanation here and I am convinced that this series
has at least done one thing wrong. And that is removal of
max_transition_latency from governors and allowing ondemand to run on
such platforms (which may end up breaking them).

So I will actually modify that patch and set max_transition_latency to
CPUFREQ_ETERNAL for ondemand/conservative instead of 10ms. Also we
should do the same for schedutil as well, so that will also use the
max_transition_latency field.

But I hope, this patch will still be fine. Right ?

> I am not *sure* either, I am just worried of the consequences of doing
> things out-of-spec...

Thanks for your inputs Dominik.

-- 
viresh

Re: [PATCH 00/15] HMM (Heterogeneous Memory Management) v24

[John Hubbard]

On 06/28/2017 11:00 AM, Jérôme Glisse wrote:
> 
> Patchset is on top of git://git.cmpxchg.org/linux-mmotm.git so i
> test same kernel as kbuild system, git branch:
> 
> https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-v24
> 
> Change since v23 is code comment fixes, simplify kernel configuration and
> improve allocation of new page on migration do device memory (last patch
> in this patchset).

Hi Jerome,

Tiny note: one more change is that hmm_devmem_fault_range() has been
removed (and thanks for taking care of that, btw).

Anyway, this looks good. A basic smoke test shows the following:

1. We definitely *require* your other patch, 
"[PATCH] x86/mm/hotplug: fix BUG_ON() after hotremove by not freeing pud v3",
otherwise I will reliably hit that bug every time I run my simple page fault
test. So, let me know if I should ping that thread. It looks like your patch
was not rejected, but I can't tell if (!rejected == accepted), there. :)

We'll continue testing, but I expect at this point that anything we find
can be patched up after HMM finally gets merged.

thanks,
John Hubbard
NVIDIA

> 
> Everything else is the same. Below is the long description of what HMM
> is about and why. At the end of this email i describe briefly each patch
> and suggest reviewers for each of them.
> 
> 
> Heterogeneous Memory Management (HMM) (description and justification)
> 
> Today device driver expose dedicated memory allocation API through their
> device file, often relying on a combination of IOCTL and mmap calls. The
> device can only access and use memory allocated through this API. This
> effectively split the program address space into object allocated for the
> device and useable by the device and other regular memory (malloc, mmap
> of a file, share memory, â) only accessible by CPU (or in a very limited
> way by a device by pinning memory).
> 
> Allowing different isolated component of a program to use a device thus
> require duplication of the input data structure using device memory
> allocator. This is reasonable for simple data structure (array, grid,
> image, â) but this get extremely complex with advance data structure
> (list, tree, graph, â) that rely on a web of memory pointers. This is
> becoming a serious limitation on the kind of work load that can be
> offloaded to device like GPU.
> 
> New industry standard like C++, OpenCL or CUDA are pushing to remove this
> barrier. This require a shared address space between GPU device and CPU so
> that GPU can access any memory of a process (while still obeying memory
> protection like read only). This kind of feature is also appearing in
> various other operating systems.
> 
> HMM is a set of helpers to facilitate several aspects of address space
> sharing and device memory management. Unlike existing sharing mechanism
> that rely on pining pages use by a device, HMM relies on mmu_notifier to
> propagate CPU page table update to device page table.
> 
> Duplicating CPU page table is only one aspect necessary for efficiently
> using device like GPU. GPU local memory have bandwidth in the TeraBytes/
> second range but they are connected to main memory through a system bus
> like PCIE that is limited to 32GigaBytes/second (PCIE 4.0 16x). Thus it
> is necessary to allow migration of process memory from main system memory
> to device memory. Issue is that on platform that only have PCIE the device
> memory is not accessible by the CPU with the same properties as main
> memory (cache coherency, atomic operations, ...).
> 
> To allow migration from main memory to device memory HMM provides a set
> of helper to hotplug device memory as a new type of ZONE_DEVICE memory
> which is un-addressable by CPU but still has struct page representing it.
> This allow most of the core kernel logic that deals with a process memory
> to stay oblivious of the peculiarity of device memory.
> 
> When page backing an address of a process is migrated to device memory
> the CPU page table entry is set to a new specific swap entry. CPU access
> to such address triggers a migration back to system memory, just like if
> the page was swap on disk. HMM also blocks any one from pinning a
> ZONE_DEVICE page so that it can always be migrated back to system memory
> if CPU access it. Conversely HMM does not migrate to device memory any
> page that is pin in system memory.
> 
> To allow efficient migration between device memory and main memory a new
> migrate_vma() helpers is added with this patchset. It allows to leverage
> device DMA engine to perform the copy operation.
> 
> This feature will be use by upstream driver like nouveau mlx5 and probably
> other in the future (amdgpu is next suspect in line). We are actively
> working on nouveau and mlx5 support. To test this patchset we also worked
> with NVidia close source driver team, they have more resources than us to
> test this kind of infrastructure and also a bigger and better userspace
> eco-system with various real industry workload they 

Re: [PATCH 00/15] HMM (Heterogeneous Memory Management) v24

[John Hubbard]

On 06/28/2017 11:00 AM, Jérôme Glisse wrote:
> 
> Patchset is on top of git://git.cmpxchg.org/linux-mmotm.git so i
> test same kernel as kbuild system, git branch:
> 
> https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-v24
> 
> Change since v23 is code comment fixes, simplify kernel configuration and
> improve allocation of new page on migration do device memory (last patch
> in this patchset).

Hi Jerome,

Tiny note: one more change is that hmm_devmem_fault_range() has been
removed (and thanks for taking care of that, btw).

Anyway, this looks good. A basic smoke test shows the following:

1. We definitely *require* your other patch, 
"[PATCH] x86/mm/hotplug: fix BUG_ON() after hotremove by not freeing pud v3",
otherwise I will reliably hit that bug every time I run my simple page fault
test. So, let me know if I should ping that thread. It looks like your patch
was not rejected, but I can't tell if (!rejected == accepted), there. :)

We'll continue testing, but I expect at this point that anything we find
can be patched up after HMM finally gets merged.

thanks,
John Hubbard
NVIDIA

> 
> Everything else is the same. Below is the long description of what HMM
> is about and why. At the end of this email i describe briefly each patch
> and suggest reviewers for each of them.
> 
> 
> Heterogeneous Memory Management (HMM) (description and justification)
> 
> Today device driver expose dedicated memory allocation API through their
> device file, often relying on a combination of IOCTL and mmap calls. The
> device can only access and use memory allocated through this API. This
> effectively split the program address space into object allocated for the
> device and useable by the device and other regular memory (malloc, mmap
> of a file, share memory, â) only accessible by CPU (or in a very limited
> way by a device by pinning memory).
> 
> Allowing different isolated component of a program to use a device thus
> require duplication of the input data structure using device memory
> allocator. This is reasonable for simple data structure (array, grid,
> image, â) but this get extremely complex with advance data structure
> (list, tree, graph, â) that rely on a web of memory pointers. This is
> becoming a serious limitation on the kind of work load that can be
> offloaded to device like GPU.
> 
> New industry standard like C++, OpenCL or CUDA are pushing to remove this
> barrier. This require a shared address space between GPU device and CPU so
> that GPU can access any memory of a process (while still obeying memory
> protection like read only). This kind of feature is also appearing in
> various other operating systems.
> 
> HMM is a set of helpers to facilitate several aspects of address space
> sharing and device memory management. Unlike existing sharing mechanism
> that rely on pining pages use by a device, HMM relies on mmu_notifier to
> propagate CPU page table update to device page table.
> 
> Duplicating CPU page table is only one aspect necessary for efficiently
> using device like GPU. GPU local memory have bandwidth in the TeraBytes/
> second range but they are connected to main memory through a system bus
> like PCIE that is limited to 32GigaBytes/second (PCIE 4.0 16x). Thus it
> is necessary to allow migration of process memory from main system memory
> to device memory. Issue is that on platform that only have PCIE the device
> memory is not accessible by the CPU with the same properties as main
> memory (cache coherency, atomic operations, ...).
> 
> To allow migration from main memory to device memory HMM provides a set
> of helper to hotplug device memory as a new type of ZONE_DEVICE memory
> which is un-addressable by CPU but still has struct page representing it.
> This allow most of the core kernel logic that deals with a process memory
> to stay oblivious of the peculiarity of device memory.
> 
> When page backing an address of a process is migrated to device memory
> the CPU page table entry is set to a new specific swap entry. CPU access
> to such address triggers a migration back to system memory, just like if
> the page was swap on disk. HMM also blocks any one from pinning a
> ZONE_DEVICE page so that it can always be migrated back to system memory
> if CPU access it. Conversely HMM does not migrate to device memory any
> page that is pin in system memory.
> 
> To allow efficient migration between device memory and main memory a new
> migrate_vma() helpers is added with this patchset. It allows to leverage
> device DMA engine to perform the copy operation.
> 
> This feature will be use by upstream driver like nouveau mlx5 and probably
> other in the future (amdgpu is next suspect in line). We are actively
> working on nouveau and mlx5 support. To test this patchset we also worked
> with NVidia close source driver team, they have more resources than us to
> test this kind of infrastructure and also a bigger and better userspace
> eco-system with various real industry workload they 

[PATCH] fs: GFS2: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   52591344   8661119d3 fs/gfs2/sys.o

File size After adding 'const':
   textdata bss dec hex filename
   53711216   8659519c3 fs/gfs2/sys.o

Signed-off-by: Arvind Yadav 
---
 fs/gfs2/sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 7a51534..01dc027 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -637,12 +637,12 @@ static ssize_t tune_set(struct gfs2_sbd *sdp, unsigned 
int *field,
NULL,
 };
 
-static struct attribute_group tune_group = {
+static const struct attribute_group tune_group = {
.name = "tune",
.attrs = tune_attrs,
 };
 
-static struct attribute_group lock_module_group = {
+static const struct attribute_group lock_module_group = {
.name = "lock_module",
.attrs = lock_module_attrs,
 };
-- 
1.9.1

[PATCH] fs: GFS2: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   52591344   8661119d3 fs/gfs2/sys.o

File size After adding 'const':
   textdata bss dec hex filename
   53711216   8659519c3 fs/gfs2/sys.o

Signed-off-by: Arvind Yadav 
---
 fs/gfs2/sys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 7a51534..01dc027 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -637,12 +637,12 @@ static ssize_t tune_set(struct gfs2_sbd *sdp, unsigned 
int *field,
NULL,
 };
 
-static struct attribute_group tune_group = {
+static const struct attribute_group tune_group = {
.name = "tune",
.attrs = tune_attrs,
 };
 
-static struct attribute_group lock_module_group = {
+static const struct attribute_group lock_module_group = {
.name = "lock_module",
.attrs = lock_module_attrs,
 };
-- 
1.9.1

[PATCH] pata_imx: print error message on platform_get_irq failure

[Gustavo A. R. Silva]

Print error message on platform_get_irq failure before return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/pata_imx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
index d4caa23..65bbf36 100644
--- a/drivers/ata/pata_imx.c
+++ b/drivers/ata/pata_imx.c
@@ -132,8 +132,10 @@ static int pata_imx_probe(struct platform_device *pdev)
int ret;
 
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
+   if (irq < 0) {
+   dev_err(>dev, "failed to get IRQ\n");
return irq;
+   }
 
priv = devm_kzalloc(>dev,
sizeof(struct pata_imx_priv), GFP_KERNEL);
-- 
2.5.0

[PATCH] pata_imx: print error message on platform_get_irq failure

[Gustavo A. R. Silva]

Print error message on platform_get_irq failure before return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/pata_imx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
index d4caa23..65bbf36 100644
--- a/drivers/ata/pata_imx.c
+++ b/drivers/ata/pata_imx.c
@@ -132,8 +132,10 @@ static int pata_imx_probe(struct platform_device *pdev)
int ret;
 
irq = platform_get_irq(pdev, 0);
-   if (irq < 0)
+   if (irq < 0) {
+   dev_err(>dev, "failed to get IRQ\n");
return irq;
+   }
 
priv = devm_kzalloc(>dev,
sizeof(struct pata_imx_priv), GFP_KERNEL);
-- 
2.5.0

Re: [linux-next] cpus stalls detected few hours after booting next kernel

[Abdul Haleem]

On Fri, 2017-06-30 at 00:45 +1000, Nicholas Piggin wrote:
> On Thu, 29 Jun 2017 20:23:05 +1000
> Nicholas Piggin  wrote:
> 
> > On Thu, 29 Jun 2017 19:36:14 +1000
> > Nicholas Piggin  wrote:
> 
> > > I don't *think* the replay-wakeup-interrupt patch is directly involved, 
> > > but
> > > it's likely to be one of the idle patches.  
> 
> Okay this turned out to be misconfigured sleep states I added for the
> simulator, sorry for the false alarm.
> 
> > Although you have this in the backtrace. I wonder if that's a stuck
> > lock in rcu_process_callbacks?
> 
> So this spinlock becomes top of the list of suspects. Can you try
> enabling lockdep and try to reproduce it?

Yes, recreated again with CONFIG_LOCKDEP=y & CONFIG_DEBUG_LOCKDEP=y set.
I do not see any difference in trace messages with and without LOCKDEP
enabled.

Please find the attached log file.

> 
> > [ 5948.345539] CPU: 63 PID: 7360 Comm: rs:main Q:Reg Not tainted \
> > 4.12.0-rc7-next-20170628 #2 [ 5948.345612] task: c00f1c14e600 
> > task.stack: \
> > c00f1c1e8000 [ 5948.345672] NIP: c09c7a10 LR: c09c7a08 
> > CTR: \
> > c015eda0 [ 5948.358553] REGS: c00f1c1eb150 TRAP: 0501   Not 
> > tainted  \
> > (4.12.0-rc7-next-20170628) [ 5948.358623] MSR: 90009033 \
> >  [ 5948.358626]   CR: 28082884  XER: 2000
> > [ 5948.358706] CFAR: c09c7a28 SOFTE: 1 
> > GPR00: c015f14c c00f1c1eb3d0 c1062b00 0001 
> > GPR04: c00fff6e6180  0001 00cc 
> > GPR08: 0001 804a   
> > GPR12: c015eda0 cfd55a80 
> > [ 5948.358986] NIP [c09c7a10] _raw_spin_lock_irqsave+0x90/0x100
> > [ 5948.359043] LR [c09c7a08] _raw_spin_lock_irqsave+0x88/0x100
> > [ 5948.359099] Call Trace:
> > [ 5948.359123] [c00f1c1eb3d0] [0001] 0x1 (unreliable)
> > [ 5948.359182] [c00f1c1eb410] [c015f14c] \
> d> rcu_process_callbacks+0x3ac/0x620 [ 5948.359252] [c00f1c1eb4c0] \
> > [c00e1e0c] __do_softirq+0x14c/0x3a0 [ 5948.365958] 
> > [c00f1c1eb5b0] \
> > [c00e2448] irq_exit+0x108/0x120 [ 5948.366016] [c00f1c1eb5d0] \
> > [c00232b4] timer_interrupt+0xa4/0xe0 [ 5948.366075] 
> > [c00f1c1eb600] \
> > [c0009208] decrementer_common+0x158/0x160 [ 5948.366149] --- 
> > interrupt: 901 \
> > at start_this_handle+0xd0/0x4b0  LR = jbd2__journal_start+0x17c/0x2b0
> > [ 5948.366242] [c00f1c1eb8f0] [c00f20a5cb00] 0xc00f20a5cb00 
> > (unreliable)
> > [ 5948.366314] [c00f1c1eba00] [c040717c] 
> > jbd2__journal_start+0x17c/0x2b0
> > [ 5948.366388] [c00f1c1eba70] [c038edf4] \
> > __ext4_journal_start_sb+0x84/0x180 [ 5948.366459] [c00f1c1ebad0] \
> > [c03b47dc] ext4_da_write_begin+0x17c/0x520 [ 5948.366532] 
> > [c00f1c1ebb90] \
> > [c021f9c8] generic_perform_write+0xe8/0x250 [ 5948.366604] 
> > [c00f1c1ebc20] \
> > [c0220d20] __generic_file_write_iter+0x200/0x240 [ 5948.366677] \
> > [c00f1c1ebc80] [c039d614] ext4_file_write_iter+0x2e4/0x4d0 [ 
> > 5948.373255] \
> > [c00f1c1ebd00] [c02e13c0] __vfs_write+0x120/0x200 [ 
> > 5948.373313] \
> > [c00f1c1ebd90] [c02e2c48] vfs_write+0xc8/0x240 [ 5948.373371] \
> > [c00f1c1ebde0] [c02e4940] SyS_write+0x60/0x110 [ 5948.373430] \
> > [c00f1c1ebe30] [c000b8e0] system_call+0x38/0xdc [ 5948.373486] \
> > Instruction dump: [ 5948.373521] 7fe3fb78 e8010010 eba1ffe8 ebc1fff0 
> > ebe1fff8 \
> > 7c0803a6 4e800020 8bad028a  [ 5948.373592] 7fe3fb78 4b64db15 6000 
> > 7c210b78 \
> >  89290009 792affe3 40820048  [ 5948.374515] Sending NMI from CPU 
> > 74 to CPUs \
> > 
> 


-- 
Regard's

Abdul Haleem
IBM Linux Technology Centre


[ 9296.490900] INFO: rcu_sched detected stalls on CPUs/tasks:
[ 9296.491039]  8-...: (18788 GPs behind) idle=0cc/0/0 softirq=653/653 fqs=0 
[ 9296.491081]  9-...: (18788 GPs behind) idle=860/0/0 softirq=702/702 fqs=0 
[ 9296.491114]  10-...: (47558 GPs behind) idle=f90/0/0 softirq=493/493 fqs=0 
[ 9296.491147]  14-...: (18791 GPs behind) idle=840/0/0 softirq=816/816 fqs=0 
[ 9296.491180]  17-...: (1976 GPs behind) idle=0ec/0/0 softirq=1147/1147 fqs=0 
[ 9296.491221]  19-...: (60 GPs behind) idle=6a8/0/0 softirq=1551/1551 fqs=0 
[ 9296.491256]  23-...: (965 GPs behind) idle=f2c/0/0 softirq=1130/1131 fqs=0 
[ 9296.491289]  25-...: (37469 GPs behind) idle=170/0/0 softirq=599/599 fqs=0 
[ 9296.491444]  27-...: (47543 GPs behind) idle=3b0/0/0 softirq=606/606 fqs=0 
[ 9296.491479]  28-...: (47541 GPs behind) idle=5d4/0/0 softirq=503/504 fqs=0 
[ 9296.491514]  29-...: (47541 GPs behind) idle=8b4/0/0 softirq=470/470 fqs=0 
[ 9296.491547]  32-...: (2324 GPs behind) idle=7c4/0/0 softirq=946/946 fqs=0 
[ 9296.491580]  33-...: (37473 GPs behind) idle=5a0/0/0 softirq=618/618 fqs=0 
[ 

Re: [linux-next] cpus stalls detected few hours after booting next kernel

[Abdul Haleem]

On Fri, 2017-06-30 at 00:45 +1000, Nicholas Piggin wrote:
> On Thu, 29 Jun 2017 20:23:05 +1000
> Nicholas Piggin  wrote:
> 
> > On Thu, 29 Jun 2017 19:36:14 +1000
> > Nicholas Piggin  wrote:
> 
> > > I don't *think* the replay-wakeup-interrupt patch is directly involved, 
> > > but
> > > it's likely to be one of the idle patches.  
> 
> Okay this turned out to be misconfigured sleep states I added for the
> simulator, sorry for the false alarm.
> 
> > Although you have this in the backtrace. I wonder if that's a stuck
> > lock in rcu_process_callbacks?
> 
> So this spinlock becomes top of the list of suspects. Can you try
> enabling lockdep and try to reproduce it?

Yes, recreated again with CONFIG_LOCKDEP=y & CONFIG_DEBUG_LOCKDEP=y set.
I do not see any difference in trace messages with and without LOCKDEP
enabled.

Please find the attached log file.

> 
> > [ 5948.345539] CPU: 63 PID: 7360 Comm: rs:main Q:Reg Not tainted \
> > 4.12.0-rc7-next-20170628 #2 [ 5948.345612] task: c00f1c14e600 
> > task.stack: \
> > c00f1c1e8000 [ 5948.345672] NIP: c09c7a10 LR: c09c7a08 
> > CTR: \
> > c015eda0 [ 5948.358553] REGS: c00f1c1eb150 TRAP: 0501   Not 
> > tainted  \
> > (4.12.0-rc7-next-20170628) [ 5948.358623] MSR: 90009033 \
> >  [ 5948.358626]   CR: 28082884  XER: 2000
> > [ 5948.358706] CFAR: c09c7a28 SOFTE: 1 
> > GPR00: c015f14c c00f1c1eb3d0 c1062b00 0001 
> > GPR04: c00fff6e6180  0001 00cc 
> > GPR08: 0001 804a   
> > GPR12: c015eda0 cfd55a80 
> > [ 5948.358986] NIP [c09c7a10] _raw_spin_lock_irqsave+0x90/0x100
> > [ 5948.359043] LR [c09c7a08] _raw_spin_lock_irqsave+0x88/0x100
> > [ 5948.359099] Call Trace:
> > [ 5948.359123] [c00f1c1eb3d0] [0001] 0x1 (unreliable)
> > [ 5948.359182] [c00f1c1eb410] [c015f14c] \
> d> rcu_process_callbacks+0x3ac/0x620 [ 5948.359252] [c00f1c1eb4c0] \
> > [c00e1e0c] __do_softirq+0x14c/0x3a0 [ 5948.365958] 
> > [c00f1c1eb5b0] \
> > [c00e2448] irq_exit+0x108/0x120 [ 5948.366016] [c00f1c1eb5d0] \
> > [c00232b4] timer_interrupt+0xa4/0xe0 [ 5948.366075] 
> > [c00f1c1eb600] \
> > [c0009208] decrementer_common+0x158/0x160 [ 5948.366149] --- 
> > interrupt: 901 \
> > at start_this_handle+0xd0/0x4b0  LR = jbd2__journal_start+0x17c/0x2b0
> > [ 5948.366242] [c00f1c1eb8f0] [c00f20a5cb00] 0xc00f20a5cb00 
> > (unreliable)
> > [ 5948.366314] [c00f1c1eba00] [c040717c] 
> > jbd2__journal_start+0x17c/0x2b0
> > [ 5948.366388] [c00f1c1eba70] [c038edf4] \
> > __ext4_journal_start_sb+0x84/0x180 [ 5948.366459] [c00f1c1ebad0] \
> > [c03b47dc] ext4_da_write_begin+0x17c/0x520 [ 5948.366532] 
> > [c00f1c1ebb90] \
> > [c021f9c8] generic_perform_write+0xe8/0x250 [ 5948.366604] 
> > [c00f1c1ebc20] \
> > [c0220d20] __generic_file_write_iter+0x200/0x240 [ 5948.366677] \
> > [c00f1c1ebc80] [c039d614] ext4_file_write_iter+0x2e4/0x4d0 [ 
> > 5948.373255] \
> > [c00f1c1ebd00] [c02e13c0] __vfs_write+0x120/0x200 [ 
> > 5948.373313] \
> > [c00f1c1ebd90] [c02e2c48] vfs_write+0xc8/0x240 [ 5948.373371] \
> > [c00f1c1ebde0] [c02e4940] SyS_write+0x60/0x110 [ 5948.373430] \
> > [c00f1c1ebe30] [c000b8e0] system_call+0x38/0xdc [ 5948.373486] \
> > Instruction dump: [ 5948.373521] 7fe3fb78 e8010010 eba1ffe8 ebc1fff0 
> > ebe1fff8 \
> > 7c0803a6 4e800020 8bad028a  [ 5948.373592] 7fe3fb78 4b64db15 6000 
> > 7c210b78 \
> >  89290009 792affe3 40820048  [ 5948.374515] Sending NMI from CPU 
> > 74 to CPUs \
> > 
> 


-- 
Regard's

Abdul Haleem
IBM Linux Technology Centre


[ 9296.490900] INFO: rcu_sched detected stalls on CPUs/tasks:
[ 9296.491039]  8-...: (18788 GPs behind) idle=0cc/0/0 softirq=653/653 fqs=0 
[ 9296.491081]  9-...: (18788 GPs behind) idle=860/0/0 softirq=702/702 fqs=0 
[ 9296.491114]  10-...: (47558 GPs behind) idle=f90/0/0 softirq=493/493 fqs=0 
[ 9296.491147]  14-...: (18791 GPs behind) idle=840/0/0 softirq=816/816 fqs=0 
[ 9296.491180]  17-...: (1976 GPs behind) idle=0ec/0/0 softirq=1147/1147 fqs=0 
[ 9296.491221]  19-...: (60 GPs behind) idle=6a8/0/0 softirq=1551/1551 fqs=0 
[ 9296.491256]  23-...: (965 GPs behind) idle=f2c/0/0 softirq=1130/1131 fqs=0 
[ 9296.491289]  25-...: (37469 GPs behind) idle=170/0/0 softirq=599/599 fqs=0 
[ 9296.491444]  27-...: (47543 GPs behind) idle=3b0/0/0 softirq=606/606 fqs=0 
[ 9296.491479]  28-...: (47541 GPs behind) idle=5d4/0/0 softirq=503/504 fqs=0 
[ 9296.491514]  29-...: (47541 GPs behind) idle=8b4/0/0 softirq=470/470 fqs=0 
[ 9296.491547]  32-...: (2324 GPs behind) idle=7c4/0/0 softirq=946/946 fqs=0 
[ 9296.491580]  33-...: (37473 GPs behind) idle=5a0/0/0 softirq=618/618 fqs=0 
[ 9296.491616]  37-...: (9317 GPs behind) idle=c98/0/0 softirq=679/679 

Re: [PATCH] powerpc: dts: use #include "..." to include local DT

[Masahiro Yamada]

Hi Michael,

Ping.  Please apply this patch.

I need this to clean up Makefiles in the next development cycle.



2017-06-21 18:52 GMT+09:00 Masahiro Yamada :
> 2017-06-21 18:48 GMT+09:00 Michael Ellerman :
>> Masahiro Yamada  writes:
>>> 2017-06-14 15:45 GMT+09:00 Michael Ellerman :

 Acked-by: Michael Ellerman 
>>>
>>> I have not seen it in linux-next yet.
>>>
>>> Who will pick it up?
>>
>> In the original patch you said:
>>
>>   Fix them to remove -I$(srctree)/arch/$(SRCARCH)/boot/dts path from
>>   dtc_cpp_flags.
>>
>> So I assumed there was a series somewhere that did that and included
>> this patch.
>>
>> But if there isn't then I can just merge it.
>>
>
> Yes, please apply.
>
>
> At first, I sent a series including ARM, PowerPC, Makefile changes,
> but I decided to split per-subsystem after all.
>
> Sorry for confusion.
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] powerpc: dts: use #include "..." to include local DT

[Masahiro Yamada]

Hi Michael,

Ping.  Please apply this patch.

I need this to clean up Makefiles in the next development cycle.



2017-06-21 18:52 GMT+09:00 Masahiro Yamada :
> 2017-06-21 18:48 GMT+09:00 Michael Ellerman :
>> Masahiro Yamada  writes:
>>> 2017-06-14 15:45 GMT+09:00 Michael Ellerman :

 Acked-by: Michael Ellerman 
>>>
>>> I have not seen it in linux-next yet.
>>>
>>> Who will pick it up?
>>
>> In the original patch you said:
>>
>>   Fix them to remove -I$(srctree)/arch/$(SRCARCH)/boot/dts path from
>>   dtc_cpp_flags.
>>
>> So I assumed there was a series somewhere that did that and included
>> this patch.
>>
>> But if there isn't then I can just merge it.
>>
>
> Yes, please apply.
>
>
> At first, I sent a series including ARM, PowerPC, Makefile changes,
> but I decided to split per-subsystem after all.
>
> Sorry for confusion.
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Best Regards
Masahiro Yamada

Re: [RFC] tracing: Add support for critical section event tracing

[Joel Fernandes]

Hi Steven,

Did you have any comments about this patch? It was sent a while ago
and if you can provide me your initial thoughts on it, I would
appreciate it. (Sorry to ping you about it during the busy merge
window time, but I was thinking if I can get any initial early
comments from you then I can get to work on it and finish in time well
before the next merge window).

Regards,
Joel

On Tue, Apr 11, 2017 at 10:38 PM, Joel Fernandes  wrote:
> Critical section trace events can be used for tracing the start and
> end of a critical section which can be used by a trace viewer like
> systrace to graphically view the start and end of a critical section and
> correlate them with latencies and scheduling issues.
>
> Reason for starting critical section:
> Either preemption or interrupts were turned off.
> Reason for ending critical section:
> Both preemption and interrupts are now turned back on.
>
> Cc: Steven Rostedt 
> Signed-off-by: Joel Fernandes 
> --
> Here's an example of how Android's systrace will be using it to show critical
> sections as a gantt chart: http://imgur.com/download/TZplEVp
> ---
>  include/trace/events/critical.h | 55 +++
>  kernel/trace/Kconfig|  6 
>  kernel/trace/trace_irqsoff.c| 73 
> +
>  3 files changed, 134 insertions(+)
>  create mode 100644 include/trace/events/critical.h
>
> diff --git a/include/trace/events/critical.h b/include/trace/events/critical.h
> new file mode 100644
> index ..bfd58dd4f48f
> --- /dev/null
> +++ b/include/trace/events/critical.h
> @@ -0,0 +1,55 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM critical
> +
> +#if !defined(_TRACE_CRITICAL_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_CRITICAL_H
> +
> +#include 
> +#include 
> +#include 
> +
> +TRACE_EVENT(critical_start,
> +
> +   TP_PROTO(unsigned long ip, unsigned long parent_ip),
> +
> +   TP_ARGS(ip, parent_ip),
> +
> +   TP_STRUCT__entry(
> +   __array(char, func, 16)
> +   __array(char, parent, 16)
> +   ),
> +
> +   TP_fast_assign(
> +   snprintf(__entry->func, 16, "%pf", (void *)ip);
> +   snprintf(__entry->parent, 16, "%pf", (void *)parent_ip);
> +   ),
> +
> +   TP_printk("caller=%s parent=%s\n",
> + __entry->func,
> + __entry->parent)
> +);
> +
> +TRACE_EVENT(critical_end,
> +
> +   TP_PROTO(unsigned long ip, unsigned long parent_ip),
> +
> +   TP_ARGS(ip, parent_ip),
> +
> +   TP_STRUCT__entry(
> +   __array(char, func, 16)
> +   __array(char, parent, 16)
> +   ),
> +
> +   TP_fast_assign(
> +   snprintf(__entry->func, 16, "%pf", (void *)ip);
> +   snprintf(__entry->parent, 16, "%pf", (void *)parent_ip);
> +   ),
> +
> +   TP_printk("caller=%s parent=%s\n",
> + __entry->func,
> + __entry->parent)
> +);
> +#endif /* _TRACE_CRITICAL_H */
> +
> +/* This part ust be outside protection */
> +#include 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index d5038005eb5d..986a07985360 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -208,6 +208,12 @@ config PREEMPT_TRACER
>   enabled. This option and the irqs-off timing option can be
>   used together or separately.)
>
> +config TRACE_CRITICAL_SECTION_EVENTS
> +   bool "Trace critical sections as trace events"
> +   default n
> +   select IRQSOFF_TRACER
> +   select PREEMPT_TRACER
> +
>  config SCHED_TRACER
> bool "Scheduling Latency Tracer"
> select GENERIC_TRACER
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index 7758bc0617cb..a14ce6185885 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -13,6 +13,8 @@
>  #include 
>  #include 
>  #include 
> +#define CREATE_TRACE_POINTS
> +#include 
>
>  #include "trace.h"
>
> @@ -20,6 +22,7 @@ static struct trace_array *irqsoff_trace 
> __read_mostly;
>  static int tracer_enabled __read_mostly;
>
>  static DEFINE_PER_CPU(int, tracing_cpu);
> +static DEFINE_PER_CPU(int, tracing_events_cpu);
>
>  static DEFINE_RAW_SPINLOCK(max_trace_lock);
>
> @@ -362,6 +365,44 @@ check_critical_timing(struct trace_array *tr,
> __trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>  }
>
> +#ifdef CONFIG_TRACE_CRITICAL_SECTION_EVENTS
> +/*
> + * Called when either preempt or irq are turned off
> + */
> +static inline void
> +start_critical_event(unsigned long ip, unsigned long parent_ip)
> +{
> +   int cpu;
> +
> +   cpu = raw_smp_processor_id();
> +
> +   if (per_cpu(tracing_events_cpu, cpu))
> +   return;
> +
> +   trace_critical_start(ip, parent_ip);
> +
> +   per_cpu(tracing_events_cpu, cpu) = 1;
> +}
> +
> +/*
> + * 

Re: [RFC] tracing: Add support for critical section event tracing

[Joel Fernandes]

Hi Steven,

Did you have any comments about this patch? It was sent a while ago
and if you can provide me your initial thoughts on it, I would
appreciate it. (Sorry to ping you about it during the busy merge
window time, but I was thinking if I can get any initial early
comments from you then I can get to work on it and finish in time well
before the next merge window).

Regards,
Joel

On Tue, Apr 11, 2017 at 10:38 PM, Joel Fernandes  wrote:
> Critical section trace events can be used for tracing the start and
> end of a critical section which can be used by a trace viewer like
> systrace to graphically view the start and end of a critical section and
> correlate them with latencies and scheduling issues.
>
> Reason for starting critical section:
> Either preemption or interrupts were turned off.
> Reason for ending critical section:
> Both preemption and interrupts are now turned back on.
>
> Cc: Steven Rostedt 
> Signed-off-by: Joel Fernandes 
> --
> Here's an example of how Android's systrace will be using it to show critical
> sections as a gantt chart: http://imgur.com/download/TZplEVp
> ---
>  include/trace/events/critical.h | 55 +++
>  kernel/trace/Kconfig|  6 
>  kernel/trace/trace_irqsoff.c| 73 
> +
>  3 files changed, 134 insertions(+)
>  create mode 100644 include/trace/events/critical.h
>
> diff --git a/include/trace/events/critical.h b/include/trace/events/critical.h
> new file mode 100644
> index ..bfd58dd4f48f
> --- /dev/null
> +++ b/include/trace/events/critical.h
> @@ -0,0 +1,55 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM critical
> +
> +#if !defined(_TRACE_CRITICAL_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_CRITICAL_H
> +
> +#include 
> +#include 
> +#include 
> +
> +TRACE_EVENT(critical_start,
> +
> +   TP_PROTO(unsigned long ip, unsigned long parent_ip),
> +
> +   TP_ARGS(ip, parent_ip),
> +
> +   TP_STRUCT__entry(
> +   __array(char, func, 16)
> +   __array(char, parent, 16)
> +   ),
> +
> +   TP_fast_assign(
> +   snprintf(__entry->func, 16, "%pf", (void *)ip);
> +   snprintf(__entry->parent, 16, "%pf", (void *)parent_ip);
> +   ),
> +
> +   TP_printk("caller=%s parent=%s\n",
> + __entry->func,
> + __entry->parent)
> +);
> +
> +TRACE_EVENT(critical_end,
> +
> +   TP_PROTO(unsigned long ip, unsigned long parent_ip),
> +
> +   TP_ARGS(ip, parent_ip),
> +
> +   TP_STRUCT__entry(
> +   __array(char, func, 16)
> +   __array(char, parent, 16)
> +   ),
> +
> +   TP_fast_assign(
> +   snprintf(__entry->func, 16, "%pf", (void *)ip);
> +   snprintf(__entry->parent, 16, "%pf", (void *)parent_ip);
> +   ),
> +
> +   TP_printk("caller=%s parent=%s\n",
> + __entry->func,
> + __entry->parent)
> +);
> +#endif /* _TRACE_CRITICAL_H */
> +
> +/* This part ust be outside protection */
> +#include 
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index d5038005eb5d..986a07985360 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -208,6 +208,12 @@ config PREEMPT_TRACER
>   enabled. This option and the irqs-off timing option can be
>   used together or separately.)
>
> +config TRACE_CRITICAL_SECTION_EVENTS
> +   bool "Trace critical sections as trace events"
> +   default n
> +   select IRQSOFF_TRACER
> +   select PREEMPT_TRACER
> +
>  config SCHED_TRACER
> bool "Scheduling Latency Tracer"
> select GENERIC_TRACER
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index 7758bc0617cb..a14ce6185885 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -13,6 +13,8 @@
>  #include 
>  #include 
>  #include 
> +#define CREATE_TRACE_POINTS
> +#include 
>
>  #include "trace.h"
>
> @@ -20,6 +22,7 @@ static struct trace_array *irqsoff_trace 
> __read_mostly;
>  static int tracer_enabled __read_mostly;
>
>  static DEFINE_PER_CPU(int, tracing_cpu);
> +static DEFINE_PER_CPU(int, tracing_events_cpu);
>
>  static DEFINE_RAW_SPINLOCK(max_trace_lock);
>
> @@ -362,6 +365,44 @@ check_critical_timing(struct trace_array *tr,
> __trace_function(tr, CALLER_ADDR0, parent_ip, flags, pc);
>  }
>
> +#ifdef CONFIG_TRACE_CRITICAL_SECTION_EVENTS
> +/*
> + * Called when either preempt or irq are turned off
> + */
> +static inline void
> +start_critical_event(unsigned long ip, unsigned long parent_ip)
> +{
> +   int cpu;
> +
> +   cpu = raw_smp_processor_id();
> +
> +   if (per_cpu(tracing_events_cpu, cpu))
> +   return;
> +
> +   trace_critical_start(ip, parent_ip);
> +
> +   per_cpu(tracing_events_cpu, cpu) = 1;
> +}
> +
> +/*
> + * Called when both preempt and irq are turned back on
> + */
> 

Re: [GIT PULL rcu/next] RCU commits for 4.13

[Boqun Feng]

On Thu, Jun 29, 2017 at 09:02:41PM -0700, Paul E. McKenney wrote:
[...]
> > > o net/netfilter/nf_conntrack_core.c nf_conntrack_lock()
> > >   This instance of spin_unlock_wait() interacts with
> > >   nf_conntrack_all_lock()'s instance of spin_unlock_wait().
> > >   Although nf_conntrack_all_lock() has an smp_mb(), which I
> > >   believe provides release semantics given current implementations,
> > >   nf_conntrack_lock() just has smp_rmb().
> > > 
> > >   I believe that the smp_rmb() needs to be smp_mb().  Am I missing
> > >   something here that makes the current code safe on x86?
> > > 
> > 
> > actually i think the smp_rmb() or even along with the spin_unlock_wait()
> > in nf_conntrack_lock() is not needed, we could
> > implementnf_conntrack_lock() as:
> > 
> > 
> > void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
> > {
> > spin_lock(lock);
> > while (unlikely(smp_load_acquire(nf_conntrack_locks_all))) {
> > spin_unlock(lock);
> > cpu_relaxed();
> > spin_lock(lock);
> > }
> > }
> > 
> > because in nf_conntrack_all_unlock(), we have:
> > 
> > smp_store_release(_conntrack_locks_all, false);
> > spin_unlock(_conntrack_locks_all_lock);
> > 
> > so if we exit the loop, which means we observe nf_conntrack_locks_all
> > being false, we actually hold the per bucket lock and observe everything
> > before the smp_store_release(), which is the same as everything in the
> > critical section of nf_conntrack_locks_all_lock. Otherwise, we observe
> > the nf_conntrack_locks_all being true, which means a global lock
> > critical section may be on its way, we simply drop the per bucket lock
> > and test whether the global lock is finished again some time later.
> > 
> > So I think spin_unlock_wait() in the nf_conntrack_lock() just requires
> > acquire semantics, at least.
> > 
> > Maybe I miss someting?
> 
> Or perhaps I was being too paranoid.
> 
> But does the same analysis work in the case where an nf_conntrack_lock
> races with an nf_contrack_all_lock()?
> 

You mean the smp_mb()+spin_unlock_wait() in nf_conntrack_all_lock(),
right? I think it's different, because nf_conntrack_all_lock() relies
this release-like operation to let all the next critical sections of per
bucket locks observe nf_conntrack_locks_all=true, otherwise
nf_conntrack_lock() will break out the loop and access some data while
the global lock crictial section is doing the same.

The variable @nf_conntrack_locks_all is used for synchronized between
two kinds of locks and is set by nf_conntrack_all_lock(), I think this
make things different.

> > >   I believe that this code could use spin_lock+spin_unlock without
> > >   significant performance penalties -- I do not believe that
> > >   nf_conntrack_locks_all_lock gets significant contention.
> > > 
> > > raw_spin_unlock_wait() (Courtesy of Andrea Parri with added commentary):
> > > 
> > > o kernel/exit.c do_exit()
> > >   Seems to rely on both acquire and release semantics. The
> > >   raw_spin_unlock_wait() primitive is preceded by a smp_mb().
> > >   But this is task exit doing spin_unlock_wait() on the task's
> > >   lock, so spin_lock+spin_unlock should work fine here.
> > > 
> > > o kernel/sched/core.c do_task_dead()
> > >   Seems to rely on the acquire semantics only. The
> > >   raw_spin_unlock_wait() primitive is preceded by an inexplicable
> > >   smp_mb().  Again, this is task exit doing spin_unlock_wait() on
> > >   the task's lock, so spin_lock+spin_unlock should work fine here.
> > > 
> > > o kernel/task_work.c task_work_run()
> > >   Seems to rely on the acquire semantics only.  This is to handle
> > 
> > I think this one needs the stronger semantics, the smp_mb() is just
> > hidden in the cmpxchg() before the raw_spin_unlock_wait() ;-)
> > 
> > cmpxchg() sets a special value to indicate the task_work has been taken,
> > and raw_spin_unlock_wait() must wait until the next critical section of
> > ->pi_lock(in task_work_cancel()) could observe this, otherwise we may
> > cancel a task_work while executing it.
> 
> But either way, replacing the spin_unlock_wait() with a spin_lock()
> immediately followed by a spin_unlock() should work correctly, right?
> 

Yep ;-) I was thinking about the case that we kept spin_unlock_wait()
with a simpler acquire semantics, and if so, we would actually have to
do the replace. But I saw your patchset of removing it, so it doesn't
matter.

Regards,
Boqun

>   Thanx, Paul
> 
> > Regards,
> > Boqun
> > >   a race with task_work_cancel(), which appears to be quite rare.
> > >   So the spin_lock+spin_unlock should work fine here.
> > > 
> > > spin_lock()/spin_unlock():
> > > 
> > > o ipc/sem.c complexmode_enter()
> > >   This used to be spin_unlock_wait(), but was changed to a
> > >   spin_lock()/spin_unlock() pair by 27d7be1801a4 ("ipc/sem.c:
> > >   avoid using 

Re: [GIT PULL rcu/next] RCU commits for 4.13

[Boqun Feng]

On Thu, Jun 29, 2017 at 09:02:41PM -0700, Paul E. McKenney wrote:
[...]
> > > o net/netfilter/nf_conntrack_core.c nf_conntrack_lock()
> > >   This instance of spin_unlock_wait() interacts with
> > >   nf_conntrack_all_lock()'s instance of spin_unlock_wait().
> > >   Although nf_conntrack_all_lock() has an smp_mb(), which I
> > >   believe provides release semantics given current implementations,
> > >   nf_conntrack_lock() just has smp_rmb().
> > > 
> > >   I believe that the smp_rmb() needs to be smp_mb().  Am I missing
> > >   something here that makes the current code safe on x86?
> > > 
> > 
> > actually i think the smp_rmb() or even along with the spin_unlock_wait()
> > in nf_conntrack_lock() is not needed, we could
> > implementnf_conntrack_lock() as:
> > 
> > 
> > void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
> > {
> > spin_lock(lock);
> > while (unlikely(smp_load_acquire(nf_conntrack_locks_all))) {
> > spin_unlock(lock);
> > cpu_relaxed();
> > spin_lock(lock);
> > }
> > }
> > 
> > because in nf_conntrack_all_unlock(), we have:
> > 
> > smp_store_release(_conntrack_locks_all, false);
> > spin_unlock(_conntrack_locks_all_lock);
> > 
> > so if we exit the loop, which means we observe nf_conntrack_locks_all
> > being false, we actually hold the per bucket lock and observe everything
> > before the smp_store_release(), which is the same as everything in the
> > critical section of nf_conntrack_locks_all_lock. Otherwise, we observe
> > the nf_conntrack_locks_all being true, which means a global lock
> > critical section may be on its way, we simply drop the per bucket lock
> > and test whether the global lock is finished again some time later.
> > 
> > So I think spin_unlock_wait() in the nf_conntrack_lock() just requires
> > acquire semantics, at least.
> > 
> > Maybe I miss someting?
> 
> Or perhaps I was being too paranoid.
> 
> But does the same analysis work in the case where an nf_conntrack_lock
> races with an nf_contrack_all_lock()?
> 

You mean the smp_mb()+spin_unlock_wait() in nf_conntrack_all_lock(),
right? I think it's different, because nf_conntrack_all_lock() relies
this release-like operation to let all the next critical sections of per
bucket locks observe nf_conntrack_locks_all=true, otherwise
nf_conntrack_lock() will break out the loop and access some data while
the global lock crictial section is doing the same.

The variable @nf_conntrack_locks_all is used for synchronized between
two kinds of locks and is set by nf_conntrack_all_lock(), I think this
make things different.

> > >   I believe that this code could use spin_lock+spin_unlock without
> > >   significant performance penalties -- I do not believe that
> > >   nf_conntrack_locks_all_lock gets significant contention.
> > > 
> > > raw_spin_unlock_wait() (Courtesy of Andrea Parri with added commentary):
> > > 
> > > o kernel/exit.c do_exit()
> > >   Seems to rely on both acquire and release semantics. The
> > >   raw_spin_unlock_wait() primitive is preceded by a smp_mb().
> > >   But this is task exit doing spin_unlock_wait() on the task's
> > >   lock, so spin_lock+spin_unlock should work fine here.
> > > 
> > > o kernel/sched/core.c do_task_dead()
> > >   Seems to rely on the acquire semantics only. The
> > >   raw_spin_unlock_wait() primitive is preceded by an inexplicable
> > >   smp_mb().  Again, this is task exit doing spin_unlock_wait() on
> > >   the task's lock, so spin_lock+spin_unlock should work fine here.
> > > 
> > > o kernel/task_work.c task_work_run()
> > >   Seems to rely on the acquire semantics only.  This is to handle
> > 
> > I think this one needs the stronger semantics, the smp_mb() is just
> > hidden in the cmpxchg() before the raw_spin_unlock_wait() ;-)
> > 
> > cmpxchg() sets a special value to indicate the task_work has been taken,
> > and raw_spin_unlock_wait() must wait until the next critical section of
> > ->pi_lock(in task_work_cancel()) could observe this, otherwise we may
> > cancel a task_work while executing it.
> 
> But either way, replacing the spin_unlock_wait() with a spin_lock()
> immediately followed by a spin_unlock() should work correctly, right?
> 

Yep ;-) I was thinking about the case that we kept spin_unlock_wait()
with a simpler acquire semantics, and if so, we would actually have to
do the replace. But I saw your patchset of removing it, so it doesn't
matter.

Regards,
Boqun

>   Thanx, Paul
> 
> > Regards,
> > Boqun
> > >   a race with task_work_cancel(), which appears to be quite rare.
> > >   So the spin_lock+spin_unlock should work fine here.
> > > 
> > > spin_lock()/spin_unlock():
> > > 
> > > o ipc/sem.c complexmode_enter()
> > >   This used to be spin_unlock_wait(), but was changed to a
> > >   spin_lock()/spin_unlock() pair by 27d7be1801a4 ("ipc/sem.c:
> > >   avoid using 

Re: [RFC 0/5] drivers: Add boot constraints core

[Viresh Kumar]

On 30-06-17, 12:22, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar  
> wrote:
> > On 30-06-17, 12:05, Chen-Yu Tsai wrote:

> >> I also want to mention that for DT based platforms, this constraint
> >> should already be set in the device tree for the regulator, so the
> >> scenario where DMA comes up and sets a voltage level that LCD cannot
> >> use should not even be possible.
> 
> What I'm saying is for the DT case, the constraints are already limited
> to the intersection of all users, regardless of whether they are turned
> on or not.

Right, but someone needs to get the regulator first to have that
considered by the regulator core while deciding the final range.

Both DMA and LCD driver do regulator_get() for their devices but if
only DMA driver is probed until now, then the regulator core wouldn't
consider LCD as regulator_get() is never called for LCD.

> I think what you mean is that the DT constraints are the union of all
> consumer constraints (1.8 - 3.0 V in this case), then each consumer
> comes in and adds its own constraints. And for such a design, the kernel
> needs to know which and what constraints to apply.

Sorry, I am confused with what you just said and not sure if I
understand it completely.

Each consumer DT node will have its own set of constraints for the
regulator device. The kernel will do regulator_get() for them one by
one, based on when their drivers get probed. And an intersection of
those constraints (which already did regulator_get()) will be used by
the regulator core.

Now this series is saying that even if the driver didn't come up (for
LCD) and haven't done its regulator_get() yet, consider that device's
constraint while calculating the target voltage for the regulator.

> Either way regulators already support constraints, so they are easier
> to deal with. Clocks on the other hand, while the core does support
> clock rate constraints, AFAIK no one really uses or supports them.

Yeah, so I started with just regulators and that's when Mark suggested
to do something generic which can be reused by other resource types.
We may end up covering clk for sure I believe. Not sure yet about
other resource types though.

-- 
viresh

Re: [RFC 0/5] drivers: Add boot constraints core

[Viresh Kumar]

On 30-06-17, 12:22, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar  
> wrote:
> > On 30-06-17, 12:05, Chen-Yu Tsai wrote:

> >> I also want to mention that for DT based platforms, this constraint
> >> should already be set in the device tree for the regulator, so the
> >> scenario where DMA comes up and sets a voltage level that LCD cannot
> >> use should not even be possible.
> 
> What I'm saying is for the DT case, the constraints are already limited
> to the intersection of all users, regardless of whether they are turned
> on or not.

Right, but someone needs to get the regulator first to have that
considered by the regulator core while deciding the final range.

Both DMA and LCD driver do regulator_get() for their devices but if
only DMA driver is probed until now, then the regulator core wouldn't
consider LCD as regulator_get() is never called for LCD.

> I think what you mean is that the DT constraints are the union of all
> consumer constraints (1.8 - 3.0 V in this case), then each consumer
> comes in and adds its own constraints. And for such a design, the kernel
> needs to know which and what constraints to apply.

Sorry, I am confused with what you just said and not sure if I
understand it completely.

Each consumer DT node will have its own set of constraints for the
regulator device. The kernel will do regulator_get() for them one by
one, based on when their drivers get probed. And an intersection of
those constraints (which already did regulator_get()) will be used by
the regulator core.

Now this series is saying that even if the driver didn't come up (for
LCD) and haven't done its regulator_get() yet, consider that device's
constraint while calculating the target voltage for the regulator.

> Either way regulators already support constraints, so they are easier
> to deal with. Clocks on the other hand, while the core does support
> clock rate constraints, AFAIK no one really uses or supports them.

Yeah, so I started with just regulators and that's when Mark suggested
to do something generic which can be reused by other resource types.
We may end up covering clk for sure I believe. Not sure yet about
other resource types though.

-- 
viresh

Re: linux-next: build warning after merge of the drivers-x86 tree

[Arvind Yadav]

Hi,
 I have push one more patch which does not have this warning.
please avoid my first patch.

Thanks
~ arvind

On Friday 30 June 2017 09:30 AM, Stephen Rothwell wrote:

Hi Darren,

After merging the drivers-x86 tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/platform/x86/sony-laptop.c: In function 'sony_laptop_setup_input':
drivers/platform/x86/sony-laptop.c:494:19: warning: assignment discards 'const' 
qualifier from pointer target type [-Wdiscarded-array-qualifiers]
   key_dev->keycode = _laptop_input_keycode_map;
^

Introduced by commit

   a0f0a5e1978b ("platform/x86: sony-laptop: constify few static structures")

Re: linux-next: build warning after merge of the drivers-x86 tree

[Arvind Yadav]

Hi,
 I have push one more patch which does not have this warning.
please avoid my first patch.

Thanks
~ arvind

On Friday 30 June 2017 09:30 AM, Stephen Rothwell wrote:

Hi Darren,

After merging the drivers-x86 tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/platform/x86/sony-laptop.c: In function 'sony_laptop_setup_input':
drivers/platform/x86/sony-laptop.c:494:19: warning: assignment discards 'const' 
qualifier from pointer target type [-Wdiscarded-array-qualifiers]
   key_dev->keycode = _laptop_input_keycode_map;
^

Introduced by commit

   a0f0a5e1978b ("platform/x86: sony-laptop: constify few static structures")

Re: linux-next: manual merge of the target-updates tree with the scsi-mkp tree

[Stephen Rothwell]

Hi all,

With the merge window approaching, this is just a reminder that the
following conflict still exists.  No action is necessarily needed
except to maybe mention this to Linus at the appropriate time.

James, you have been added since you merged the scsi-mkp tree int the
scsi tree.

On Tue, 13 Jun 2017 16:47:42 +1000 Stephen Rothwell  
wrote:
>
> Hi Nicholas,
> 
> Today's linux-next merge of the target-updates tree got a conflict in:
> 
>   drivers/scsi/qla2xxx/qla_target.c
> 
> between commit:
> 
>   f775bd14e44d ("scsi: qla2xxx: Convert 32-bit LUN usage to 64-bit")
> 
> from the scsi-mkp tree and commit:
> 
>   6b26726af699 ("qla2xxx: Convert QLA_TGT_ABTS to 
> TARGET_SCF_LOOKUP_LUN_FROM_TAG")
> 
> from the target-updates tree.
> 
> I fixed it up (I basically used the latter verion of
> __qlt_24xx_handle_abts()) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: manual merge of the target-updates tree with the scsi-mkp tree

[Stephen Rothwell]

Hi all,

With the merge window approaching, this is just a reminder that the
following conflict still exists.  No action is necessarily needed
except to maybe mention this to Linus at the appropriate time.

James, you have been added since you merged the scsi-mkp tree int the
scsi tree.

On Tue, 13 Jun 2017 16:47:42 +1000 Stephen Rothwell  
wrote:
>
> Hi Nicholas,
> 
> Today's linux-next merge of the target-updates tree got a conflict in:
> 
>   drivers/scsi/qla2xxx/qla_target.c
> 
> between commit:
> 
>   f775bd14e44d ("scsi: qla2xxx: Convert 32-bit LUN usage to 64-bit")
> 
> from the scsi-mkp tree and commit:
> 
>   6b26726af699 ("qla2xxx: Convert QLA_TGT_ABTS to 
> TARGET_SCF_LOOKUP_LUN_FROM_TAG")
> 
> from the target-updates tree.
> 
> I fixed it up (I basically used the latter verion of
> __qlt_24xx_handle_abts()) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

-- 
Cheers,
Stephen Rothwell

[PATCH v2] sony-laptop: constify spic_attribute_group and sony_laptop_input_index.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
  312735176 372   368218fd5 drivers/platform/x86/sony-laptop.o

File size After adding 'const':
   textdata bss dec hex filename
  313375112 372   368218fd5 drivers/platform/x86/sony-laptop.o

Signed-off-by: Arvind Yadav 
---
Changes in v2:
  Resolve warning: assignment discards 'const' qualifier from 
pointer target type [-Wdiscarded-array-qualifiers]
  key_dev->keycode = _laptop_input_keycode_map;

 drivers/platform/x86/sony-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c 
b/drivers/platform/x86/sony-laptop.c
index aa2ee51..bfae795 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -222,7 +222,7 @@ struct sony_laptop_keypress {
 /* Correspondance table between sonypi events
  * and input layer indexes in the keymap
  */
-static int sony_laptop_input_index[] = {
+static const int sony_laptop_input_index[] = {
-1, /*  0 no event */
-1, /*  1 SONYPI_EVENT_JOGDIAL_DOWN */
-1, /*  2 SONYPI_EVENT_JOGDIAL_UP */
@@ -4032,7 +4032,7 @@ struct device_attribute spic_attr_##_name = __ATTR(_name, 
\
NULL
 };
 
-static struct attribute_group spic_attribute_group = {
+static const struct attribute_group spic_attribute_group = {
.attrs = spic_attributes
 };
 
-- 
1.9.1

[PATCH v2] sony-laptop: constify spic_attribute_group and sony_laptop_input_index.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
  312735176 372   368218fd5 drivers/platform/x86/sony-laptop.o

File size After adding 'const':
   textdata bss dec hex filename
  313375112 372   368218fd5 drivers/platform/x86/sony-laptop.o

Signed-off-by: Arvind Yadav 
---
Changes in v2:
  Resolve warning: assignment discards 'const' qualifier from 
pointer target type [-Wdiscarded-array-qualifiers]
  key_dev->keycode = _laptop_input_keycode_map;

 drivers/platform/x86/sony-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c 
b/drivers/platform/x86/sony-laptop.c
index aa2ee51..bfae795 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -222,7 +222,7 @@ struct sony_laptop_keypress {
 /* Correspondance table between sonypi events
  * and input layer indexes in the keymap
  */
-static int sony_laptop_input_index[] = {
+static const int sony_laptop_input_index[] = {
-1, /*  0 no event */
-1, /*  1 SONYPI_EVENT_JOGDIAL_DOWN */
-1, /*  2 SONYPI_EVENT_JOGDIAL_UP */
@@ -4032,7 +4032,7 @@ struct device_attribute spic_attr_##_name = __ATTR(_name, 
\
NULL
 };
 
-static struct attribute_group spic_attribute_group = {
+static const struct attribute_group spic_attribute_group = {
.attrs = spic_attributes
 };
 
-- 
1.9.1

Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

[Andy Lutomirski]

On Thu, Jun 29, 2017 at 7:12 PM, Josh Poimboeuf  wrote:
> On Thu, Jun 29, 2017 at 03:59:04PM -0700, Andy Lutomirski wrote:
>> >
>> > Sorry, I didn't explain it very well.  Undwarf can find the regs pointer
>> > in rdi, it just doesn't trust its value.
>> >
>> > See the stack_info.next_sp field, which is set in in_irq_stack():
>> >
>> >/*
>> > * The next stack pointer is the first thing pushed by the entry code
>> > * after switching to the irq stack.
>> > */
>> >info->next_sp = (unsigned long *)*(end - 1);
>> >
>> > It's a safety mechanism.  The unwinder needs the last word of the irq
>> > stack page to point to the previous stack.  That way it can double check
>> > that the stack pointer it calculates is within the bounds of either the
>> > current stack or the previous stack.
>> >
>> > In the above code, the previous stack pointer (or next stack pointer,
>> > depending on your perspective) hasn't been set up before it switches
>> > stacks.  So the unwinder reads an uninitialized value into
>> > info->next_sp, and compares that with the regs pointer, and then stops
>> > the unwind because it thinks it went off into the weeds.
>> >
>>
>> That should be manageable, though, I think.  With my patch applied
>> (and maybe even without it), the only exception to that rule is if
>> regs->sp points just above the top of the IRQ stack and the next
>> instruction is push reg.  In that case, the reg is exactly as
>> trustworthy as the normal rule.*  Can you teach the unwinding code
>> that this is okay?
>>
>> * If an NMI hits right there, then it relies on unwinding out of the
>> NMI correctly.  But the usual checks that the target stack is a valid
>> stack should prevent us from going off into the weeds regardless.
>
> But that would remove a safeguard against the undwarf data being
> corrupt.  Sure, it would only affect the rare case where the stack
> pointer is at the top of the IRQ stack, but still...
>
> Also, the frame pointer and guess unwinders have the same issue, and
> this solution wouldn't work for them.
>
> And, worst of all, the oops stack dumping code in show_trace_log_lvl()
> also has this issue .  It relies on those previous stack pointers.  And
> it's separated from the unwinder logic by design, so it can't ask the
> unwinder where the next stack is.

Ugh.

I feel like we had this debate before, and I thought it was rather
silly that the unwinder cared.  After all, we already have separate
safety mechanisms to make sure that the unwinder never wanders off of
the valid stacks and that it never touches any given stack more than
once.  But it is indeed useful for the oops unwinder, so c'est la vie.

That being said, I bet we could get away with this (sorry for immense
whitespace damage):

.macro ENTER_IRQ_STACK old_rsp scratch_reg
  DEBUG_ENTRY_ASSERT_IRQS_OFF
  movq %rsp, \old_rsp
  incl PER_CPU_VAR(irq_count)
  jnz .Lrecurse_irq_stack_\@

   /*
   * Right now, we just incremented irq_count to zero, so we've
   * claimed the IRQ stack but we haven't switched to it yet.
   * Anything that can interrupt us here without using IST
   * must be *extremely* careful to limit its stack usage.
   *
   * We write old_rsp to the IRQ stack before switching to
   * %rsp for the benefit of the OOPS unwinder.
   */
   movq PER_CPU_VAR(irq_stack_ptr), \scratch_reg
   movq \old_rsp, -8(\scratch_reg)
   leaq -8(\old_rsp), %rsp
   jmp .Lout_\@

   .Lrecurse_irq_stack_\@:
   pushq \old_rsp

   .Lout_\@:
.endm

After all, it looks like all the users have a scratch reg available.

Hmm.  There's another option that might be considerably nicer, though:
put the IRQ stack at a known (at link time) position *in percpu
space*.  (Presumably it already is -- I haven't checked.)  Then we do:

.macro ENTER_IRQ_STACK old_rsp
DEBUG_ENTRY_ASSERT_IRQS_OFF
movq%rsp, \old_rsp
inclPER_CPU_VAR(irq_count)

/*
 * Right now, if we just incremented irq_count to zero, we've
 * claimed the IRQ stack but we haven't switched to it yet.
 * Anything that can interrupt us here without using IST
 * must be *extremely* careful to limit its stack usage.
 */
jnz .Lpush_old_rsp_\@
movq\old_rsp, PER_CPU_VAR(top_word_in_irq_stack)
movqPER_CPU_VAR(irq_stack_ptr), %rsp
.Lpush_old_rsp_\@:
pushq\old_rsp
.endm

This pushes the old pointer twice, but that's easy enough to fix if we
really cared.  I think I like this variant better.  What do you think?

--Andy

Re: [PATCH v2 6/8] x86/entry: add unwind hint annotations

[Andy Lutomirski]

On Thu, Jun 29, 2017 at 7:12 PM, Josh Poimboeuf  wrote:
> On Thu, Jun 29, 2017 at 03:59:04PM -0700, Andy Lutomirski wrote:
>> >
>> > Sorry, I didn't explain it very well.  Undwarf can find the regs pointer
>> > in rdi, it just doesn't trust its value.
>> >
>> > See the stack_info.next_sp field, which is set in in_irq_stack():
>> >
>> >/*
>> > * The next stack pointer is the first thing pushed by the entry code
>> > * after switching to the irq stack.
>> > */
>> >info->next_sp = (unsigned long *)*(end - 1);
>> >
>> > It's a safety mechanism.  The unwinder needs the last word of the irq
>> > stack page to point to the previous stack.  That way it can double check
>> > that the stack pointer it calculates is within the bounds of either the
>> > current stack or the previous stack.
>> >
>> > In the above code, the previous stack pointer (or next stack pointer,
>> > depending on your perspective) hasn't been set up before it switches
>> > stacks.  So the unwinder reads an uninitialized value into
>> > info->next_sp, and compares that with the regs pointer, and then stops
>> > the unwind because it thinks it went off into the weeds.
>> >
>>
>> That should be manageable, though, I think.  With my patch applied
>> (and maybe even without it), the only exception to that rule is if
>> regs->sp points just above the top of the IRQ stack and the next
>> instruction is push reg.  In that case, the reg is exactly as
>> trustworthy as the normal rule.*  Can you teach the unwinding code
>> that this is okay?
>>
>> * If an NMI hits right there, then it relies on unwinding out of the
>> NMI correctly.  But the usual checks that the target stack is a valid
>> stack should prevent us from going off into the weeds regardless.
>
> But that would remove a safeguard against the undwarf data being
> corrupt.  Sure, it would only affect the rare case where the stack
> pointer is at the top of the IRQ stack, but still...
>
> Also, the frame pointer and guess unwinders have the same issue, and
> this solution wouldn't work for them.
>
> And, worst of all, the oops stack dumping code in show_trace_log_lvl()
> also has this issue .  It relies on those previous stack pointers.  And
> it's separated from the unwinder logic by design, so it can't ask the
> unwinder where the next stack is.

Ugh.

I feel like we had this debate before, and I thought it was rather
silly that the unwinder cared.  After all, we already have separate
safety mechanisms to make sure that the unwinder never wanders off of
the valid stacks and that it never touches any given stack more than
once.  But it is indeed useful for the oops unwinder, so c'est la vie.

That being said, I bet we could get away with this (sorry for immense
whitespace damage):

.macro ENTER_IRQ_STACK old_rsp scratch_reg
  DEBUG_ENTRY_ASSERT_IRQS_OFF
  movq %rsp, \old_rsp
  incl PER_CPU_VAR(irq_count)
  jnz .Lrecurse_irq_stack_\@

   /*
   * Right now, we just incremented irq_count to zero, so we've
   * claimed the IRQ stack but we haven't switched to it yet.
   * Anything that can interrupt us here without using IST
   * must be *extremely* careful to limit its stack usage.
   *
   * We write old_rsp to the IRQ stack before switching to
   * %rsp for the benefit of the OOPS unwinder.
   */
   movq PER_CPU_VAR(irq_stack_ptr), \scratch_reg
   movq \old_rsp, -8(\scratch_reg)
   leaq -8(\old_rsp), %rsp
   jmp .Lout_\@

   .Lrecurse_irq_stack_\@:
   pushq \old_rsp

   .Lout_\@:
.endm

After all, it looks like all the users have a scratch reg available.

Hmm.  There's another option that might be considerably nicer, though:
put the IRQ stack at a known (at link time) position *in percpu
space*.  (Presumably it already is -- I haven't checked.)  Then we do:

.macro ENTER_IRQ_STACK old_rsp
DEBUG_ENTRY_ASSERT_IRQS_OFF
movq%rsp, \old_rsp
inclPER_CPU_VAR(irq_count)

/*
 * Right now, if we just incremented irq_count to zero, we've
 * claimed the IRQ stack but we haven't switched to it yet.
 * Anything that can interrupt us here without using IST
 * must be *extremely* careful to limit its stack usage.
 */
jnz .Lpush_old_rsp_\@
movq\old_rsp, PER_CPU_VAR(top_word_in_irq_stack)
movqPER_CPU_VAR(irq_stack_ptr), %rsp
.Lpush_old_rsp_\@:
pushq\old_rsp
.endm

This pushes the old pointer twice, but that's easy enough to fix if we
really cared.  I think I like this variant better.  What do you think?

--Andy

Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Zhang Rui]

On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
> Under each thermal zone there is a optional file called "mode".
> Writing
> enabled or disabled to this file allows a given thermal zone to be
> enabled
> or disabled, but in current code, the monitoring queue doesn't stops.
> Add
> the code to disable polling when disabling thermal zone and enable
> polling
> when enabling the thermal zone.
> 
> This patch is based on the original Sameer Nanda  >
> patch that implemented this idea for the ACPI thermal driver.
> 

Before these two patches, only platform thermal driver cares about
"mode", thermal core does nothing but invokes platform .set_mode()
callback upon sysfs I/F write.
But after this patch set, "mode" becomes something that we should
take into account in thermal core as well.
Thus, IMO, we have a couple of things more to do, like the prototype
patch attached, which I have not tested yet.

>From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00 2001
From: Zhang Rui 
Date: Fri, 30 Jun 2017 11:11:45 +0800
Subject: [RFC PATCH] Thermal: introduce thermal zone device mode control

Thermal "mode" sysfs attribute is introduced to enable/disable a thermal zone,
and .get_mode()/.set_mode() callback is introduced for platform thermal driver
to enable/disable the hardware thermal control logic. And thermal core takes
no action upon thermal zone enable/disable.

Actually, this is not quite right because thermal core still pokes those
disabled thermal zones occasionally, e.g. upon system resume.

To fix this, a new flag 'mode' is introduced in struct thermal_zone_device
to represent the thermal zone mode, and several decisions have been made
based on this flag, including
1. check the thermal zone mode right after it's registered.
2. skip updating thermal zone if the zone is disabled
3. stop the polling timer when the thermal zone is disabled

Note: basically, this patch doesn't affect the existing always-enabled
thermal zones much, with just one exception -
thermal zone .get_mode() must be well prepared to reflect the real thermal
zone status upon the thermal zone registration.

Signed-off-by: Zhang Rui 
---
 drivers/thermal/thermal_core.c  | 37 +++--
 drivers/thermal/thermal_sysfs.c | 22 ++
 include/linux/thermal.h |  3 +++
 3 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5a51c74..89b2254 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct thermal_zone_device 
*tz)
 {
    mutex_lock(>lock);
 
-   if (tz->passive)
+   if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)
    thermal_zone_device_set_polling(tz, tz->passive_delay);
-   else if (tz->polling_delay)
+   else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->polling_delay)
    thermal_zone_device_set_polling(tz, tz->polling_delay);
    else
    thermal_zone_device_set_polling(tz, 0);
@@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct 
thermal_zone_device *tz)
    pos->initialized = false;
 }
 
+int thermal_zone_set_mode(struct thermal_zone_device *tz,
+    enum thermal_device_mode mode)
+{
+   int result;
+
+   if (!tz->ops->set_mode)
+   return -EPERM;
+
+   result = tz->ops->set_mode(tz, mode);
+   if (result)
+   return result;
+
+   if (tz->mode != mode) {
+   tz->mode = mode;
+   monitor_thermal_zone(tz);
+   }
+   return 0;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
+
 void thermal_zone_device_update(struct thermal_zone_device *tz,
    enum thermal_notify_event event)
 {
    int count;
 
+   /* Do nothing if the thermal zone is disabled */
+   if (tz->mode == THERMAL_DEVICE_DISABLED)
+   return;
+
    if (atomic_read(_suspend))
    return;
 
@@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char *type, int 
trips, int mask,
    INIT_DELAYED_WORK(>poll_queue, thermal_zone_device_check);
 
    thermal_zone_device_reset(tz);
+
+   if (tz->ops->get_mode()) {
+   enum thermal_device_mode mode;
+
+   result = tz->ops->get_mode(tz, );
+   tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
+   } else
+   tz->mode = THERMAL_DEVICE_ENABLED;
+
    /* Update the new thermal zone and mark it as already updated. */
    if (atomic_cmpxchg(>need_update, 1, 0))
    thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index a694de9..95d2587 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ 

Re: [PATCH v2 2/2] thermal: core: Allow to disable polling when disabling thermal zone.

[Zhang Rui]

On Thu, 2017-06-29 at 18:50 +0200, Enric Balletbo i Serra wrote:
> Under each thermal zone there is a optional file called "mode".
> Writing
> enabled or disabled to this file allows a given thermal zone to be
> enabled
> or disabled, but in current code, the monitoring queue doesn't stops.
> Add
> the code to disable polling when disabling thermal zone and enable
> polling
> when enabling the thermal zone.
> 
> This patch is based on the original Sameer Nanda  >
> patch that implemented this idea for the ACPI thermal driver.
> 

Before these two patches, only platform thermal driver cares about
"mode", thermal core does nothing but invokes platform .set_mode()
callback upon sysfs I/F write.
But after this patch set, "mode" becomes something that we should
take into account in thermal core as well.
Thus, IMO, we have a couple of things more to do, like the prototype
patch attached, which I have not tested yet.

>From 8bf51fe65bd386b7e8c3c2ca8b9f7d321c92b22f Mon Sep 17 00:00:00 2001
From: Zhang Rui 
Date: Fri, 30 Jun 2017 11:11:45 +0800
Subject: [RFC PATCH] Thermal: introduce thermal zone device mode control

Thermal "mode" sysfs attribute is introduced to enable/disable a thermal zone,
and .get_mode()/.set_mode() callback is introduced for platform thermal driver
to enable/disable the hardware thermal control logic. And thermal core takes
no action upon thermal zone enable/disable.

Actually, this is not quite right because thermal core still pokes those
disabled thermal zones occasionally, e.g. upon system resume.

To fix this, a new flag 'mode' is introduced in struct thermal_zone_device
to represent the thermal zone mode, and several decisions have been made
based on this flag, including
1. check the thermal zone mode right after it's registered.
2. skip updating thermal zone if the zone is disabled
3. stop the polling timer when the thermal zone is disabled

Note: basically, this patch doesn't affect the existing always-enabled
thermal zones much, with just one exception -
thermal zone .get_mode() must be well prepared to reflect the real thermal
zone status upon the thermal zone registration.

Signed-off-by: Zhang Rui 
---
 drivers/thermal/thermal_core.c  | 37 +++--
 drivers/thermal/thermal_sysfs.c | 22 ++
 include/linux/thermal.h |  3 +++
 3 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 5a51c74..89b2254 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -306,9 +306,9 @@ static void monitor_thermal_zone(struct thermal_zone_device 
*tz)
 {
    mutex_lock(>lock);
 
-   if (tz->passive)
+   if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->passive)
    thermal_zone_device_set_polling(tz, tz->passive_delay);
-   else if (tz->polling_delay)
+   else if (tz->enabled == THERMAL_DEVICE_ENABLED && tz->polling_delay)
    thermal_zone_device_set_polling(tz, tz->polling_delay);
    else
    thermal_zone_device_set_polling(tz, 0);
@@ -464,11 +464,35 @@ static void thermal_zone_device_reset(struct 
thermal_zone_device *tz)
    pos->initialized = false;
 }
 
+int thermal_zone_set_mode(struct thermal_zone_device *tz,
+    enum thermal_device_mode mode)
+{
+   int result;
+
+   if (!tz->ops->set_mode)
+   return -EPERM;
+
+   result = tz->ops->set_mode(tz, mode);
+   if (result)
+   return result;
+
+   if (tz->mode != mode) {
+   tz->mode = mode;
+   monitor_thermal_zone(tz);
+   }
+   return 0;
+}
+EXPORT_SYMBOL_GPL(thermal_zone_set_mode);
+
 void thermal_zone_device_update(struct thermal_zone_device *tz,
    enum thermal_notify_event event)
 {
    int count;
 
+   /* Do nothing if the thermal zone is disabled */
+   if (tz->mode == THERMAL_DEVICE_DISABLED)
+   return;
+
    if (atomic_read(_suspend))
    return;
 
@@ -1287,6 +1311,15 @@ thermal_zone_device_register(const char *type, int 
trips, int mask,
    INIT_DELAYED_WORK(>poll_queue, thermal_zone_device_check);
 
    thermal_zone_device_reset(tz);
+
+   if (tz->ops->get_mode()) {
+   enum thermal_device_mode mode;
+
+   result = tz->ops->get_mode(tz, );
+   tz->mode = result ? THERMAL_DEVICE_ENABLED : mode;
+   } else
+   tz->mode = THERMAL_DEVICE_ENABLED;
+
    /* Update the new thermal zone and mark it as already updated. */
    if (atomic_cmpxchg(>need_update, 1, 0))
    thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index a694de9..95d2587 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -51,17 +51,8 @@ static ssize_t
 

[PATCH] sata_highbank: fix error return code in ahci_highbank_probe()

[Gustavo A. R. Silva]

Propagate the return value of platform_get_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/sata_highbank.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index aafb8cc..2fc451c 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -483,9 +483,9 @@ static int ahci_highbank_probe(struct platform_device *pdev)
}
 
irq = platform_get_irq(pdev, 0);
-   if (irq <= 0) {
+   if (irq < 0) {
dev_err(dev, "no irq\n");
-   return -EINVAL;
+   return irq;
}
 
hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
-- 
2.5.0

[PATCH] sata_highbank: fix error return code in ahci_highbank_probe()

[Gustavo A. R. Silva]

Propagate the return value of platform_get_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/ata/sata_highbank.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index aafb8cc..2fc451c 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -483,9 +483,9 @@ static int ahci_highbank_probe(struct platform_device *pdev)
}
 
irq = platform_get_irq(pdev, 0);
-   if (irq <= 0) {
+   if (irq < 0) {
dev_err(dev, "no irq\n");
-   return -EINVAL;
+   return irq;
}
 
hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
-- 
2.5.0

[PATCH] staging: ks7010: fix styling WARNINGs

[Mark Rogers]

Trivial style changes. There are still 3 "line over 80 characters"
checkpatch.pl warnings, but I think they are best left alone as
breaking the first two warning lines could hurt readability. The third
warning is a message that should not be broken for the sake of grep.

All but one of the changes fix lines that exceed 80 characters. An
embedded function name was replaced by __func__ as well.

Signed-off-by: Mark Rogers 
---
 drivers/staging/ks7010/ks7010_sdio.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index c325f48..6c0c6b2 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -548,7 +548,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
if (atomic_read(>psstatus.status) == PS_SNOOZE) {
if (cnt_txqbody(priv)) {
ks_wlan_hw_wakeup_request(priv);
-   queue_delayed_work(priv->wq, 
>rw_dwork, 1);
+   queue_delayed_work(priv->wq,
+  >rw_dwork, 1);
return;
}
} else {
@@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card 
*card)
memcpy(rom_buf, fw_entry->data + n, size);
 
offset = n;
-   ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + 
offset);
+   ret = ks7010_sdio_update_index(priv,
+  KS7010_IRAM_ADDRESS + offset);
if (ret)
goto release_firmware;
 
-   ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
+   ret = ks7010_sdio_write(priv,
+   DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
 
-   ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, 
size);
+   ret = ks7010_sdio_data_compare(priv,
+  DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
 
@@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
priv = netdev_priv(netdev);
 
card->priv = priv;
-   SET_NETDEV_DEV(netdev, >func->dev);   /* for create sysfs 
symlinks */
+   SET_NETDEV_DEV(netdev, >func->dev);/* for create sysfs symlinks */
 
/* private memory initialize */
priv->ks_sdio_card = card;
@@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
}
 
/* interrupt setting */
-   /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024) 
*/
+   /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024)*/
sdio_claim_host(func);
ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
sdio_release_host(func);
@@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
struct ks_sdio_card *card;
struct ks_wlan_private *priv;
 
-   DPRINTK(1, "ks7010_sdio_remove()\n");
+   DPRINTK(1, "%s()\n", __func__);
 
card = sdio_get_drvdata(func);
 
-- 
2.7.4

[PATCH] staging: ks7010: fix styling WARNINGs

[Mark Rogers]

Trivial style changes. There are still 3 "line over 80 characters"
checkpatch.pl warnings, but I think they are best left alone as
breaking the first two warning lines could hurt readability. The third
warning is a message that should not be broken for the sake of grep.

All but one of the changes fix lines that exceed 80 characters. An
embedded function name was replaced by __func__ as well.

Signed-off-by: Mark Rogers 
---
 drivers/staging/ks7010/ks7010_sdio.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index c325f48..6c0c6b2 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -548,7 +548,8 @@ static void ks_sdio_interrupt(struct sdio_func *func)
if (atomic_read(>psstatus.status) == PS_SNOOZE) {
if (cnt_txqbody(priv)) {
ks_wlan_hw_wakeup_request(priv);
-   queue_delayed_work(priv->wq, 
>rw_dwork, 1);
+   queue_delayed_work(priv->wq,
+  >rw_dwork, 1);
return;
}
} else {
@@ -693,15 +694,18 @@ static int ks7010_upload_firmware(struct ks_sdio_card 
*card)
memcpy(rom_buf, fw_entry->data + n, size);
 
offset = n;
-   ret = ks7010_sdio_update_index(priv, KS7010_IRAM_ADDRESS + 
offset);
+   ret = ks7010_sdio_update_index(priv,
+  KS7010_IRAM_ADDRESS + offset);
if (ret)
goto release_firmware;
 
-   ret = ks7010_sdio_write(priv, DATA_WINDOW, rom_buf, size);
+   ret = ks7010_sdio_write(priv,
+   DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
 
-   ret = ks7010_sdio_data_compare(priv, DATA_WINDOW, rom_buf, 
size);
+   ret = ks7010_sdio_data_compare(priv,
+  DATA_WINDOW, rom_buf, size);
if (ret)
goto release_firmware;
 
@@ -889,7 +893,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
priv = netdev_priv(netdev);
 
card->priv = priv;
-   SET_NETDEV_DEV(netdev, >func->dev);   /* for create sysfs 
symlinks */
+   SET_NETDEV_DEV(netdev, >func->dev);/* for create sysfs symlinks */
 
/* private memory initialize */
priv->ks_sdio_card = card;
@@ -923,7 +927,7 @@ static int ks7010_sdio_probe(struct sdio_func *func,
}
 
/* interrupt setting */
-   /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024) 
*/
+   /* clear Interrupt status write (ARMtoSD_InterruptPending FN1:00_0024)*/
sdio_claim_host(func);
ret = ks7010_sdio_writeb(priv, INT_PENDING, 0xff);
sdio_release_host(func);
@@ -1006,7 +1010,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
struct ks_sdio_card *card;
struct ks_wlan_private *priv;
 
-   DPRINTK(1, "ks7010_sdio_remove()\n");
+   DPRINTK(1, "%s()\n", __func__);
 
card = sdio_get_drvdata(func);
 
-- 
2.7.4

Re: linux-next: build warning after merge of the pinctrl tree

[Stephen Rothwell]

Hi all,

[Just adding cc's]

On Fri, 30 Jun 2017 14:56:01 +1000 Stephen Rothwell  
wrote:
>
> After merging the pinctrl tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
> 
> drivers/pinctrl/pinctrl-rza1.c: In function 'rza1_pinctrl_probe':
> drivers/pinctrl/pinctrl-rza1.c:1260:5: warning: 'ret' may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
>   if (ret)
>  ^
> 
> Introduced by commit
> 
>   5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller")

-- 
Cheers,
Stephen Rothwell

Re: linux-next: build warning after merge of the pinctrl tree

[Stephen Rothwell]

Hi all,

[Just adding cc's]

On Fri, 30 Jun 2017 14:56:01 +1000 Stephen Rothwell  
wrote:
>
> After merging the pinctrl tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
> 
> drivers/pinctrl/pinctrl-rza1.c: In function 'rza1_pinctrl_probe':
> drivers/pinctrl/pinctrl-rza1.c:1260:5: warning: 'ret' may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
>   if (ret)
>  ^
> 
> Introduced by commit
> 
>   5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller")

-- 
Cheers,
Stephen Rothwell

linux-next: build warning after merge of the pinctrl tree

[Stephen Rothwell]

Hi Linus,

After merging the pinctrl tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/pinctrl/pinctrl-rza1.c: In function 'rza1_pinctrl_probe':
drivers/pinctrl/pinctrl-rza1.c:1260:5: warning: 'ret' may be used uninitialized 
in this function [-Wmaybe-uninitialized]
  if (ret)
 ^

Introduced by commit

  5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller")

-- 
Cheers,
Stephen Rothwell

linux-next: build warning after merge of the pinctrl tree

[Stephen Rothwell]

Hi Linus,

After merging the pinctrl tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/pinctrl/pinctrl-rza1.c: In function 'rza1_pinctrl_probe':
drivers/pinctrl/pinctrl-rza1.c:1260:5: warning: 'ret' may be used uninitialized 
in this function [-Wmaybe-uninitialized]
  if (ret)
 ^

Introduced by commit

  5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller")

-- 
Cheers,
Stephen Rothwell

Panic with bochs_drm module on qemu-system-sparc64

[Mark Cave-Ayland]

Hi all,

I'm one of the QEMU SPARC maintainers and I've been investigating why
enabling the fb console via the bochs_drm module causes a panic on
startup. The reproducer with QEMU 2.9 is easy:

$ ./qemu-system-sparc64 -m 512 -kernel rel-sparc/vmlinux -append
'console=ttyS0' -serial stdio

This gives the following panic on the serial console:

[   14.759388] [drm] Found bochs VGA, ID 0xb0c5.
[   14.760018] [drm] Framebuffer size 16384 kB @ 0x1ff0100, mmio @
0x1ff0200.
[   14.763370] [TTM] Zone  kernel: Available graphics memory: 252808 kiB
[   14.764240] [TTM] Initializing pool allocator
[   14.894178] Unable to handle kernel paging request at virtual address
01ff0100
[   14.894247] tsk->{mm,active_mm}->context = 
[   14.894308] tsk->{mm,active_mm}->pgd = f8402000
[   14.894372]   \|/  \|/
[   14.894372]   "@'/ .. \`@"
[   14.894372]   /_| \__/ |_\
[   14.894372]  \__U_/
[   14.894435] swapper/0(1): Oops [#1]
[   14.895400] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc7+ #45
[   14.895634] task: f8001c097800 task.stack: f8001c09c000
[   14.895722] TSTATE: 80001607 TPC: 006f3c24 TNPC:
006f3c30 Y: Not tainted
[   14.896697] TPC: 
[   14.896916] g0: 0080 g1: f8001c36 g2:
 g3: 0007
[   14.896976] g4: f8001c097800 g5: f8001ecea000 g6:
f8001c09c000 g7: 0090dfc0
[   14.897036] o0: 0026 o1: 0001 o2:
000f o3: 01ff0100
[   14.897094] o4: 0007 o5: 0008 sp:
f8001c09e0a1 ret_pc: 0001
[   14.897551] RPC: <0x1>
[   14.897715] l0: 0020 l1: 00a43800 l2:
0080 l3: 0001
[   14.897773] l4: 00b93800 l5: 0080 l6:
0030 l7: 
[   14.897830] i0: f8001c32c000 i1: f8001c36 i2:
 i3: 
[   14.897887] i4: 0001 i5: 01ff0100 i6:
f8001c09e151 i7: 007381d4
[   14.897984] I7: 
[   14.898166] Call Trace:
[   14.898442]  [007381d4] drm_fb_helper_sys_imageblit+0x14/0x34
[   14.898561]  [006e6c9c] soft_cursor+0x174/0x19c
[   14.898601]  [006e6744] bit_cursor+0x45c/0x490
[   14.898641]  [006e324c] fbcon_cursor+0x16c/0x17c
[   14.898685]  [00710f60] hide_cursor+0x2c/0xa8
[   14.898724]  [007120fc] redraw_screen+0xc4/0x208
[   14.898765]  [006e2328] fbcon_prepare_logo+0x288/0x358
[   14.898803]  [006e27cc] fbcon_init+0x3d4/0x448
[   14.898844]  [007113e4] visual_init+0xa4/0x100
[   14.898884]  [00712ba0] do_bind_con_driver+0x1c8/0x300
[   14.898925]  [0071304c] do_take_over_console+0x170/0x198
[   14.898965]  [006e28c4] do_fbcon_takeover+0x84/0xe8
[   14.899017]  [004747dc] notifier_call_chain+0x38/0x74
[   14.899061]  [00474a5c] __blocking_notifier_call_chain+0x28/0x44
[   14.899104]  [006ec214] register_framebuffer+0x2b8/0x2ec
[   14.899147]  [007399f0] drm_fb_helper_initial_config+0x2d0/0x36c
[   14.899294] Disabling lock debugging due to kernel taint
[   14.899551] Caller[007381d4]:
drm_fb_helper_sys_imageblit+0x14/0x34
[   14.899656] Caller[006e6c9c]: soft_cursor+0x174/0x19c
[   14.899696] Caller[006e6744]: bit_cursor+0x45c/0x490
[   14.899735] Caller[006e324c]: fbcon_cursor+0x16c/0x17c
[   14.899774] Caller[00710f60]: hide_cursor+0x2c/0xa8
[   14.899812] Caller[007120fc]: redraw_screen+0xc4/0x208
[   14.899852] Caller[006e2328]: fbcon_prepare_logo+0x288/0x358
[   14.899891] Caller[006e27cc]: fbcon_init+0x3d4/0x448
[   14.899930] Caller[007113e4]: visual_init+0xa4/0x100
[   14.899970] Caller[00712ba0]: do_bind_con_driver+0x1c8/0x300
[   14.900018] Caller[0071304c]: do_take_over_console+0x170/0x198
[   14.900061] Caller[006e28c4]: do_fbcon_takeover+0x84/0xe8
[   14.900132] Caller[004747dc]: notifier_call_chain+0x38/0x74
[   14.900218] Caller[00474a5c]:
__blocking_notifier_call_chain+0x28/0x44
[   14.900263] Caller[006ec214]: register_framebuffer+0x2b8/0x2ec
[   14.900306] Caller[007399f0]:
drm_fb_helper_initial_config+0x2d0/0x36c
[   14.900351] Caller[00761e60]: bochs_fbdev_init+0x6c/0xb0
[   14.900389] Caller[00760b2c]: bochs_load+0x84/0xa8
[   14.900439] Caller[00741c88]: drm_dev_register+0x114/0x1e8
[   14.900628] Caller[00742a10]: drm_get_pci_dev+0xa8/0x118
[   14.900672] Caller[006c8364]: pci_device_probe+0x70/0xdc
[   14.900713] Caller[00768cf0]: driver_probe_device+0x148/0x2a4
[   14.900752] Caller[00768ec4]: __driver_attach+0x78/0xa8
[   14.900790] Caller[007674ec]: bus_for_each_dev+0x58/0x7c
[   

Panic with bochs_drm module on qemu-system-sparc64

[Mark Cave-Ayland]

Hi all,

I'm one of the QEMU SPARC maintainers and I've been investigating why
enabling the fb console via the bochs_drm module causes a panic on
startup. The reproducer with QEMU 2.9 is easy:

$ ./qemu-system-sparc64 -m 512 -kernel rel-sparc/vmlinux -append
'console=ttyS0' -serial stdio

This gives the following panic on the serial console:

[   14.759388] [drm] Found bochs VGA, ID 0xb0c5.
[   14.760018] [drm] Framebuffer size 16384 kB @ 0x1ff0100, mmio @
0x1ff0200.
[   14.763370] [TTM] Zone  kernel: Available graphics memory: 252808 kiB
[   14.764240] [TTM] Initializing pool allocator
[   14.894178] Unable to handle kernel paging request at virtual address
01ff0100
[   14.894247] tsk->{mm,active_mm}->context = 
[   14.894308] tsk->{mm,active_mm}->pgd = f8402000
[   14.894372]   \|/  \|/
[   14.894372]   "@'/ .. \`@"
[   14.894372]   /_| \__/ |_\
[   14.894372]  \__U_/
[   14.894435] swapper/0(1): Oops [#1]
[   14.895400] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc7+ #45
[   14.895634] task: f8001c097800 task.stack: f8001c09c000
[   14.895722] TSTATE: 80001607 TPC: 006f3c24 TNPC:
006f3c30 Y: Not tainted
[   14.896697] TPC: 
[   14.896916] g0: 0080 g1: f8001c36 g2:
 g3: 0007
[   14.896976] g4: f8001c097800 g5: f8001ecea000 g6:
f8001c09c000 g7: 0090dfc0
[   14.897036] o0: 0026 o1: 0001 o2:
000f o3: 01ff0100
[   14.897094] o4: 0007 o5: 0008 sp:
f8001c09e0a1 ret_pc: 0001
[   14.897551] RPC: <0x1>
[   14.897715] l0: 0020 l1: 00a43800 l2:
0080 l3: 0001
[   14.897773] l4: 00b93800 l5: 0080 l6:
0030 l7: 
[   14.897830] i0: f8001c32c000 i1: f8001c36 i2:
 i3: 
[   14.897887] i4: 0001 i5: 01ff0100 i6:
f8001c09e151 i7: 007381d4
[   14.897984] I7: 
[   14.898166] Call Trace:
[   14.898442]  [007381d4] drm_fb_helper_sys_imageblit+0x14/0x34
[   14.898561]  [006e6c9c] soft_cursor+0x174/0x19c
[   14.898601]  [006e6744] bit_cursor+0x45c/0x490
[   14.898641]  [006e324c] fbcon_cursor+0x16c/0x17c
[   14.898685]  [00710f60] hide_cursor+0x2c/0xa8
[   14.898724]  [007120fc] redraw_screen+0xc4/0x208
[   14.898765]  [006e2328] fbcon_prepare_logo+0x288/0x358
[   14.898803]  [006e27cc] fbcon_init+0x3d4/0x448
[   14.898844]  [007113e4] visual_init+0xa4/0x100
[   14.898884]  [00712ba0] do_bind_con_driver+0x1c8/0x300
[   14.898925]  [0071304c] do_take_over_console+0x170/0x198
[   14.898965]  [006e28c4] do_fbcon_takeover+0x84/0xe8
[   14.899017]  [004747dc] notifier_call_chain+0x38/0x74
[   14.899061]  [00474a5c] __blocking_notifier_call_chain+0x28/0x44
[   14.899104]  [006ec214] register_framebuffer+0x2b8/0x2ec
[   14.899147]  [007399f0] drm_fb_helper_initial_config+0x2d0/0x36c
[   14.899294] Disabling lock debugging due to kernel taint
[   14.899551] Caller[007381d4]:
drm_fb_helper_sys_imageblit+0x14/0x34
[   14.899656] Caller[006e6c9c]: soft_cursor+0x174/0x19c
[   14.899696] Caller[006e6744]: bit_cursor+0x45c/0x490
[   14.899735] Caller[006e324c]: fbcon_cursor+0x16c/0x17c
[   14.899774] Caller[00710f60]: hide_cursor+0x2c/0xa8
[   14.899812] Caller[007120fc]: redraw_screen+0xc4/0x208
[   14.899852] Caller[006e2328]: fbcon_prepare_logo+0x288/0x358
[   14.899891] Caller[006e27cc]: fbcon_init+0x3d4/0x448
[   14.899930] Caller[007113e4]: visual_init+0xa4/0x100
[   14.899970] Caller[00712ba0]: do_bind_con_driver+0x1c8/0x300
[   14.900018] Caller[0071304c]: do_take_over_console+0x170/0x198
[   14.900061] Caller[006e28c4]: do_fbcon_takeover+0x84/0xe8
[   14.900132] Caller[004747dc]: notifier_call_chain+0x38/0x74
[   14.900218] Caller[00474a5c]:
__blocking_notifier_call_chain+0x28/0x44
[   14.900263] Caller[006ec214]: register_framebuffer+0x2b8/0x2ec
[   14.900306] Caller[007399f0]:
drm_fb_helper_initial_config+0x2d0/0x36c
[   14.900351] Caller[00761e60]: bochs_fbdev_init+0x6c/0xb0
[   14.900389] Caller[00760b2c]: bochs_load+0x84/0xa8
[   14.900439] Caller[00741c88]: drm_dev_register+0x114/0x1e8
[   14.900628] Caller[00742a10]: drm_get_pci_dev+0xa8/0x118
[   14.900672] Caller[006c8364]: pci_device_probe+0x70/0xdc
[   14.900713] Caller[00768cf0]: driver_probe_device+0x148/0x2a4
[   14.900752] Caller[00768ec4]: __driver_attach+0x78/0xa8
[   14.900790] Caller[007674ec]: bus_for_each_dev+0x58/0x7c
[   14.900830] Caller[007683bc]: bus_add_driver+0xd0/0x1fc
[   

Re: [PATCH 3/6] cpufreq: governor: Drop min_sampling_rate

[Dominik Brodowski]

On Fri, Jun 30, 2017 at 09:04:25AM +0530, Viresh Kumar wrote:
> On 29-06-17, 20:01, Dominik Brodowski wrote:
> > On Thu, Jun 29, 2017 at 04:29:06PM +0530, Viresh Kumar wrote:
> > > The cpufreq core and governors aren't supposed to set a limit on how
> > > fast we want to try changing the frequency. This is currently done for
> > > the legacy governors with help of min_sampling_rate.
> > > 
> > > At worst, we may end up setting the sampling rate to a value lower than
> > > the rate at which frequency can be changed and then one of the CPUs in
> > > the policy will be only changing frequency for ever.
> > 
> > Is it safe to issue requests to change the CPU frequency so frequently,
> 
> Well, I assumed so. I am not sure the hardware would break though.
> Overheating ?
> 
> > even
> > on historic hardware such as speedstep-{ich,smi,centrino}? In the past,
> > these checks more or less disallowed the running of dynamic frequency
> > scaling at least on speedstep-smi[*],
> 
> We must by doing dynamic freq scaling even without this patch. I don't
> see why you say the above then.
> 
> All we do here is that we get rid of the limit on how soon we can
> change the freq again.

Well, as I understand it, first generation "speedstep" was designed more or
less to switch frequencies only when AC power was lost or restored.

The Linux implementation merely said: "no on-the-fly changes", but switch
frequencies whenever a user explicitly requested such a change (presumably
only every once in an unspecified while).

This same reasoning may be present in other drivers using CPUFREQ_ETERNAL.

> > but maybe on a few other platforms as
> > well. That's why I am curious on whether this may break systems potentially
> > on a hardware level if the hardware was not designed to do dynamic frequency
> > scaling (and not just frequency switches on battery/AC).
> 
> Honestly I am not sure if any hardware can break or not, just because
> of this commit.

I am not *sure* either, I am just worried of the consequences of doing
things out-of-spec...

Best
Dominik


signature.asc
Description: PGP signature

Re: [PATCH 3/6] cpufreq: governor: Drop min_sampling_rate

[Dominik Brodowski]

On Fri, Jun 30, 2017 at 09:04:25AM +0530, Viresh Kumar wrote:
> On 29-06-17, 20:01, Dominik Brodowski wrote:
> > On Thu, Jun 29, 2017 at 04:29:06PM +0530, Viresh Kumar wrote:
> > > The cpufreq core and governors aren't supposed to set a limit on how
> > > fast we want to try changing the frequency. This is currently done for
> > > the legacy governors with help of min_sampling_rate.
> > > 
> > > At worst, we may end up setting the sampling rate to a value lower than
> > > the rate at which frequency can be changed and then one of the CPUs in
> > > the policy will be only changing frequency for ever.
> > 
> > Is it safe to issue requests to change the CPU frequency so frequently,
> 
> Well, I assumed so. I am not sure the hardware would break though.
> Overheating ?
> 
> > even
> > on historic hardware such as speedstep-{ich,smi,centrino}? In the past,
> > these checks more or less disallowed the running of dynamic frequency
> > scaling at least on speedstep-smi[*],
> 
> We must by doing dynamic freq scaling even without this patch. I don't
> see why you say the above then.
> 
> All we do here is that we get rid of the limit on how soon we can
> change the freq again.

Well, as I understand it, first generation "speedstep" was designed more or
less to switch frequencies only when AC power was lost or restored.

The Linux implementation merely said: "no on-the-fly changes", but switch
frequencies whenever a user explicitly requested such a change (presumably
only every once in an unspecified while).

This same reasoning may be present in other drivers using CPUFREQ_ETERNAL.

> > but maybe on a few other platforms as
> > well. That's why I am curious on whether this may break systems potentially
> > on a hardware level if the hardware was not designed to do dynamic frequency
> > scaling (and not just frequency switches on battery/AC).
> 
> Honestly I am not sure if any hardware can break or not, just because
> of this commit.

I am not *sure* either, I am just worried of the consequences of doing
things out-of-spec...

Best
Dominik


signature.asc
Description: PGP signature

[PATCH] PM / sleep: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   3802 624  324458116a kernel/power/main.o

File size After adding 'const':
   textdata bss dec hex filename
   3866 560  324458116a kernel/power/main.o

Signed-off-by: Arvind Yadav 
---
 kernel/power/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/power/main.c b/kernel/power/main.c
index d401c21..42bd800 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -705,7 +705,7 @@ static ssize_t pm_freeze_timeout_store(struct kobject *kobj,
NULL,
 };
 
-static struct attribute_group attr_group = {
+static const struct attribute_group attr_group = {
.attrs = g,
 };
 
-- 
1.9.1

[PATCH] PM / sleep: constify attribute_group structures.

[Arvind Yadav]

attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by  work with const
attribute_group. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   3802 624  324458116a kernel/power/main.o

File size After adding 'const':
   textdata bss dec hex filename
   3866 560  324458116a kernel/power/main.o

Signed-off-by: Arvind Yadav 
---
 kernel/power/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/power/main.c b/kernel/power/main.c
index d401c21..42bd800 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -705,7 +705,7 @@ static ssize_t pm_freeze_timeout_store(struct kobject *kobj,
NULL,
 };
 
-static struct attribute_group attr_group = {
+static const struct attribute_group attr_group = {
.attrs = g,
 };
 
-- 
1.9.1

[PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Gustavo A. R. Silva]

Propagate the return values of platform_get_irq and
devm_request_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/clocksource/em_sti.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index bc48cbf..c4818dd 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "failed to get irq\n");
-   return -EINVAL;
+   return irq;
}
 
/* map memory, let base point to the STI instance */
@@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
if (IS_ERR(p->base))
return PTR_ERR(p->base);
 
-   if (devm_request_irq(>dev, irq, em_sti_interrupt,
+   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
 IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-dev_name(>dev), p)) {
+dev_name(>dev), p);
+   if (irq) {
dev_err(>dev, "failed to request low IRQ\n");
-   return -ENOENT;
+   return irq;
}
 
/* get hold of clock */
-- 
2.5.0

[PATCH] clocksource: em_sti: fix error return codes in em_sti_probe()

[Gustavo A. R. Silva]

Propagate the return values of platform_get_irq and
devm_request_irq on failure.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/clocksource/em_sti.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index bc48cbf..c4818dd 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -305,7 +305,7 @@ static int em_sti_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "failed to get irq\n");
-   return -EINVAL;
+   return irq;
}
 
/* map memory, let base point to the STI instance */
@@ -314,11 +314,12 @@ static int em_sti_probe(struct platform_device *pdev)
if (IS_ERR(p->base))
return PTR_ERR(p->base);
 
-   if (devm_request_irq(>dev, irq, em_sti_interrupt,
+   irq = devm_request_irq(>dev, irq, em_sti_interrupt,
 IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-dev_name(>dev), p)) {
+dev_name(>dev), p);
+   if (irq) {
dev_err(>dev, "failed to request low IRQ\n");
-   return -ENOENT;
+   return irq;
}
 
/* get hold of clock */
-- 
2.5.0

Re: [RFC 0/5] drivers: Add boot constraints core

[Chen-Yu Tsai]

On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar  wrote:
> On 30-06-17, 12:05, Chen-Yu Tsai wrote:
>> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar  
>> wrote:
>> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:
>> >> AFAIK regulator constraints are supposed to satisfy all users of it.
>> >
>> > Right.
>> >
>> >> >> >Let me try with an example. A regulator is shared between LCD and DMA
>> >> >> >controller.
>> >> >> >
>> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V
>> >> >> >Range required by LCD: 2.0 - 3.0 V
>> >> >> >Range required by DMA: 1.8 - 2.5 V
>> >>
>> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,
>> >> or the intersection of all voltage requirements.
>> >
>> > Had a look at regulator_check_consumers() and the range selected by it
>> > is the *highest* min_uV and *lowest* max_uV, to find that intersection
>> > point.
>> >
>> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V
>> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V
>> >
>> > Highest min_uV = 2.0 V
>> > Lowest max_uV = 2.5 V
>> >
>> > And so I mentioned the regulator's final range (that satisfies all
>> > consumers) is 2 - 2.5 V.
>> >
>> > Why do you say it should be 2.5 - 3.0 V ?
>>
>> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this
>> morning. :(
>
> And I was worrying if I had something else in my coffee :)
>
>> I also want to mention that for DT based platforms, this constraint
>> should already be set in the device tree for the regulator, so the
>> scenario where DMA comes up and sets a voltage level that LCD cannot
>> use should not even be possible.
>
> Yes, such constraints are already present. But the problem (this
> series is trying to solve) is that the kernel doesn't know if the LCD
> is already powered ON. And so when DMA gets probed first, the kernel
> thinks that DMA is the only user of the regulator and the voltage is
> set to 1.8-2.5 V. And so this series is somehow trying to make the
> kernel aware about the constraints of the LCD controller which was
> enabled in the bootloader.

What I'm saying is for the DT case, the constraints are already limited
to the intersection of all users, regardless of whether they are turned
on or not. At least this is what I believe makes sense. You really don't
want to set a regulator such that it over voltages for a subset of its
consumers. Consumers might not have proper power isolation for this.

I think what you mean is that the DT constraints are the union of all
consumer constraints (1.8 - 3.0 V in this case), then each consumer
comes in and adds its own constraints. And for such a design, the kernel
needs to know which and what constraints to apply.

Either way regulators already support constraints, so they are easier
to deal with. Clocks on the other hand, while the core does support
clock rate constraints, AFAIK no one really uses or supports them.

ChenYu

Re: [RFC 0/5] drivers: Add boot constraints core

[Chen-Yu Tsai]

On Fri, Jun 30, 2017 at 12:12 PM, Viresh Kumar  wrote:
> On 30-06-17, 12:05, Chen-Yu Tsai wrote:
>> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar  
>> wrote:
>> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:
>> >> AFAIK regulator constraints are supposed to satisfy all users of it.
>> >
>> > Right.
>> >
>> >> >> >Let me try with an example. A regulator is shared between LCD and DMA
>> >> >> >controller.
>> >> >> >
>> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V
>> >> >> >Range required by LCD: 2.0 - 3.0 V
>> >> >> >Range required by DMA: 1.8 - 2.5 V
>> >>
>> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,
>> >> or the intersection of all voltage requirements.
>> >
>> > Had a look at regulator_check_consumers() and the range selected by it
>> > is the *highest* min_uV and *lowest* max_uV, to find that intersection
>> > point.
>> >
>> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V
>> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V
>> >
>> > Highest min_uV = 2.0 V
>> > Lowest max_uV = 2.5 V
>> >
>> > And so I mentioned the regulator's final range (that satisfies all
>> > consumers) is 2 - 2.5 V.
>> >
>> > Why do you say it should be 2.5 - 3.0 V ?
>>
>> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this
>> morning. :(
>
> And I was worrying if I had something else in my coffee :)
>
>> I also want to mention that for DT based platforms, this constraint
>> should already be set in the device tree for the regulator, so the
>> scenario where DMA comes up and sets a voltage level that LCD cannot
>> use should not even be possible.
>
> Yes, such constraints are already present. But the problem (this
> series is trying to solve) is that the kernel doesn't know if the LCD
> is already powered ON. And so when DMA gets probed first, the kernel
> thinks that DMA is the only user of the regulator and the voltage is
> set to 1.8-2.5 V. And so this series is somehow trying to make the
> kernel aware about the constraints of the LCD controller which was
> enabled in the bootloader.

What I'm saying is for the DT case, the constraints are already limited
to the intersection of all users, regardless of whether they are turned
on or not. At least this is what I believe makes sense. You really don't
want to set a regulator such that it over voltages for a subset of its
consumers. Consumers might not have proper power isolation for this.

I think what you mean is that the DT constraints are the union of all
consumer constraints (1.8 - 3.0 V in this case), then each consumer
comes in and adds its own constraints. And for such a design, the kernel
needs to know which and what constraints to apply.

Either way regulators already support constraints, so they are easier
to deal with. Clocks on the other hand, while the core does support
clock rate constraints, AFAIK no one really uses or supports them.

ChenYu

Re: [RFC 0/5] drivers: Add boot constraints core

[Viresh Kumar]

On 30-06-17, 12:05, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar  
> wrote:
> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:
> >> AFAIK regulator constraints are supposed to satisfy all users of it.
> >
> > Right.
> >
> >> >> >Let me try with an example. A regulator is shared between LCD and DMA
> >> >> >controller.
> >> >> >
> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V
> >> >> >Range required by LCD: 2.0 - 3.0 V
> >> >> >Range required by DMA: 1.8 - 2.5 V
> >>
> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,
> >> or the intersection of all voltage requirements.
> >
> > Had a look at regulator_check_consumers() and the range selected by it
> > is the *highest* min_uV and *lowest* max_uV, to find that intersection
> > point.
> >
> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V
> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V
> >
> > Highest min_uV = 2.0 V
> > Lowest max_uV = 2.5 V
> >
> > And so I mentioned the regulator's final range (that satisfies all
> > consumers) is 2 - 2.5 V.
> >
> > Why do you say it should be 2.5 - 3.0 V ?
> 
> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this
> morning. :(

And I was worrying if I had something else in my coffee :)

> I also want to mention that for DT based platforms, this constraint
> should already be set in the device tree for the regulator, so the
> scenario where DMA comes up and sets a voltage level that LCD cannot
> use should not even be possible.

Yes, such constraints are already present. But the problem (this
series is trying to solve) is that the kernel doesn't know if the LCD
is already powered ON. And so when DMA gets probed first, the kernel
thinks that DMA is the only user of the regulator and the voltage is
set to 1.8-2.5 V. And so this series is somehow trying to make the
kernel aware about the constraints of the LCD controller which was
enabled in the bootloader.

-- 
viresh

Re: [RFC 0/5] drivers: Add boot constraints core

[Viresh Kumar]

On 30-06-17, 12:05, Chen-Yu Tsai wrote:
> On Fri, Jun 30, 2017 at 11:55 AM, Viresh Kumar  
> wrote:
> > On 30-06-17, 11:33, Chen-Yu Tsai wrote:
> >> AFAIK regulator constraints are supposed to satisfy all users of it.
> >
> > Right.
> >
> >> >> >Let me try with an example. A regulator is shared between LCD and DMA
> >> >> >controller.
> >> >> >
> >> >> >Operable ranges of the regulator: 1.8 - 3.0 V
> >> >> >Range required by LCD: 2.0 - 3.0 V
> >> >> >Range required by DMA: 1.8 - 2.5 V
> >>
> >> So for the example here, the regulator constraint should be 2.5 - 3.0 V,
> >> or the intersection of all voltage requirements.
> >
> > Had a look at regulator_check_consumers() and the range selected by it
> > is the *highest* min_uV and *lowest* max_uV, to find that intersection
> > point.
> >
> > For LCD: min_uV = 2.0 V, max_uV = 3.0 V
> > For DMA: min_uV = 1.8 V, max_uV = 2.5 V
> >
> > Highest min_uV = 2.0 V
> > Lowest max_uV = 2.5 V
> >
> > And so I mentioned the regulator's final range (that satisfies all
> > consumers) is 2 - 2.5 V.
> >
> > Why do you say it should be 2.5 - 3.0 V ?
> 
> You are right. It should be 2.0 - 2.5 V. Haven't had my coffee this
> morning. :(

And I was worrying if I had something else in my coffee :)

> I also want to mention that for DT based platforms, this constraint
> should already be set in the device tree for the regulator, so the
> scenario where DMA comes up and sets a voltage level that LCD cannot
> use should not even be possible.

Yes, such constraints are already present. But the problem (this
series is trying to solve) is that the kernel doesn't know if the LCD
is already powered ON. And so when DMA gets probed first, the kernel
thinks that DMA is the only user of the regulator and the voltage is
set to 1.8-2.5 V. And so this series is somehow trying to make the
kernel aware about the constraints of the LCD controller which was
enabled in the bootloader.

-- 
viresh

[PATCH v5 02/12] x86/apic: Prepare for unifying the interrupt delivery modes setup

[Dou Liyang]

There are three positions for initializing the interrupt delivery
modes:

1) In IRQ initial function, may setup the through-local-APIC
   virtual wire mode.

2) In an SMP-capable system, will try to switch to symmetric I/O
   model when preparing the cpus in native_smp_prepare_cpus().

3) In UP system with UP_LATE_INIT=y, will set up local APIC and
   I/O APIC in smp_init().

Switching to symmetric I/O mode is so late, which causes kernel
in an unmatched mode at the beginning of booting time. And it
causes the dump-capture kernel hangs with 'notsc' option inherited
from 1st kernel option.

Provide a new function to unify that three positions. Preparatory
patch to initialize an interrupt mode directly.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  2 ++
 arch/x86/kernel/apic/apic.c | 19 +++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bdffcd9..ddc16ff 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -128,6 +128,7 @@ extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
 extern void init_bsp_APIC(void);
+extern void apic_intr_mode_init(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
 void register_lapic_address(unsigned long address);
@@ -170,6 +171,7 @@ static inline void disable_local_APIC(void) { }
 # define setup_boot_APIC_clock x86_init_noop
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
+static inline void apic_intr_mode_init(void) { }
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_X2APIC
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 88be65e..00d97f4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1266,6 +1266,25 @@ void __init init_bsp_APIC(void)
apic_write(APIC_LVT1, value);
 }
 
+/* Init the interrupt delivery mode for the BSP */
+void __init apic_intr_mode_init(void)
+{
+   switch (apic_intr_mode_select()) {
+   case APIC_PIC:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Keep in PIC mode(8259)\n");
+   return;
+   case APIC_VIRTUAL_WIRE:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to virtual wire mode setup\n");
+   return;
+   case APIC_SYMMETRIC_IO:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to symmectic I/O mode setup\n");
+   return;
+   }
+}
+
 static void lapic_setup_esr(void)
 {
unsigned int oldvalue, value, maxlvt;
-- 
2.5.5

[PATCH v5 02/12] x86/apic: Prepare for unifying the interrupt delivery modes setup

[Dou Liyang]

There are three positions for initializing the interrupt delivery
modes:

1) In IRQ initial function, may setup the through-local-APIC
   virtual wire mode.

2) In an SMP-capable system, will try to switch to symmetric I/O
   model when preparing the cpus in native_smp_prepare_cpus().

3) In UP system with UP_LATE_INIT=y, will set up local APIC and
   I/O APIC in smp_init().

Switching to symmetric I/O mode is so late, which causes kernel
in an unmatched mode at the beginning of booting time. And it
causes the dump-capture kernel hangs with 'notsc' option inherited
from 1st kernel option.

Provide a new function to unify that three positions. Preparatory
patch to initialize an interrupt mode directly.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  2 ++
 arch/x86/kernel/apic/apic.c | 19 +++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bdffcd9..ddc16ff 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -128,6 +128,7 @@ extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
 extern void init_bsp_APIC(void);
+extern void apic_intr_mode_init(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
 void register_lapic_address(unsigned long address);
@@ -170,6 +171,7 @@ static inline void disable_local_APIC(void) { }
 # define setup_boot_APIC_clock x86_init_noop
 # define setup_secondary_APIC_clock x86_init_noop
 static inline void lapic_update_tsc_freq(void) { }
+static inline void apic_intr_mode_init(void) { }
 #endif /* !CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_X2APIC
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 88be65e..00d97f4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1266,6 +1266,25 @@ void __init init_bsp_APIC(void)
apic_write(APIC_LVT1, value);
 }
 
+/* Init the interrupt delivery mode for the BSP */
+void __init apic_intr_mode_init(void)
+{
+   switch (apic_intr_mode_select()) {
+   case APIC_PIC:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Keep in PIC mode(8259)\n");
+   return;
+   case APIC_VIRTUAL_WIRE:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to virtual wire mode setup\n");
+   return;
+   case APIC_SYMMETRIC_IO:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to symmectic I/O mode setup\n");
+   return;
+   }
+}
+
 static void lapic_setup_esr(void)
 {
unsigned int oldvalue, value, maxlvt;
-- 
2.5.5

[PATCH v5 00/12] Unify the interrupt delivery mode and do its setup in advance

[Dou Liyang]

[Background]

MP specification defines three different interrupt delivery modes as follows:

 1. PIC Mode
 2. Virtual Wire Mode
 3. Symmetric I/O Mode

They will be setup in the different periods of booting time:
 1. *PIC Mode*, the default interrupt delivery modes, will be set first.
 2. *Virtual Wire Mode* will be setup during ISA IRQ initialization( step 1
in the figure.1).
 3. *Symmetric I/O Mode*'s setup is related to the system
3.1 In SMP-capable system, setup during prepares CPUs(step 2) 
3.2 In UP system, setup during initializes itself(step 3).


 start_kernel
+---+
|
+--> ...
|
|setup_arch
+--> +---+
|
|init_IRQ
+-> +--+-+
|  |init_ISA_irqs
|  +--> +-++
| | ++
+---> +-->  | 1.init_bsp_APIC|
| ...   ++
+--->
| rest_init
+--->---+-+
|   |   kernel_init
|   +> +-+
|  |   kernel_init_freeable
|  +->  +-+
|   | smp_prepare_cpus
|   +---> ++-+
|   |  |   +---+
|   |  +-> |2.  apic_bsp_setup |
|   |  +---+
|   |
v   | smp_init
+---> +---++
  |+---+
  +--> |3.  apic_bsp_setup |
   +---+
figure.1 The flow chart of the kernel startup process

[Problem]

1. Cause kernel in an unmatched mode at the beginning of booting time.
2. Cause the dump-capture kernel hangs with 'notsc' option inherited
   from 1st kernel option.
3. Cause the code hard to read and maintain.

As Ingo's and Eric's discusses[1,2], it need to be refactor.

[Solution]

1. Construct a selector to unify these switches

   ++
   |disable_apic++
   ++   true |
  |false |
  |  |
 +v--+   |
 |!boot_cpu_has(X86_FEATURE_APIC)+---+
 +---+  true |
  |false |
  |  |
  +---v-+v
  |!smp_found_config|PIC MODE
  +---+-+
   |false |true
   |  |
   v  +---v-+
SYMMETRIC IO MODE | !acpi_lapic |
  +--+--+
 |
 v
   VIRTUAL WIRE MODE 

2. Unifying these setup steps of SMP-capable and UP system

   start_kernel
---+
|
|
|
|x86_late_time_init
+>---++
||
||  ++
|+> | 4. init_interrupt_mode |
|   ++
v


3. Execute the function as soon as possible.

[Test]

1. In a theoretical code analysis, the patchset can wrap the original
logic.

1) The original logic of the interrupt delivery mode setup：

-Step O_1) Keep in PIC mode or virtual wire mode:

  Check (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
true:  PIC mode
false: virtual wire mode

-Step O_2) Try to switch to symmetric IO mode:
  O_2_1) In up system:

-Check disable_apic
  ture: *O_S_1* (original situation 1)
-Check whether there is a separate or integrated chip
  don't has: *O_S_2*
-Check !smp_found_config
  ture: *O_S_3*
-Others:
  *O_S_4*

  O_2_2) In smp-capable system:

-Check !smp_found_config && !acpi_lapic
  true: goto *O_2_1)*
-Check if it is LAPIC
  don't has: *O_S_5*
-Check !max_cpus
  true: *O_S_6*
-read_apic_id() != boot_cpu_physical_apicid
  true: *O_S_7*
-Others:
*O_S_8*

2) After that patchset, the new logic:

-Step N_1) Skip step O_1 and try to switch to the final interrupt mode
   -Check disable_apic
 ture: *N_S_1* (New situation 1)
   -Check whether there is a separate or integrated chip
 ture: *N_S_2*
   -Check if (!smp_found_config)
 ture: *N_S_3*
   -Check !setup_max_cpus
 ture: *N_S_4*
   -Check read_apic_id() != boot_cpu_physical_apicid
 ture: *N_S_5*
   -Others:
   *N_S_6*

O_S_1 is covered in N_S_1
O_S_2 is covered in N_S_2
O_S_3 is covered in N_S_3
O_S_4 is covered in N_S_6
O_S_5 is covered in N_S_2
O_S_6 is covered in N_S_4
O_S_7 is covered in N_S_5
O_S_8 is covered in N_S_6

2. In the actual test, It also can work well in the situations of
my test matrix

The factors of test matrix:

 X86  | SMP |LOCAL APIC|I/O APIC|UP_LATE_INIT|
- |-|--|||
32-bit|  Y  | Y|Y   | Y  |
64-bit|  N  | N|N   | N  |
xen PV 

[PATCH v5 05/12] x86/apic: Unify interrupt mode setup for SMP-capable system

[Dou Liyang]

In the SMP-capable system, enable and setup the interrupt delivery
mode in native_smp_prepare_cpus().

This design mixs the APIC and SMP together, it has highly coupling.

Make the initialization of interrupt mode independent, Unify and
refine it to apic_intr_mode_init() for SMP-capable system.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 41 -
 arch/x86/kernel/smpboot.c   | 13 ++---
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 498edbe..bea8032 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1160,10 +1160,12 @@ void __init sync_Arb_IDs(void)
 enum apic_intr_mode {
APIC_PIC,
APIC_VIRTUAL_WIRE,
+   APIC_VIRTUAL_WIRE_NO_CONFIG,
APIC_SYMMETRIC_IO,
+   APIC_SYMMETRIC_IO_NO_ROUTING,
 };
 
-static int __init apic_intr_mode_select(void)
+static int __init apic_intr_mode_select(int *upmode)
 {
/* Check kernel option */
if (disable_apic) {
@@ -1206,12 +1208,30 @@ static int __init apic_intr_mode_select(void)
if (!smp_found_config) {
disable_ioapic_support();
 
-   if (!acpi_lapic)
+   if (!acpi_lapic) {
pr_info("APIC: ACPI MADT or MP tables are not 
detected\n");
+   *upmode = true;
+
+   return APIC_VIRTUAL_WIRE_NO_CONFIG;
+   }
 
return APIC_VIRTUAL_WIRE;
}
 
+#ifdef CONFIG_SMP
+   /* If SMP should be disabled, then really disable it! */
+   if (!setup_max_cpus) {
+   pr_info("APIC: SMP mode deactivated\n");
+   return APIC_SYMMETRIC_IO_NO_ROUTING;
+   }
+
+   if (read_apic_id() != boot_cpu_physical_apicid) {
+   panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
+read_apic_id(), boot_cpu_physical_apicid);
+   /* Or can we switch back to PIC here? */
+   }
+#endif
+
/* Other checks of APIC options will be done in each setup function */
 
return APIC_SYMMETRIC_IO;
@@ -1269,20 +1289,31 @@ void __init init_bsp_APIC(void)
 /* Init the interrupt delivery mode for the BSP */
 void __init apic_intr_mode_init(void)
 {
-   switch (apic_intr_mode_select()) {
+   int upmode = false;
+
+   switch (apic_intr_mode_select()) {
case APIC_PIC:
apic_printk(APIC_VERBOSE, KERN_INFO
"Keep in PIC mode(8259)\n");
return;
case APIC_VIRTUAL_WIRE:
+   case APIC_VIRTUAL_WIRE_NO_CONFIG:
apic_printk(APIC_VERBOSE, KERN_INFO
"Switch to virtual wire mode setup\n");
-   return;
+   default_setup_apic_routing();
+   break;
case APIC_SYMMETRIC_IO:
apic_printk(APIC_VERBOSE, KERN_INFO
"Switch to symmectic I/O mode setup\n");
-   return;
+   default_setup_apic_routing();
+   break;
+   case APIC_SYMMETRIC_IO_NO_ROUTING:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to symmectic I/O mode setup in no SMP 
routine\n");
+   break;
}
+
+   apic_bsp_setup(upmode);
 }
 
 static void lapic_setup_esr(void)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index d6721f0..b9b2a43 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1321,18 +1321,17 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
 
set_cpu_sibling_map(0);
 
+   apic_intr_mode_init();
+
switch (smp_sanity_check(max_cpus)) {
case SMP_NO_CONFIG:
disable_smp();
-   if (APIC_init_uniprocessor())
-   pr_notice("Local APIC not detected. Using dummy APIC 
emulation.\n");
return;
case SMP_NO_APIC:
disable_smp();
return;
case SMP_FORCE_UP:
disable_smp();
-   apic_bsp_setup(false);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
@@ -1340,14 +1339,6 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
break;
}
 
-   if (read_apic_id() != boot_cpu_physical_apicid) {
-   panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
-read_apic_id(), boot_cpu_physical_apicid);
-   /* Or can we switch back to PIC here? */
-   }
-
-   default_setup_apic_routing();
-   apic_bsp_setup(false);
if (x2apic_mode)
cpu0_logical_apicid = apic_read(APIC_LDR);
else
-- 
2.5.5

[PATCH v5 06/12] x86/apic: Mark the apic_intr_mode extern for sanity check cleanup

[Dou Liyang]

Calling native_smp_prepare_cpus() to prepare for SMP bootup, does
some sanity checking, enables APIC mode and disables SMP feature.

Now, APIC mode setup has been unified to apic_intr_mode_init(),
some sanity checks are redundant and need to be cleanup.

Mark the apic_intr_mode extern to refine the switch and remove
the redundant sanity check.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  9 
 arch/x86/kernel/apic/apic.c | 11 +++---
 arch/x86/kernel/smpboot.c   | 50 -
 3 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index c3bedbd..bfbf715 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -53,6 +53,15 @@ extern int local_apic_timer_c2_ok;
 extern int disable_apic;
 extern unsigned int lapic_timer_frequency;
 
+extern enum apic_intr_mode_id apic_intr_mode;
+enum apic_intr_mode_id {
+   APIC_PIC,
+   APIC_VIRTUAL_WIRE,
+   APIC_VIRTUAL_WIRE_NO_CONFIG,
+   APIC_SYMMETRIC_IO,
+   APIC_SYMMETRIC_IO_NO_ROUTING
+};
+
 #ifdef CONFIG_SMP
 extern void __inquire_remote_apic(int apicid);
 #else /* CONFIG_SMP */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index bea8032..0601054 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1157,13 +1157,7 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
 }
 
-enum apic_intr_mode {
-   APIC_PIC,
-   APIC_VIRTUAL_WIRE,
-   APIC_VIRTUAL_WIRE_NO_CONFIG,
-   APIC_SYMMETRIC_IO,
-   APIC_SYMMETRIC_IO_NO_ROUTING,
-};
+enum apic_intr_mode_id apic_intr_mode;
 
 static int __init apic_intr_mode_select(int *upmode)
 {
@@ -1291,7 +1285,8 @@ void __init apic_intr_mode_init(void)
 {
int upmode = false;
 
-   switch (apic_intr_mode_select()) {
+   apic_intr_mode = apic_intr_mode_select();
+   switch (apic_intr_mode) {
case APIC_PIC:
apic_printk(APIC_VERBOSE, KERN_INFO
"Keep in PIC mode(8259)\n");
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b9b2a43..2db61dcb 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1193,7 +1193,7 @@ enum {
 /*
  * Various sanity checks.
  */
-static int __init smp_sanity_check(unsigned max_cpus)
+static void __init smp_sanity_check(void)
 {
preempt_disable();
 
@@ -1231,16 +1231,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
}
 
/*
-* If we couldn't find an SMP configuration at boot time,
-* get out of here now!
-*/
-   if (!smp_found_config && !acpi_lapic) {
-   preempt_enable();
-   pr_notice("SMP motherboard not detected\n");
-   return SMP_NO_CONFIG;
-   }
-
-   /*
 * Should not be necessary because the MP table should list the boot
 * CPU too, but we do it for the sake of robustness anyway.
 */
@@ -1250,29 +1240,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
physid_set(hard_smp_processor_id(), phys_cpu_present_map);
}
preempt_enable();
-
-   /*
-* If we couldn't find a local APIC, then get out of here now!
-*/
-   if (APIC_INTEGRATED(boot_cpu_apic_version) &&
-   !boot_cpu_has(X86_FEATURE_APIC)) {
-   if (!disable_apic) {
-   pr_err("BIOS bug, local APIC #%d not detected!...\n",
-   boot_cpu_physical_apicid);
-   pr_err("... forcing use of dummy APIC emulation (tell 
your hw vendor)\n");
-   }
-   return SMP_NO_APIC;
-   }
-
-   /*
-* If SMP should be disabled, then really disable it!
-*/
-   if (!max_cpus) {
-   pr_info("SMP mode deactivated\n");
-   return SMP_FORCE_UP;
-   }
-
-   return SMP_OK;
 }
 
 static void __init smp_cpu_index_default(void)
@@ -1322,20 +1289,20 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
set_cpu_sibling_map(0);
 
apic_intr_mode_init();
+   smp_sanity_check();
 
-   switch (smp_sanity_check(max_cpus)) {
-   case SMP_NO_CONFIG:
-   disable_smp();
-   return;
-   case SMP_NO_APIC:
+   switch (apic_intr_mode) {
+   case APIC_PIC:
+   case APIC_VIRTUAL_WIRE_NO_CONFIG:
disable_smp();
return;
-   case SMP_FORCE_UP:
+   case APIC_SYMMETRIC_IO_NO_ROUTING:
disable_smp();
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
-   case SMP_OK:
+   case APIC_VIRTUAL_WIRE:
+   case APIC_SYMMETRIC_IO:
break;
}
 
@@ -1343,6 +1310,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)

[PATCH v5 00/12] Unify the interrupt delivery mode and do its setup in advance

[Dou Liyang]

[Background]

MP specification defines three different interrupt delivery modes as follows:

 1. PIC Mode
 2. Virtual Wire Mode
 3. Symmetric I/O Mode

They will be setup in the different periods of booting time:
 1. *PIC Mode*, the default interrupt delivery modes, will be set first.
 2. *Virtual Wire Mode* will be setup during ISA IRQ initialization( step 1
in the figure.1).
 3. *Symmetric I/O Mode*'s setup is related to the system
3.1 In SMP-capable system, setup during prepares CPUs(step 2) 
3.2 In UP system, setup during initializes itself(step 3).


 start_kernel
+---+
|
+--> ...
|
|setup_arch
+--> +---+
|
|init_IRQ
+-> +--+-+
|  |init_ISA_irqs
|  +--> +-++
| | ++
+---> +-->  | 1.init_bsp_APIC|
| ...   ++
+--->
| rest_init
+--->---+-+
|   |   kernel_init
|   +> +-+
|  |   kernel_init_freeable
|  +->  +-+
|   | smp_prepare_cpus
|   +---> ++-+
|   |  |   +---+
|   |  +-> |2.  apic_bsp_setup |
|   |  +---+
|   |
v   | smp_init
+---> +---++
  |+---+
  +--> |3.  apic_bsp_setup |
   +---+
figure.1 The flow chart of the kernel startup process

[Problem]

1. Cause kernel in an unmatched mode at the beginning of booting time.
2. Cause the dump-capture kernel hangs with 'notsc' option inherited
   from 1st kernel option.
3. Cause the code hard to read and maintain.

As Ingo's and Eric's discusses[1,2], it need to be refactor.

[Solution]

1. Construct a selector to unify these switches

   ++
   |disable_apic++
   ++   true |
  |false |
  |  |
 +v--+   |
 |!boot_cpu_has(X86_FEATURE_APIC)+---+
 +---+  true |
  |false |
  |  |
  +---v-+v
  |!smp_found_config|PIC MODE
  +---+-+
   |false |true
   |  |
   v  +---v-+
SYMMETRIC IO MODE | !acpi_lapic |
  +--+--+
 |
 v
   VIRTUAL WIRE MODE 

2. Unifying these setup steps of SMP-capable and UP system

   start_kernel
---+
|
|
|
|x86_late_time_init
+>---++
||
||  ++
|+> | 4. init_interrupt_mode |
|   ++
v


3. Execute the function as soon as possible.

[Test]

1. In a theoretical code analysis, the patchset can wrap the original
logic.

1) The original logic of the interrupt delivery mode setup：

-Step O_1) Keep in PIC mode or virtual wire mode:

  Check (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
true:  PIC mode
false: virtual wire mode

-Step O_2) Try to switch to symmetric IO mode:
  O_2_1) In up system:

-Check disable_apic
  ture: *O_S_1* (original situation 1)
-Check whether there is a separate or integrated chip
  don't has: *O_S_2*
-Check !smp_found_config
  ture: *O_S_3*
-Others:
  *O_S_4*

  O_2_2) In smp-capable system:

-Check !smp_found_config && !acpi_lapic
  true: goto *O_2_1)*
-Check if it is LAPIC
  don't has: *O_S_5*
-Check !max_cpus
  true: *O_S_6*
-read_apic_id() != boot_cpu_physical_apicid
  true: *O_S_7*
-Others:
*O_S_8*

2) After that patchset, the new logic:

-Step N_1) Skip step O_1 and try to switch to the final interrupt mode
   -Check disable_apic
 ture: *N_S_1* (New situation 1)
   -Check whether there is a separate or integrated chip
 ture: *N_S_2*
   -Check if (!smp_found_config)
 ture: *N_S_3*
   -Check !setup_max_cpus
 ture: *N_S_4*
   -Check read_apic_id() != boot_cpu_physical_apicid
 ture: *N_S_5*
   -Others:
   *N_S_6*

O_S_1 is covered in N_S_1
O_S_2 is covered in N_S_2
O_S_3 is covered in N_S_3
O_S_4 is covered in N_S_6
O_S_5 is covered in N_S_2
O_S_6 is covered in N_S_4
O_S_7 is covered in N_S_5
O_S_8 is covered in N_S_6

2. In the actual test, It also can work well in the situations of
my test matrix

The factors of test matrix:

 X86  | SMP |LOCAL APIC|I/O APIC|UP_LATE_INIT|
- |-|--|||
32-bit|  Y  | Y|Y   | Y  |
64-bit|  N  | N|N   | N  |
xen PV 

[PATCH v5 05/12] x86/apic: Unify interrupt mode setup for SMP-capable system

[Dou Liyang]

In the SMP-capable system, enable and setup the interrupt delivery
mode in native_smp_prepare_cpus().

This design mixs the APIC and SMP together, it has highly coupling.

Make the initialization of interrupt mode independent, Unify and
refine it to apic_intr_mode_init() for SMP-capable system.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 41 -
 arch/x86/kernel/smpboot.c   | 13 ++---
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 498edbe..bea8032 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1160,10 +1160,12 @@ void __init sync_Arb_IDs(void)
 enum apic_intr_mode {
APIC_PIC,
APIC_VIRTUAL_WIRE,
+   APIC_VIRTUAL_WIRE_NO_CONFIG,
APIC_SYMMETRIC_IO,
+   APIC_SYMMETRIC_IO_NO_ROUTING,
 };
 
-static int __init apic_intr_mode_select(void)
+static int __init apic_intr_mode_select(int *upmode)
 {
/* Check kernel option */
if (disable_apic) {
@@ -1206,12 +1208,30 @@ static int __init apic_intr_mode_select(void)
if (!smp_found_config) {
disable_ioapic_support();
 
-   if (!acpi_lapic)
+   if (!acpi_lapic) {
pr_info("APIC: ACPI MADT or MP tables are not 
detected\n");
+   *upmode = true;
+
+   return APIC_VIRTUAL_WIRE_NO_CONFIG;
+   }
 
return APIC_VIRTUAL_WIRE;
}
 
+#ifdef CONFIG_SMP
+   /* If SMP should be disabled, then really disable it! */
+   if (!setup_max_cpus) {
+   pr_info("APIC: SMP mode deactivated\n");
+   return APIC_SYMMETRIC_IO_NO_ROUTING;
+   }
+
+   if (read_apic_id() != boot_cpu_physical_apicid) {
+   panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
+read_apic_id(), boot_cpu_physical_apicid);
+   /* Or can we switch back to PIC here? */
+   }
+#endif
+
/* Other checks of APIC options will be done in each setup function */
 
return APIC_SYMMETRIC_IO;
@@ -1269,20 +1289,31 @@ void __init init_bsp_APIC(void)
 /* Init the interrupt delivery mode for the BSP */
 void __init apic_intr_mode_init(void)
 {
-   switch (apic_intr_mode_select()) {
+   int upmode = false;
+
+   switch (apic_intr_mode_select()) {
case APIC_PIC:
apic_printk(APIC_VERBOSE, KERN_INFO
"Keep in PIC mode(8259)\n");
return;
case APIC_VIRTUAL_WIRE:
+   case APIC_VIRTUAL_WIRE_NO_CONFIG:
apic_printk(APIC_VERBOSE, KERN_INFO
"Switch to virtual wire mode setup\n");
-   return;
+   default_setup_apic_routing();
+   break;
case APIC_SYMMETRIC_IO:
apic_printk(APIC_VERBOSE, KERN_INFO
"Switch to symmectic I/O mode setup\n");
-   return;
+   default_setup_apic_routing();
+   break;
+   case APIC_SYMMETRIC_IO_NO_ROUTING:
+   apic_printk(APIC_VERBOSE, KERN_INFO
+   "Switch to symmectic I/O mode setup in no SMP 
routine\n");
+   break;
}
+
+   apic_bsp_setup(upmode);
 }
 
 static void lapic_setup_esr(void)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index d6721f0..b9b2a43 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1321,18 +1321,17 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
 
set_cpu_sibling_map(0);
 
+   apic_intr_mode_init();
+
switch (smp_sanity_check(max_cpus)) {
case SMP_NO_CONFIG:
disable_smp();
-   if (APIC_init_uniprocessor())
-   pr_notice("Local APIC not detected. Using dummy APIC 
emulation.\n");
return;
case SMP_NO_APIC:
disable_smp();
return;
case SMP_FORCE_UP:
disable_smp();
-   apic_bsp_setup(false);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
@@ -1340,14 +1339,6 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
break;
}
 
-   if (read_apic_id() != boot_cpu_physical_apicid) {
-   panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
-read_apic_id(), boot_cpu_physical_apicid);
-   /* Or can we switch back to PIC here? */
-   }
-
-   default_setup_apic_routing();
-   apic_bsp_setup(false);
if (x2apic_mode)
cpu0_logical_apicid = apic_read(APIC_LDR);
else
-- 
2.5.5

[PATCH v5 06/12] x86/apic: Mark the apic_intr_mode extern for sanity check cleanup

[Dou Liyang]

Calling native_smp_prepare_cpus() to prepare for SMP bootup, does
some sanity checking, enables APIC mode and disables SMP feature.

Now, APIC mode setup has been unified to apic_intr_mode_init(),
some sanity checks are redundant and need to be cleanup.

Mark the apic_intr_mode extern to refine the switch and remove
the redundant sanity check.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  9 
 arch/x86/kernel/apic/apic.c | 11 +++---
 arch/x86/kernel/smpboot.c   | 50 -
 3 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index c3bedbd..bfbf715 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -53,6 +53,15 @@ extern int local_apic_timer_c2_ok;
 extern int disable_apic;
 extern unsigned int lapic_timer_frequency;
 
+extern enum apic_intr_mode_id apic_intr_mode;
+enum apic_intr_mode_id {
+   APIC_PIC,
+   APIC_VIRTUAL_WIRE,
+   APIC_VIRTUAL_WIRE_NO_CONFIG,
+   APIC_SYMMETRIC_IO,
+   APIC_SYMMETRIC_IO_NO_ROUTING
+};
+
 #ifdef CONFIG_SMP
 extern void __inquire_remote_apic(int apicid);
 #else /* CONFIG_SMP */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index bea8032..0601054 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1157,13 +1157,7 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
 }
 
-enum apic_intr_mode {
-   APIC_PIC,
-   APIC_VIRTUAL_WIRE,
-   APIC_VIRTUAL_WIRE_NO_CONFIG,
-   APIC_SYMMETRIC_IO,
-   APIC_SYMMETRIC_IO_NO_ROUTING,
-};
+enum apic_intr_mode_id apic_intr_mode;
 
 static int __init apic_intr_mode_select(int *upmode)
 {
@@ -1291,7 +1285,8 @@ void __init apic_intr_mode_init(void)
 {
int upmode = false;
 
-   switch (apic_intr_mode_select()) {
+   apic_intr_mode = apic_intr_mode_select();
+   switch (apic_intr_mode) {
case APIC_PIC:
apic_printk(APIC_VERBOSE, KERN_INFO
"Keep in PIC mode(8259)\n");
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b9b2a43..2db61dcb 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1193,7 +1193,7 @@ enum {
 /*
  * Various sanity checks.
  */
-static int __init smp_sanity_check(unsigned max_cpus)
+static void __init smp_sanity_check(void)
 {
preempt_disable();
 
@@ -1231,16 +1231,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
}
 
/*
-* If we couldn't find an SMP configuration at boot time,
-* get out of here now!
-*/
-   if (!smp_found_config && !acpi_lapic) {
-   preempt_enable();
-   pr_notice("SMP motherboard not detected\n");
-   return SMP_NO_CONFIG;
-   }
-
-   /*
 * Should not be necessary because the MP table should list the boot
 * CPU too, but we do it for the sake of robustness anyway.
 */
@@ -1250,29 +1240,6 @@ static int __init smp_sanity_check(unsigned max_cpus)
physid_set(hard_smp_processor_id(), phys_cpu_present_map);
}
preempt_enable();
-
-   /*
-* If we couldn't find a local APIC, then get out of here now!
-*/
-   if (APIC_INTEGRATED(boot_cpu_apic_version) &&
-   !boot_cpu_has(X86_FEATURE_APIC)) {
-   if (!disable_apic) {
-   pr_err("BIOS bug, local APIC #%d not detected!...\n",
-   boot_cpu_physical_apicid);
-   pr_err("... forcing use of dummy APIC emulation (tell 
your hw vendor)\n");
-   }
-   return SMP_NO_APIC;
-   }
-
-   /*
-* If SMP should be disabled, then really disable it!
-*/
-   if (!max_cpus) {
-   pr_info("SMP mode deactivated\n");
-   return SMP_FORCE_UP;
-   }
-
-   return SMP_OK;
 }
 
 static void __init smp_cpu_index_default(void)
@@ -1322,20 +1289,20 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
set_cpu_sibling_map(0);
 
apic_intr_mode_init();
+   smp_sanity_check();
 
-   switch (smp_sanity_check(max_cpus)) {
-   case SMP_NO_CONFIG:
-   disable_smp();
-   return;
-   case SMP_NO_APIC:
+   switch (apic_intr_mode) {
+   case APIC_PIC:
+   case APIC_VIRTUAL_WIRE_NO_CONFIG:
disable_smp();
return;
-   case SMP_FORCE_UP:
+   case APIC_SYMMETRIC_IO_NO_ROUTING:
disable_smp();
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
return;
-   case SMP_OK:
+   case APIC_VIRTUAL_WIRE:
+   case APIC_SYMMETRIC_IO:
break;
}
 
@@ -1343,6 +1310,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)

[PATCH v5 04/12] x86/apic: Move logical APIC ID away from apic_bsp_setup()

[Dou Liyang]

apic_bsp_setup() sets and returns logical APIC ID for initializing
cpu0_logical_apicid in SMP-capable system.

The id has nothing to do with the initialization of local APIC and
I/O APIC. And apic_bsp_setup() should be called for interrupt mode
setup intently.

Move the id setup to native_smp_prepare_cpus() for cleanup and mark
apic_bsp_setup() void.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  2 +-
 arch/x86/kernel/apic/apic.c | 10 +-
 arch/x86/kernel/smpboot.c   |  7 +--
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index ddc16ff..c3bedbd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -146,7 +146,7 @@ static inline int apic_force_enable(unsigned long addr)
 extern int apic_force_enable(unsigned long addr);
 #endif
 
-extern int apic_bsp_setup(bool upmode);
+extern void apic_bsp_setup(bool upmode);
 extern void apic_ap_setup(void);
 
 /*
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 63220ca..498edbe 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2341,25 +2341,17 @@ static void __init apic_bsp_up_setup(void)
  * Returns:
  * apic_id of BSP APIC
  */
-int __init apic_bsp_setup(bool upmode)
+void __init apic_bsp_setup(bool upmode)
 {
-   int id;
-
connect_bsp_APIC();
if (upmode)
apic_bsp_up_setup();
setup_local_APIC();
 
-   if (x2apic_mode)
-   id = apic_read(APIC_LDR);
-   else
-   id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-
enable_IO_APIC();
end_local_APIC_setup();
irq_remap_enable_fault_handling();
setup_IO_APIC();
-   return id;
 }
 
 /*
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 93f0cda..d6721f0 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1347,8 +1347,11 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
}
 
default_setup_apic_routing();
-   cpu0_logical_apicid = apic_bsp_setup(false);
-
+   apic_bsp_setup(false);
+   if (x2apic_mode)
+   cpu0_logical_apicid = apic_read(APIC_LDR);
+   else
+   cpu0_logical_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
 
-- 
2.5.5

[PATCH v5 10/12] x86/xen: Bypass intr mode setup in enlighten_pv system

[Dou Liyang]

xen_smp_ops overwrites smp_prepare_cpus to xen_pv_smp_prepare_cpus
which initializes interrupt itself.

Touching the intr_mode_init causes unexpected results on the system.

Bypass it in enlighten_pv system.

Signed-off-by: Dou Liyang 
---
 arch/x86/xen/enlighten_pv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index f33eef4..d3362a3 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1268,6 +1268,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
x86_platform.get_nmi_reason = xen_get_nmi_reason;
 
x86_init.resources.memory_setup = xen_memory_setup;
+   x86_init.irqs.intr_mode_init= x86_init_noop;
x86_init.oem.arch_setup = xen_arch_setup;
x86_init.oem.banner = xen_banner;
 
-- 
2.5.5

[PATCH v5 04/12] x86/apic: Move logical APIC ID away from apic_bsp_setup()

[Dou Liyang]

apic_bsp_setup() sets and returns logical APIC ID for initializing
cpu0_logical_apicid in SMP-capable system.

The id has nothing to do with the initialization of local APIC and
I/O APIC. And apic_bsp_setup() should be called for interrupt mode
setup intently.

Move the id setup to native_smp_prepare_cpus() for cleanup and mark
apic_bsp_setup() void.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  2 +-
 arch/x86/kernel/apic/apic.c | 10 +-
 arch/x86/kernel/smpboot.c   |  7 +--
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index ddc16ff..c3bedbd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -146,7 +146,7 @@ static inline int apic_force_enable(unsigned long addr)
 extern int apic_force_enable(unsigned long addr);
 #endif
 
-extern int apic_bsp_setup(bool upmode);
+extern void apic_bsp_setup(bool upmode);
 extern void apic_ap_setup(void);
 
 /*
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 63220ca..498edbe 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2341,25 +2341,17 @@ static void __init apic_bsp_up_setup(void)
  * Returns:
  * apic_id of BSP APIC
  */
-int __init apic_bsp_setup(bool upmode)
+void __init apic_bsp_setup(bool upmode)
 {
-   int id;
-
connect_bsp_APIC();
if (upmode)
apic_bsp_up_setup();
setup_local_APIC();
 
-   if (x2apic_mode)
-   id = apic_read(APIC_LDR);
-   else
-   id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
-
enable_IO_APIC();
end_local_APIC_setup();
irq_remap_enable_fault_handling();
setup_IO_APIC();
-   return id;
 }
 
 /*
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 93f0cda..d6721f0 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1347,8 +1347,11 @@ void __init native_smp_prepare_cpus(unsigned int 
max_cpus)
}
 
default_setup_apic_routing();
-   cpu0_logical_apicid = apic_bsp_setup(false);
-
+   apic_bsp_setup(false);
+   if (x2apic_mode)
+   cpu0_logical_apicid = apic_read(APIC_LDR);
+   else
+   cpu0_logical_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
 
-- 
2.5.5

[PATCH v5 10/12] x86/xen: Bypass intr mode setup in enlighten_pv system

[Dou Liyang]

xen_smp_ops overwrites smp_prepare_cpus to xen_pv_smp_prepare_cpus
which initializes interrupt itself.

Touching the intr_mode_init causes unexpected results on the system.

Bypass it in enlighten_pv system.

Signed-off-by: Dou Liyang 
---
 arch/x86/xen/enlighten_pv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index f33eef4..d3362a3 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1268,6 +1268,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
x86_platform.get_nmi_reason = xen_get_nmi_reason;
 
x86_init.resources.memory_setup = xen_memory_setup;
+   x86_init.irqs.intr_mode_init= x86_init_noop;
x86_init.oem.arch_setup = xen_arch_setup;
x86_init.oem.banner = xen_banner;
 
-- 
2.5.5

[PATCH v5 09/12] x86/init: add intr_mode_init to x86_init_ops

[Dou Liyang]

Add an unconditional x86_init_ops function which defaults to the
standard function and can be overridden by the early platform code.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/x86_init.h | 2 ++
 arch/x86/kernel/apic/apic.c | 2 +-
 arch/x86/kernel/smpboot.c   | 2 +-
 arch/x86/kernel/x86_init.c  | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 7ba7e90..f45acdf 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -50,11 +50,13 @@ struct x86_init_resources {
  * are set up.
  * @intr_init: interrupt init code
  * @trap_init: platform specific trap setup
+ * @intr_mode_init:interrupt delivery mode setup
  */
 struct x86_init_irqs {
void (*pre_vector_init)(void);
void (*intr_init)(void);
void (*trap_init)(void);
+   void (*intr_mode_init)(void);
 };
 
 /**
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 9bf7e95..51204d4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2387,7 +2387,7 @@ void __init apic_bsp_setup(bool upmode)
 #ifdef CONFIG_UP_LATE_INIT
 void __init up_late_init(void)
 {
-   apic_intr_mode_init();
+   x86_init.irqs.intr_mode_init();
 
if (apic_intr_mode == APIC_PIC)
return;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2db61dcb..80d25fe 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1288,7 +1288,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
set_cpu_sibling_map(0);
 
-   apic_intr_mode_init();
+   x86_init.irqs.intr_mode_init();
smp_sanity_check();
 
switch (apic_intr_mode) {
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index a088b2c..a7889b9 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -55,6 +55,7 @@ struct x86_init_ops x86_init __initdata = {
.pre_vector_init= init_ISA_irqs,
.intr_init  = native_init_IRQ,
.trap_init  = x86_init_noop,
+   .intr_mode_init = apic_intr_mode_init
},
 
.oem = {
-- 
2.5.5

[PATCH v5 09/12] x86/init: add intr_mode_init to x86_init_ops

[Dou Liyang]

Add an unconditional x86_init_ops function which defaults to the
standard function and can be overridden by the early platform code.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/x86_init.h | 2 ++
 arch/x86/kernel/apic/apic.c | 2 +-
 arch/x86/kernel/smpboot.c   | 2 +-
 arch/x86/kernel/x86_init.c  | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 7ba7e90..f45acdf 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -50,11 +50,13 @@ struct x86_init_resources {
  * are set up.
  * @intr_init: interrupt init code
  * @trap_init: platform specific trap setup
+ * @intr_mode_init:interrupt delivery mode setup
  */
 struct x86_init_irqs {
void (*pre_vector_init)(void);
void (*intr_init)(void);
void (*trap_init)(void);
+   void (*intr_mode_init)(void);
 };
 
 /**
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 9bf7e95..51204d4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2387,7 +2387,7 @@ void __init apic_bsp_setup(bool upmode)
 #ifdef CONFIG_UP_LATE_INIT
 void __init up_late_init(void)
 {
-   apic_intr_mode_init();
+   x86_init.irqs.intr_mode_init();
 
if (apic_intr_mode == APIC_PIC)
return;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2db61dcb..80d25fe 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1288,7 +1288,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
set_cpu_sibling_map(0);
 
-   apic_intr_mode_init();
+   x86_init.irqs.intr_mode_init();
smp_sanity_check();
 
switch (apic_intr_mode) {
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index a088b2c..a7889b9 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -55,6 +55,7 @@ struct x86_init_ops x86_init __initdata = {
.pre_vector_init= init_ISA_irqs,
.intr_init  = native_init_IRQ,
.trap_init  = x86_init_noop,
+   .intr_mode_init = apic_intr_mode_init
},
 
.oem = {
-- 
2.5.5

[PATCH v5 11/12] x86/time: Initialize interrupt mode behind timer init

[Dou Liyang]

In start_kernel(), firstly, it works on the default interrupy mode, then
switch to the final mode. Normally, Booting with BIOS reset is OK.

But, At dump-capture kernel, it boot up without BIOS reset, default mode
may not be compatible with the actual registers, that causes the delivery
interrupt to fail.

Try to set up the final mode as soon as possible. according to the parts
which split from that initialization:

1) Set up the APIC/IOAPIC (including testing whether the timer
   interrupt works)

2) Calibrate TSC

3) Set up the local APIC timer

-- From Thomas Gleixner

Initializing the mode should be earlier than calibrating TSC as soon as
possible and needs testing whether the timer interrupt works at the same
time.

call it behind timers init, which meets the above conditions.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 2 --
 arch/x86/kernel/smpboot.c   | 2 --
 arch/x86/kernel/time.c  | 5 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 51204d4..d4722c2 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2387,8 +2387,6 @@ void __init apic_bsp_setup(bool upmode)
 #ifdef CONFIG_UP_LATE_INIT
 void __init up_late_init(void)
 {
-   x86_init.irqs.intr_mode_init();
-
if (apic_intr_mode == APIC_PIC)
return;
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 80d25fe..d1185dd 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1287,8 +1287,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
set_sched_topology(x86_topology);
 
set_cpu_sibling_map(0);
-
-   x86_init.irqs.intr_mode_init();
smp_sanity_check();
 
switch (apic_intr_mode) {
diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c
index d39c091..0f04d4b 100644
--- a/arch/x86/kernel/time.c
+++ b/arch/x86/kernel/time.c
@@ -84,6 +84,11 @@ void __init hpet_time_init(void)
 static __init void x86_late_time_init(void)
 {
x86_init.timers.timer_init();
+   /*
+* After PIT/HPET timers init, select and setup
+* the final interrupt mode for delivering IRQs.
+*/
+   x86_init.irqs.intr_mode_init();
tsc_init();
 }
 
-- 
2.5.5

[PATCH v5 11/12] x86/time: Initialize interrupt mode behind timer init

[Dou Liyang]

In start_kernel(), firstly, it works on the default interrupy mode, then
switch to the final mode. Normally, Booting with BIOS reset is OK.

But, At dump-capture kernel, it boot up without BIOS reset, default mode
may not be compatible with the actual registers, that causes the delivery
interrupt to fail.

Try to set up the final mode as soon as possible. according to the parts
which split from that initialization:

1) Set up the APIC/IOAPIC (including testing whether the timer
   interrupt works)

2) Calibrate TSC

3) Set up the local APIC timer

-- From Thomas Gleixner

Initializing the mode should be earlier than calibrating TSC as soon as
possible and needs testing whether the timer interrupt works at the same
time.

call it behind timers init, which meets the above conditions.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 2 --
 arch/x86/kernel/smpboot.c   | 2 --
 arch/x86/kernel/time.c  | 5 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 51204d4..d4722c2 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2387,8 +2387,6 @@ void __init apic_bsp_setup(bool upmode)
 #ifdef CONFIG_UP_LATE_INIT
 void __init up_late_init(void)
 {
-   x86_init.irqs.intr_mode_init();
-
if (apic_intr_mode == APIC_PIC)
return;
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 80d25fe..d1185dd 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1287,8 +1287,6 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
set_sched_topology(x86_topology);
 
set_cpu_sibling_map(0);
-
-   x86_init.irqs.intr_mode_init();
smp_sanity_check();
 
switch (apic_intr_mode) {
diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c
index d39c091..0f04d4b 100644
--- a/arch/x86/kernel/time.c
+++ b/arch/x86/kernel/time.c
@@ -84,6 +84,11 @@ void __init hpet_time_init(void)
 static __init void x86_late_time_init(void)
 {
x86_init.timers.timer_init();
+   /*
+* After PIT/HPET timers init, select and setup
+* the final interrupt mode for delivering IRQs.
+*/
+   x86_init.irqs.intr_mode_init();
tsc_init();
 }
 
-- 
2.5.5

[PATCH v5 08/12] x86/ioapic: Refactor the delay logic in timer_irq_works()

[Dou Liyang]

Kernel use timer_irq_works() to detects the timer IRQs. It calls
mdelay(10) to delay ten ticks and check whether the timer IRQ work
or not. The mdelay() depends on the loops_per_jiffy which is set up
in calibrate_delay(). Current kernel defaults the IRQ 0 is available
when it calibrates delay.

But it is wrong in the dump-capture kernel with 'notsc' option inherited
from 1st kernel option. dump-capture kernel can't make sure the timer IRQ
works well.

The correct design is making the interrupt mode setup and checking timer
IRQ works in advance of calibrate_delay(). That results in the mdelay()
being unusable in timer_irq_works().

Preparatory patch to make the setup in advance. Refactor the delay logic
by waiting for some cycles. In the system with X86_FEATURE_TSC feature,
Use rdtsc(), others will call __delay() directly.

Note: regard 4G as the max CPU frequence of current single CPU.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/io_apic.c | 45 --
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 347bb9f..f710077 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1607,6 +1607,43 @@ static int __init notimercheck(char *s)
 }
 __setup("no_timer_check", notimercheck);
 
+static void __init delay_with_tsc(void)
+{
+   unsigned long long start, now;
+   unsigned long ticks = jiffies;
+
+   start = rdtsc();
+
+   /*
+* We don't know the TSC frequency yet, but waiting for
+* 400/HZ TSC cycles is safe:
+* 4 GHz == 10 jiffies
+* 1 GHz == 40 jiffies
+*/
+   do {
+   rep_nop();
+   now = rdtsc();
+   } while ((now - start) < 400UL / HZ &&
+   time_before_eq(jiffies, ticks + 4));
+}
+
+static void __init delay_without_tsc(void)
+{
+   int band = 1;
+   unsigned long ticks = jiffies;
+
+   /*
+* We don't know any frequency yet, but waiting for
+* 4094000/HZ cycles is safe:
+* 4 GHz == 10 jiffies
+* 1 GHz == 40 jiffies
+* 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094
+*/
+   do {
+   __delay(((1 << band++) * 1000UL) / HZ);
+   } while (band < 12 && time_before_eq(jiffies, ticks + 4));
+}
+
 /*
  * There is a nasty bug in some older SMP boards, their mptable lies
  * about the timer IRQ. We do the following to work around the situation:
@@ -1625,8 +1662,12 @@ static int __init timer_irq_works(void)
 
local_save_flags(flags);
local_irq_enable();
-   /* Let ten ticks pass... */
-   mdelay((10 * 1000) / HZ);
+
+   if (boot_cpu_has(X86_FEATURE_TSC))
+   delay_with_tsc();
+   else
+   delay_without_tsc();
+
local_irq_restore(flags);
 
/*
-- 
2.5.5

[PATCH v5 12/12] x86/apic: Remove the init_bsp_APIC()

[Dou Liyang]

The init_bsp_APIC() which works for the virtual wire mode is used
in ISA irq initialization at the booting time.

Currently, enable and setup the interrupt mode has been unified
and advanced just behind the timer IRQ setup. Kernel switches to
the final interrupt delivery mode directly. So init_bsp_APIC()
is redundant.

Remove the init_bsp_APIC() function.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  1 -
 arch/x86/kernel/apic/apic.c | 49 -
 arch/x86/kernel/irqinit.c   |  3 ---
 3 files changed, 53 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index b35bbbf..3c9fc9e 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -136,7 +136,6 @@ extern void disconnect_bsp_APIC(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
-extern void init_bsp_APIC(void);
 extern void apic_intr_mode_init(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d4722c2..47c94d5 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1235,55 +1235,6 @@ static int __init apic_intr_mode_select(int *upmode)
return APIC_SYMMETRIC_IO;
 }
 
-/*
- * An initial setup of the virtual wire mode.
- */
-void __init init_bsp_APIC(void)
-{
-   unsigned int value;
-
-   /*
-* Don't do the setup now if we have a SMP BIOS as the
-* through-I/O-APIC virtual wire mode might be active.
-*/
-   if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
-   return;
-
-   /*
-* Do not trust the local APIC being empty at bootup.
-*/
-   clear_local_APIC();
-
-   /*
-* Enable APIC.
-*/
-   value = apic_read(APIC_SPIV);
-   value &= ~APIC_VECTOR_MASK;
-   value |= APIC_SPIV_APIC_ENABLED;
-
-#ifdef CONFIG_X86_32
-   /* This bit is reserved on P4/Xeon and should be cleared */
-   if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
-   (boot_cpu_data.x86 == 15))
-   value &= ~APIC_SPIV_FOCUS_DISABLED;
-   else
-#endif
-   value |= APIC_SPIV_FOCUS_DISABLED;
-   value |= SPURIOUS_APIC_VECTOR;
-   apic_write(APIC_SPIV, value);
-
-   /*
-* Set up the virtual wire mode.
-*/
-   apic_write(APIC_LVT0, APIC_DM_EXTINT);
-   value = APIC_DM_NMI;
-   if (!lapic_is_integrated()) /* 82489DX */
-   value |= APIC_LVT_LEVEL_TRIGGER;
-   if (apic_extnmi == APIC_EXTNMI_NONE)
-   value |= APIC_LVT_MASKED;
-   apic_write(APIC_LVT1, value);
-}
-
 /* Init the interrupt delivery mode for the BSP */
 void __init apic_intr_mode_init(void)
 {
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 7468c69..488c9e2 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,9 +72,6 @@ void __init init_ISA_irqs(void)
struct irq_chip *chip = legacy_pic->chip;
int i;
 
-#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
-   init_bsp_APIC();
-#endif
legacy_pic->init(0);
 
for (i = 0; i < nr_legacy_irqs(); i++)
-- 
2.5.5

[PATCH v5 07/12] x86/apic: Unify interrupt mode setup for UP system

[Dou Liyang]

In UniProcessor kernel with UP_LATE_INIT=y, it enables and setups
interrupt delivery mode in up_late_init().

Unify it to apic_intr_mode_init(), remove APIC_init_uniprocessor().

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  1 -
 arch/x86/kernel/apic/apic.c | 49 +
 2 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bfbf715..b35bbbf 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -144,7 +144,6 @@ void register_lapic_address(unsigned long address);
 extern void setup_boot_APIC_clock(void);
 extern void setup_secondary_APIC_clock(void);
 extern void lapic_update_tsc_freq(void);
-extern int APIC_init_uniprocessor(void);
 
 #ifdef CONFIG_X86_64
 static inline int apic_force_enable(unsigned long addr)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 0601054..9bf7e95 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1198,6 +1198,10 @@ static int __init apic_intr_mode_select(int *upmode)
}
 #endif
 
+#ifdef CONFIG_UP_LATE_INIT
+   *upmode = true;
+#endif
+
/* Check MP table or ACPI MADT configuration */
if (!smp_found_config) {
disable_ioapic_support();
@@ -2380,51 +2384,16 @@ void __init apic_bsp_setup(bool upmode)
setup_IO_APIC();
 }
 
-/*
- * This initializes the IO-APIC and APIC hardware if this is
- * a UP kernel.
- */
-int __init APIC_init_uniprocessor(void)
+#ifdef CONFIG_UP_LATE_INIT
+void __init up_late_init(void)
 {
-   if (disable_apic) {
-   pr_info("Apic disabled\n");
-   return -1;
-   }
-#ifdef CONFIG_X86_64
-   if (!boot_cpu_has(X86_FEATURE_APIC)) {
-   disable_apic = 1;
-   pr_info("Apic disabled by BIOS\n");
-   return -1;
-   }
-#else
-   if (!smp_found_config && !boot_cpu_has(X86_FEATURE_APIC))
-   return -1;
-
-   /*
-* Complain if the BIOS pretends there is one.
-*/
-   if (!boot_cpu_has(X86_FEATURE_APIC) &&
-   APIC_INTEGRATED(boot_cpu_apic_version)) {
-   pr_err("BIOS bug, local APIC 0x%x not detected!...\n",
-   boot_cpu_physical_apicid);
-   return -1;
-   }
-#endif
+   apic_intr_mode_init();
 
-   if (!smp_found_config)
-   disable_ioapic_support();
+   if (apic_intr_mode == APIC_PIC)
+   return;
 
-   default_setup_apic_routing();
-   apic_bsp_setup(true);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
-   return 0;
-}
-
-#ifdef CONFIG_UP_LATE_INIT
-void __init up_late_init(void)
-{
-   APIC_init_uniprocessor();
 }
 #endif
 
-- 
2.5.5

[PATCH v5 08/12] x86/ioapic: Refactor the delay logic in timer_irq_works()

[Dou Liyang]

Kernel use timer_irq_works() to detects the timer IRQs. It calls
mdelay(10) to delay ten ticks and check whether the timer IRQ work
or not. The mdelay() depends on the loops_per_jiffy which is set up
in calibrate_delay(). Current kernel defaults the IRQ 0 is available
when it calibrates delay.

But it is wrong in the dump-capture kernel with 'notsc' option inherited
from 1st kernel option. dump-capture kernel can't make sure the timer IRQ
works well.

The correct design is making the interrupt mode setup and checking timer
IRQ works in advance of calibrate_delay(). That results in the mdelay()
being unusable in timer_irq_works().

Preparatory patch to make the setup in advance. Refactor the delay logic
by waiting for some cycles. In the system with X86_FEATURE_TSC feature,
Use rdtsc(), others will call __delay() directly.

Note: regard 4G as the max CPU frequence of current single CPU.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/io_apic.c | 45 --
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 347bb9f..f710077 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1607,6 +1607,43 @@ static int __init notimercheck(char *s)
 }
 __setup("no_timer_check", notimercheck);
 
+static void __init delay_with_tsc(void)
+{
+   unsigned long long start, now;
+   unsigned long ticks = jiffies;
+
+   start = rdtsc();
+
+   /*
+* We don't know the TSC frequency yet, but waiting for
+* 400/HZ TSC cycles is safe:
+* 4 GHz == 10 jiffies
+* 1 GHz == 40 jiffies
+*/
+   do {
+   rep_nop();
+   now = rdtsc();
+   } while ((now - start) < 400UL / HZ &&
+   time_before_eq(jiffies, ticks + 4));
+}
+
+static void __init delay_without_tsc(void)
+{
+   int band = 1;
+   unsigned long ticks = jiffies;
+
+   /*
+* We don't know any frequency yet, but waiting for
+* 4094000/HZ cycles is safe:
+* 4 GHz == 10 jiffies
+* 1 GHz == 40 jiffies
+* 1 << 1 + 1 << 2 +...+ 1 << 11 = 4094
+*/
+   do {
+   __delay(((1 << band++) * 1000UL) / HZ);
+   } while (band < 12 && time_before_eq(jiffies, ticks + 4));
+}
+
 /*
  * There is a nasty bug in some older SMP boards, their mptable lies
  * about the timer IRQ. We do the following to work around the situation:
@@ -1625,8 +1662,12 @@ static int __init timer_irq_works(void)
 
local_save_flags(flags);
local_irq_enable();
-   /* Let ten ticks pass... */
-   mdelay((10 * 1000) / HZ);
+
+   if (boot_cpu_has(X86_FEATURE_TSC))
+   delay_with_tsc();
+   else
+   delay_without_tsc();
+
local_irq_restore(flags);
 
/*
-- 
2.5.5

[PATCH v5 12/12] x86/apic: Remove the init_bsp_APIC()

[Dou Liyang]

The init_bsp_APIC() which works for the virtual wire mode is used
in ISA irq initialization at the booting time.

Currently, enable and setup the interrupt mode has been unified
and advanced just behind the timer IRQ setup. Kernel switches to
the final interrupt delivery mode directly. So init_bsp_APIC()
is redundant.

Remove the init_bsp_APIC() function.

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  1 -
 arch/x86/kernel/apic/apic.c | 49 -
 arch/x86/kernel/irqinit.c   |  3 ---
 3 files changed, 53 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index b35bbbf..3c9fc9e 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -136,7 +136,6 @@ extern void disconnect_bsp_APIC(int virt_wire_setup);
 extern void disable_local_APIC(void);
 extern void lapic_shutdown(void);
 extern void sync_Arb_IDs(void);
-extern void init_bsp_APIC(void);
 extern void apic_intr_mode_init(void);
 extern void setup_local_APIC(void);
 extern void init_apic_mappings(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d4722c2..47c94d5 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1235,55 +1235,6 @@ static int __init apic_intr_mode_select(int *upmode)
return APIC_SYMMETRIC_IO;
 }
 
-/*
- * An initial setup of the virtual wire mode.
- */
-void __init init_bsp_APIC(void)
-{
-   unsigned int value;
-
-   /*
-* Don't do the setup now if we have a SMP BIOS as the
-* through-I/O-APIC virtual wire mode might be active.
-*/
-   if (smp_found_config || !boot_cpu_has(X86_FEATURE_APIC))
-   return;
-
-   /*
-* Do not trust the local APIC being empty at bootup.
-*/
-   clear_local_APIC();
-
-   /*
-* Enable APIC.
-*/
-   value = apic_read(APIC_SPIV);
-   value &= ~APIC_VECTOR_MASK;
-   value |= APIC_SPIV_APIC_ENABLED;
-
-#ifdef CONFIG_X86_32
-   /* This bit is reserved on P4/Xeon and should be cleared */
-   if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
-   (boot_cpu_data.x86 == 15))
-   value &= ~APIC_SPIV_FOCUS_DISABLED;
-   else
-#endif
-   value |= APIC_SPIV_FOCUS_DISABLED;
-   value |= SPURIOUS_APIC_VECTOR;
-   apic_write(APIC_SPIV, value);
-
-   /*
-* Set up the virtual wire mode.
-*/
-   apic_write(APIC_LVT0, APIC_DM_EXTINT);
-   value = APIC_DM_NMI;
-   if (!lapic_is_integrated()) /* 82489DX */
-   value |= APIC_LVT_LEVEL_TRIGGER;
-   if (apic_extnmi == APIC_EXTNMI_NONE)
-   value |= APIC_LVT_MASKED;
-   apic_write(APIC_LVT1, value);
-}
-
 /* Init the interrupt delivery mode for the BSP */
 void __init apic_intr_mode_init(void)
 {
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 7468c69..488c9e2 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -72,9 +72,6 @@ void __init init_ISA_irqs(void)
struct irq_chip *chip = legacy_pic->chip;
int i;
 
-#if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
-   init_bsp_APIC();
-#endif
legacy_pic->init(0);
 
for (i = 0; i < nr_legacy_irqs(); i++)
-- 
2.5.5

[PATCH v5 07/12] x86/apic: Unify interrupt mode setup for UP system

[Dou Liyang]

In UniProcessor kernel with UP_LATE_INIT=y, it enables and setups
interrupt delivery mode in up_late_init().

Unify it to apic_intr_mode_init(), remove APIC_init_uniprocessor().

Signed-off-by: Dou Liyang 
---
 arch/x86/include/asm/apic.h |  1 -
 arch/x86/kernel/apic/apic.c | 49 +
 2 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index bfbf715..b35bbbf 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -144,7 +144,6 @@ void register_lapic_address(unsigned long address);
 extern void setup_boot_APIC_clock(void);
 extern void setup_secondary_APIC_clock(void);
 extern void lapic_update_tsc_freq(void);
-extern int APIC_init_uniprocessor(void);
 
 #ifdef CONFIG_X86_64
 static inline int apic_force_enable(unsigned long addr)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 0601054..9bf7e95 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1198,6 +1198,10 @@ static int __init apic_intr_mode_select(int *upmode)
}
 #endif
 
+#ifdef CONFIG_UP_LATE_INIT
+   *upmode = true;
+#endif
+
/* Check MP table or ACPI MADT configuration */
if (!smp_found_config) {
disable_ioapic_support();
@@ -2380,51 +2384,16 @@ void __init apic_bsp_setup(bool upmode)
setup_IO_APIC();
 }
 
-/*
- * This initializes the IO-APIC and APIC hardware if this is
- * a UP kernel.
- */
-int __init APIC_init_uniprocessor(void)
+#ifdef CONFIG_UP_LATE_INIT
+void __init up_late_init(void)
 {
-   if (disable_apic) {
-   pr_info("Apic disabled\n");
-   return -1;
-   }
-#ifdef CONFIG_X86_64
-   if (!boot_cpu_has(X86_FEATURE_APIC)) {
-   disable_apic = 1;
-   pr_info("Apic disabled by BIOS\n");
-   return -1;
-   }
-#else
-   if (!smp_found_config && !boot_cpu_has(X86_FEATURE_APIC))
-   return -1;
-
-   /*
-* Complain if the BIOS pretends there is one.
-*/
-   if (!boot_cpu_has(X86_FEATURE_APIC) &&
-   APIC_INTEGRATED(boot_cpu_apic_version)) {
-   pr_err("BIOS bug, local APIC 0x%x not detected!...\n",
-   boot_cpu_physical_apicid);
-   return -1;
-   }
-#endif
+   apic_intr_mode_init();
 
-   if (!smp_found_config)
-   disable_ioapic_support();
+   if (apic_intr_mode == APIC_PIC)
+   return;
 
-   default_setup_apic_routing();
-   apic_bsp_setup(true);
/* Setup local timer */
x86_init.timers.setup_percpu_clockev();
-   return 0;
-}
-
-#ifdef CONFIG_UP_LATE_INIT
-void __init up_late_init(void)
-{
-   APIC_init_uniprocessor();
 }
 #endif
 
-- 
2.5.5

[PATCH v5 03/12] x86/apic: Split local APIC timer setup from the APIC setup

[Dou Liyang]

apic_bsp_setup() sets up the local APIC, I/O APIC and APIC timer.

The local APIC and I/O APIC setup belongs to interrupt delivery mode
setup. Setting up the local APIC timer for booting CPU is another job
and has nothing to do with interrupt delivery mode setup.

Split local APIC timer setup from the APIC setup, keep it in the
original position for SMP and UP kernel for preparation.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 4 ++--
 arch/x86/kernel/smpboot.c   | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 00d97f4..63220ca 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2359,8 +2359,6 @@ int __init apic_bsp_setup(bool upmode)
end_local_APIC_setup();
irq_remap_enable_fault_handling();
setup_IO_APIC();
-   /* Setup local timer */
-   x86_init.timers.setup_percpu_clockev();
return id;
 }
 
@@ -2400,6 +2398,8 @@ int __init APIC_init_uniprocessor(void)
 
default_setup_apic_routing();
apic_bsp_setup(true);
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
return 0;
 }
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f04479a..93f0cda 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1333,6 +1333,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
case SMP_FORCE_UP:
disable_smp();
apic_bsp_setup(false);
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
return;
case SMP_OK:
break;
@@ -1347,6 +1349,9 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
default_setup_apic_routing();
cpu0_logical_apicid = apic_bsp_setup(false);
 
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
+
pr_info("CPU0: ");
print_cpu_info(_data(0));
 
-- 
2.5.5

[PATCH v5 03/12] x86/apic: Split local APIC timer setup from the APIC setup

[Dou Liyang]

apic_bsp_setup() sets up the local APIC, I/O APIC and APIC timer.

The local APIC and I/O APIC setup belongs to interrupt delivery mode
setup. Setting up the local APIC timer for booting CPU is another job
and has nothing to do with interrupt delivery mode setup.

Split local APIC timer setup from the APIC setup, keep it in the
original position for SMP and UP kernel for preparation.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 4 ++--
 arch/x86/kernel/smpboot.c   | 5 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 00d97f4..63220ca 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2359,8 +2359,6 @@ int __init apic_bsp_setup(bool upmode)
end_local_APIC_setup();
irq_remap_enable_fault_handling();
setup_IO_APIC();
-   /* Setup local timer */
-   x86_init.timers.setup_percpu_clockev();
return id;
 }
 
@@ -2400,6 +2398,8 @@ int __init APIC_init_uniprocessor(void)
 
default_setup_apic_routing();
apic_bsp_setup(true);
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
return 0;
 }
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index f04479a..93f0cda 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1333,6 +1333,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
case SMP_FORCE_UP:
disable_smp();
apic_bsp_setup(false);
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
return;
case SMP_OK:
break;
@@ -1347,6 +1349,9 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
default_setup_apic_routing();
cpu0_logical_apicid = apic_bsp_setup(false);
 
+   /* Setup local timer */
+   x86_init.timers.setup_percpu_clockev();
+
pr_info("CPU0: ");
print_cpu_info(_data(0));
 
-- 
2.5.5

[PATCH v5 01/12] x86/apic: Construct a selector for the interrupt delivery mode

[Dou Liyang]

Now, there are many switches in kernel which are used to determine
the final interrupt delivery mode, as shown below:

1) kconfig:
   CONFIG_X86_64; CONFIG_X86_LOCAL_APIC; CONFIG_x86_IO_APIC
2) kernel option: disable_apic; skip_ioapic_setup
3) CPU Capability: boot_cpu_has(X86_FEATURE_APIC)
4) MP table: smp_found_config
5) ACPI: acpi_lapic; acpi_ioapic; nr_ioapic

These switches are disordered and scattered and there are also some
dependencies with each other. These make the code difficult to
maintain and read.

Construct a selector to unify them into a single function, then,
Use this selector to get an interrupt delivery mode directly.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 60 +
 1 file changed, 60 insertions(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2d75faf..88be65e 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1157,6 +1157,66 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
 }
 
+enum apic_intr_mode {
+   APIC_PIC,
+   APIC_VIRTUAL_WIRE,
+   APIC_SYMMETRIC_IO,
+};
+
+static int __init apic_intr_mode_select(void)
+{
+   /* Check kernel option */
+   if (disable_apic) {
+   pr_info("APIC disabled via kernel command line\n");
+   return APIC_PIC;
+   }
+
+   /* Check BIOS */
+#ifdef CONFIG_X86_64
+   /* On 64-bit, the APIC must be integrated, Check local APIC only */
+   if (!boot_cpu_has(X86_FEATURE_APIC)) {
+   disable_apic = 1;
+   pr_info("APIC disabled by BIOS\n");
+   return APIC_PIC;
+   }
+#else
+   /*
+* On 32-bit, check whether there is a separate chip or integrated
+* APIC
+*/
+
+   /* Has a local APIC ? */
+   if (!boot_cpu_has(X86_FEATURE_APIC) &&
+   APIC_INTEGRATED(boot_cpu_apic_version)) {
+   disable_apic = 1;
+   pr_err(FW_BUG "Local APIC %d not detected, force emulation\n",
+  boot_cpu_physical_apicid);
+   return APIC_PIC;
+   }
+
+   /* Has a separate chip ? */
+   if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
+   disable_apic = 1;
+
+   return APIC_PIC;
+   }
+#endif
+
+   /* Check MP table or ACPI MADT configuration */
+   if (!smp_found_config) {
+   disable_ioapic_support();
+
+   if (!acpi_lapic)
+   pr_info("APIC: ACPI MADT or MP tables are not 
detected\n");
+
+   return APIC_VIRTUAL_WIRE;
+   }
+
+   /* Other checks of APIC options will be done in each setup function */
+
+   return APIC_SYMMETRIC_IO;
+}
+
 /*
  * An initial setup of the virtual wire mode.
  */
-- 
2.5.5

[PATCH v5 01/12] x86/apic: Construct a selector for the interrupt delivery mode

[Dou Liyang]

Now, there are many switches in kernel which are used to determine
the final interrupt delivery mode, as shown below:

1) kconfig:
   CONFIG_X86_64; CONFIG_X86_LOCAL_APIC; CONFIG_x86_IO_APIC
2) kernel option: disable_apic; skip_ioapic_setup
3) CPU Capability: boot_cpu_has(X86_FEATURE_APIC)
4) MP table: smp_found_config
5) ACPI: acpi_lapic; acpi_ioapic; nr_ioapic

These switches are disordered and scattered and there are also some
dependencies with each other. These make the code difficult to
maintain and read.

Construct a selector to unify them into a single function, then,
Use this selector to get an interrupt delivery mode directly.

Signed-off-by: Dou Liyang 
---
 arch/x86/kernel/apic/apic.c | 60 +
 1 file changed, 60 insertions(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2d75faf..88be65e 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1157,6 +1157,66 @@ void __init sync_Arb_IDs(void)
APIC_INT_LEVELTRIG | APIC_DM_INIT);
 }
 
+enum apic_intr_mode {
+   APIC_PIC,
+   APIC_VIRTUAL_WIRE,
+   APIC_SYMMETRIC_IO,
+};
+
+static int __init apic_intr_mode_select(void)
+{
+   /* Check kernel option */
+   if (disable_apic) {
+   pr_info("APIC disabled via kernel command line\n");
+   return APIC_PIC;
+   }
+
+   /* Check BIOS */
+#ifdef CONFIG_X86_64
+   /* On 64-bit, the APIC must be integrated, Check local APIC only */
+   if (!boot_cpu_has(X86_FEATURE_APIC)) {
+   disable_apic = 1;
+   pr_info("APIC disabled by BIOS\n");
+   return APIC_PIC;
+   }
+#else
+   /*
+* On 32-bit, check whether there is a separate chip or integrated
+* APIC
+*/
+
+   /* Has a local APIC ? */
+   if (!boot_cpu_has(X86_FEATURE_APIC) &&
+   APIC_INTEGRATED(boot_cpu_apic_version)) {
+   disable_apic = 1;
+   pr_err(FW_BUG "Local APIC %d not detected, force emulation\n",
+  boot_cpu_physical_apicid);
+   return APIC_PIC;
+   }
+
+   /* Has a separate chip ? */
+   if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config) {
+   disable_apic = 1;
+
+   return APIC_PIC;
+   }
+#endif
+
+   /* Check MP table or ACPI MADT configuration */
+   if (!smp_found_config) {
+   disable_ioapic_support();
+
+   if (!acpi_lapic)
+   pr_info("APIC: ACPI MADT or MP tables are not 
detected\n");
+
+   return APIC_VIRTUAL_WIRE;
+   }
+
+   /* Other checks of APIC options will be done in each setup function */
+
+   return APIC_SYMMETRIC_IO;
+}
+
 /*
  * An initial setup of the virtual wire mode.
  */
-- 
2.5.5