Re: PowerPC agpmode issues

[Mathieu Malaterre]

On Thu, Aug 25, 2016 at 5:09 AM, Mike  wrote:
> Any improvement on your ends? Seems -1 is now the quirk. But does your
> trackpads work? Did an update after getting a new and the latest released
> powerbook up. Also found an interesting interface which can replace our ide
> drives, intended for ipod classics, but it can fit in the bay and has msata
> interface.

You should subscribe to this bug:

https://bugs.freedesktop.org/show_bug.cgi?id=94757

that's the only possible solution (when it comes out) AFAIK.

Re: PowerPC agpmode issues

[Mike]

Any improvement on your ends? Seems -1 is now the quirk. But does your
trackpads work? Did an update after getting a new and the latest released
powerbook up. Also found an interesting interface which can replace our ide
drives, intended for ipod classics, but it can fit in the bay and has msata
interface.

On 5 Feb 2016 15:32, "Herminio Hernandez Jr." <
herminio.hernande...@gmail.com> wrote:

> I have been experiencing the same thing with my iBook and PowerBook.
>
> Sent from my iPhone
>
> On Feb 4, 2016, at 8:47 PM, Mike  wrote:
>
> Hi.
> Managed to get the Radeon R300 running on mesa 11.1.1 with an old 2013
> patch from Michel Dànzer, next problem is of course enabling agpmode,
> running with pci-mode with radeon.agpmode=-1 works, but is of course slow,
> and seems to load the cpu a lot.
>
> Upon initial investigation i could not initially believe agp could be this
> this broken for this long, until i found this.
>  "committed with Ben Skeggs on Feb 26, 2013"
> https://github.com/DespairFactor/bullhead/commit/
> 650e1203c11354ba84d69ba445abc0efcfe3890a
> http://lxr.free-electrons.com/source/drivers/gpu/drm/
> nouveau/nouveau_agp.c?v=4.2
> #ifdef __powerpc__
> /* Disable AGP by default on all PowerPC machines for
> * now -- At least some UniNorth-2 AGP bridges are
> * known to be broken: DMA from the host to the card
> * works just fine, but writeback from the card to the
> * host goes straight to memory untranslated bypassing
> * the GATT somehow, making them quite painful to deal
> * with...
> */
> if (nouveau_agpmode == -1)
> return false;
> #endif
>
>  and now later this:
> https://github.com/torvalds/linux/blob/master/drivers/gpu/
> drm/nouveau/nvkm/subdev/pci/agp.c
> #ifdef __powerpc__
> /* Disable AGP by default on all PowerPC machines for now -- At
> * least some UniNorth-2 AGP bridges are known to be broken:
> * DMA from the host to the card works just fine, but writeback
> * from the card to the host goes straight to memory
> * untranslated bypassing that GATT somehow, making them quite
> * painful to deal with...
> */
> mode = 0;
> #endif
>
> All seems to point to serious issues had around the time of change to ums
> to kms and a serious regression hitting the linux kernel? No?
>
> Cheers
> -Mike
>
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>

Re: [PATCH] powerpc/ptrace: Fix cppcheck issue in gpr32_set_common/gpr32_get_common.

[Daniel Axtens]

Simon Guo  writes:

> I think keeping tm_active argument for "ifndef CONFIG_PPC_TRANSACTIONAL_MEM" 
> case (with __maybe_unused prefix) will be somehow strange -- Whatever
> value is provided in the caller function for tm_active, programmer might be 
> puzzled and cost sometime to think about it.  I don't like to use 
> "__maybe_unused" to bypass this warning.

Fair enough. I don't have strong feelings either way - we'll see if the
maintainers have any thoughts.

Regards,
Daniel


signature.asc
Description: PGP signature

[RFC PATCH] powerpc: fsl_pci: fix inbound ATMU entries for systems with >4G RAM

[Tillmann Heidsieck]

For systems with >4G of RAM, the current implementation adds a second
inbound PCIe window starting at 128G this leaves all memory from 4G to
128G inaccessible to inbound PCIe transactions. The according errors
can be observed by using the EDAC driver for MPC85XX.

This patch changes this behaviour by adding the second window starting
at 4G. The current implementation still leaves memory beyond 68G
unmapped as this would require yet another ATMU entry.

Tested on a T4240 with 12G of RAM and an AMD E6760 PCIe card working with
the in-tree radeon driver.

Signed-off-by: Tillmann Heidsieck 
---
 arch/powerpc/sysdev/fsl_pci.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 0ef9df49f0f2..260983037904 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -349,17 +349,13 @@ static void setup_pci_atmu(struct pci_controller *hose)
}
 
sz = min(mem, paddr_lo);
-   mem_log = ilog2(sz);
+   mem_log = order_base_2(sz);
 
/* PCIe can overmap inbound & outbound since RX & TX are separated */
if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
-   /* Size window to exact size if power-of-two or one size up */
-   if ((1ull << mem_log) != mem) {
-   mem_log++;
-   if ((1ull << mem_log) > mem)
-   pr_info("%s: Setting PCI inbound window "
-   "greater than memory size\n", name);
-   }
+   if ((1ull << mem_log) > mem)
+   pr_info("%s: Setting PCI inbound window greater than 
memory size\n",
+   name);
 
piwar |= ((mem_log - 1) & PIWAR_SZ_MASK);
 
@@ -379,23 +375,27 @@ static void setup_pci_atmu(struct pci_controller *hose)
 * let devices that are 64-bit address capable to work w/o
 * SWIOTLB and access the full range of memory
 */
-   if (sz != mem) {
-   mem_log = ilog2(mem);
-
-   /* Size window up if we dont fit in exact power-of-2 */
-   if ((1ull << mem_log) != mem)
-   mem_log++;
-
+   if (mem > 0x1ULL) {
+   /* ok we mapped 4G in WIN1, now lets see how much memory
+* is left un-mapped and calculate the log, also
+* make sure we dont have a window lager then 64G
+*/
+   mem_log = order_base_2(min(mem - sz, 1ULL << 36));
piwar = (piwar & ~PIWAR_SZ_MASK) | (mem_log - 1);
 
if (setup_inbound) {
-   /* Setup inbound memory window */
-   out_be32(>piw[win_idx].pitar,  0x);
+   /* Setup inbound memory window
+* The windows starts at 4G and spans all the
+* remaining memory aka (mem - 4G)
+*/
+   out_be32(>piw[win_idx].pitar,
+0x);
out_be32(>piw[win_idx].piwbear,
-   pci64_dma_offset >> 44);
+0x);
out_be32(>piw[win_idx].piwbar,
-   pci64_dma_offset >> 12);
-   out_be32(>piw[win_idx].piwar,  piwar);
+0x1ULL >> 12);
+   out_be32(>piw[win_idx].piwar,
+piwar);
}
 
/*
-- 
2.9.3

Re: [RFC PATCH] powerpc: fsl_pci: fix inbound ATMU entries for systems with >4G RAM

[Scott Wood]

On 08/24/2016 02:48 PM, Tillmann Heidsieck wrote:
> For systems with >4G of RAM, the current implementation adds a second
> inbound PCIe window starting at 128G this leaves all memory from 4G to
> 128G inaccessible to inbound PCIe transactions.

The second inbound window is at 256G, and it maps all of RAM.  Note that
for accesses in this window, the PCI address is different from the host
address.

> The according errors can be observed by using the EDAC driver for MPC85XX.
> 
> This patch changes this behaviour by adding the second window starting
> at 4G. The current implementation still leaves memory beyond 68G
> unmapped as this would require yet another ATMU entry.

Windows must be size-aligned, as per the description of PEXIWBARn[WBA].
 This window is illegal.

By trying to identity-map everything you also break the assumption that
we don't need swiotlb to avoid the PEXCSRBAR region on PCI devices
capable of generating 64-bit addresses.

> Tested on a T4240 with 12G of RAM and an AMD E6760 PCIe card working with
> the in-tree radeon driver.

I have no problem using an e1000e card on a t4240 with 24 GiB RAM.

Looking at the radeon driver I see that there's a possibility of a dma
mask that is exactly 40 bits.  I think there's an off-by-one error in
fsl_pci_dma_set_mask().  If you change "dma_mask >=
DMA_BIT_MASK(MAX_PHYS_ADDR_BITS)" to "dma_mask >
DMA_BIT_MASK(MAX_PHYS_ADDR_BITS)", does that make radeon work without
this patch?


> Signed-off-by: Tillmann Heidsieck 
> ---
>  arch/powerpc/sysdev/fsl_pci.c | 40 
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 0ef9df49f0f2..260983037904 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -349,17 +349,13 @@ static void setup_pci_atmu(struct pci_controller *hose)
>   }
>  
>   sz = min(mem, paddr_lo);
> - mem_log = ilog2(sz);
> + mem_log = order_base_2(sz);
>  
>   /* PCIe can overmap inbound & outbound since RX & TX are separated */
>   if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
> - /* Size window to exact size if power-of-two or one size up */
> - if ((1ull << mem_log) != mem) {
> - mem_log++;
> - if ((1ull << mem_log) > mem)
> - pr_info("%s: Setting PCI inbound window "
> - "greater than memory size\n", name);
> - }
> + if ((1ull << mem_log) > mem)
> + pr_info("%s: Setting PCI inbound window greater than 
> memory size\n",
> + name);
>  
>   piwar |= ((mem_log - 1) & PIWAR_SZ_MASK);
>  

This change is unrelated.

BTW, for some reason your patch is not showing up in Patchwork.

-Scott

Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe

[Luis R. Rodriguez]

On Wed, Aug 24, 2016 at 08:55:55AM +0200, Daniel Vetter wrote:
> On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez  wrote:
> > Thou shalt not make firmware calls early on init or probe.

<-- snip -->

> > There are 4 offenders at this time:
> >
> > mcgrof@ergon ~/linux-next (git::20160609)$ export 
> > COCCI=scripts/coccinelle/api/request_firmware.cocci
> > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report
> >
> > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its 
> > init routine on line 321.
> > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on 
> > its probe routine on line 136.
> > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its 
> > probe routine on line 796.
> > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on 
> > its probe routine on line 1246.
> 
> Plus all gpu drivers which need firmware. And yes we must load them at
> probe

Do you have an upstream driver in mind that does this ? Is it on device
drier module probe or a DRM subsystem specific probe call of some sort ?

> because people are generally pissed when they boot their machine
> and the screen goes black. On top of that a lot of people want their
> gpu drivers to be built-in, but can't ship the firmware blobs in the
> kernel image because gpl. Yep, there's a bit a contradiction there ...

Can they use initramfs for this ?

Also just curious -- as with other subsystems, is it possible to load
a generic driver first, say vesa, and then a more enhanced one later ?
I am not saying this is ideal or am I suggesting this, I'd just like
to know the feasibility of this.

> I think what would work is loading the different subsystems of the
> driver in parallel (we already do that largely)

Init level stuff is actually pretty synchronous, and in fact both
init and probe are called serially. There are a few subsystems which
have been doing things a bit differently, but these are exceptions.

When you say we already do this largely, can you describe a bit more
precisely what you mean ?

>, and then if one
> firmware blob isn't there yet simply stall that async worker until it
> shows up.

Is this an existing framework or do you mean if we add something
generic to do this async loading of subsystems ?

> But the answers I've gotten thus far from request_firmware()
> folks (well at least Greg) is don't do that ...

Well in this patch set I'm adding myself as a MAINTAINER and I've
been extending the firmware API recently to help with a few new
features I want, I've been wanting to hear more feedback from
other's needs and I had actually not gotten much -- except
only recently with the usermode helper and reasons why some
folks thought they could not use direct firmware loading from
the fs. I'm keen to hear or more use cases and needs specially if
they have to do with improving boot time and asynchronous boot.

> Is that problem still somewhere on the radar? 

Not mine.

> Atm there's various
> wait_for_rootfs hacks out there floating in vendor/product trees.

This one I've heard about recently, and I suggested two possible
solutions, with a preference to a simply notification of when
rootfs is available from userspace.

> "Avoid at all costs" sounds like upstream prefers to not care about
> android/cros in those case (yes I know most arm socs don't need
> firmware, which would make it a problem fo just be a subset of all
> devices).

In my days of dealing with Android I learned most folks did not frankly
care too much about upstream-first model. That means things were reactive.
That's a business mind set and that's fine. However for upstream we want
what is best and to discuss. I haven't seen proposals so, so long as
we just hear of "hacks" that some folks do in vendor/product trees,
what can we do ?

In so far as async probe is concerned -- that is now upstream.

http://www.do-not-panic.com/2015/12/linux-asynchronous-probe.html

In so far as modules are concerned -- this should work without issue now, and
if there is an issue its very likely a bug in the subsystem.  As I noted in the
post, built-in support requires more love. A simple option for you to test this
is to test the two debug patches at the end there and boot. Alternatively inits
can just peg the async request for all modules. Should be an easy change, just
hadn't had a change to do it yet. Maybe its time.

I'm also trying to make more async functionality possible early in boot with
dependencies annotated somehow, and have a bit of work to help with this (refer
to recent linker tables patches) already which may even be possible to used to
facelift our old kernel init levels -- but as I've studied this I've also
observed others working on very similar problems, nothing is quite taking a
large picture of this and trying to generalize this. Its why I proposed it as a
topic for KS.

So .. I agree, let's avoid the hacks. Patches welcomed.

  Luis

[PATCH] powerpc/powernv/pci: Use kmalloc_array() in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 24 Aug 2016 22:26:37 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus reuse the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index fd9444f..2366552 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1305,7 +1305,9 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, 
u16 num_vfs)
else
m64_bars = 1;
 
-   pdn->m64_map = kmalloc(sizeof(*pdn->m64_map) * m64_bars, GFP_KERNEL);
+   pdn->m64_map = kmalloc_array(m64_bars,
+sizeof(*pdn->m64_map),
+GFP_KERNEL);
if (!pdn->m64_map)
return -ENOMEM;
/* Initialize the m64_map to IODA_INVALID_M64 */
@@ -1572,8 +1574,9 @@ int pnv_pci_sriov_enable(struct pci_dev *pdev, u16 
num_vfs)
 
/* Allocating pe_num_map */
if (pdn->m64_single_mode)
-   pdn->pe_num_map = kmalloc(sizeof(*pdn->pe_num_map) * 
num_vfs,
-   GFP_KERNEL);
+   pdn->pe_num_map = kmalloc_array(num_vfs,
+   
sizeof(*pdn->pe_num_map),
+   GFP_KERNEL);
else
pdn->pe_num_map = kmalloc(sizeof(*pdn->pe_num_map), 
GFP_KERNEL);
 
-- 
2.9.3

Re: [PATCH] ALSA: snd-aoa: enable sound on PowerBook G4 12"

[Aaro Koskinen]

Hi,

On Wed, Aug 24, 2016 at 09:43:23PM +0200, Johannes Berg wrote:
> On Wed, 2016-08-24 at 20:57 +0300, Aaro Koskinen wrote:
> > Enable sound on PowerBook G4 12".
> 
> Looks good to me, I assume you tested it and it works :)

Yes, I have this laptop in use.

A.

Re: [PATCH] ALSA: snd-aoa: enable sound on PowerBook G4 12"

[Johannes Berg]

On Wed, 2016-08-24 at 20:57 +0300, Aaro Koskinen wrote:
> Enable sound on PowerBook G4 12".

Looks good to me, I assume you tested it and it works :)

johannes

> Signed-off-by: Aaro Koskinen 
> ---
>  sound/aoa/fabrics/layout.c   | 8 
>  sound/aoa/soundbus/i2sbus/core.c | 2 +-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
> index 8f71f7e..edc8681 100644
> --- a/sound/aoa/fabrics/layout.c
> +++ b/sound/aoa/fabrics/layout.c
> @@ -112,6 +112,7 @@ MODULE_ALIAS("sound-layout-100");
>  
>  MODULE_ALIAS("aoa-device-id-14");
>  MODULE_ALIAS("aoa-device-id-22");
> +MODULE_ALIAS("aoa-device-id-31");
>  MODULE_ALIAS("aoa-device-id-35");
>  MODULE_ALIAS("aoa-device-id-44");
>  
> @@ -362,6 +363,13 @@ static struct layout layouts[] = {
>   .connections = tas_connections_nolineout,
>     },
>   },
> + /* PowerBook6,1 */
> + { .device_id = 31,
> +   .codecs[0] = {
> + .name = "tas",
> + .connections = tas_connections_nolineout,
> +   },
> + },
>   /* PowerBook6,5 */
>   { .device_id = 44,
>     .codecs[0] = {
> diff --git a/sound/aoa/soundbus/i2sbus/core.c
> b/sound/aoa/soundbus/i2sbus/core.c
> index 1cbf210..000b585 100644
> --- a/sound/aoa/soundbus/i2sbus/core.c
> +++ b/sound/aoa/soundbus/i2sbus/core.c
> @@ -197,7 +197,7 @@ static int i2sbus_add_dev(struct macio_dev
> *macio,
>    * so restrict to those we do handle for
> now.
>    */
>   if (id && (*id == 22 || *id == 14 || *id ==
> 35 ||
> -    *id == 44)) {
> +    *id == 31 || *id == 44)) {
>   snprintf(dev->sound.modalias, 32,
>    "aoa-device-id-%d", *id);
>   ok = 1;

[PATCH] ALSA: snd-aoa: enable sound on PowerBook G4 12"

[Aaro Koskinen]

Enable sound on PowerBook G4 12".

Signed-off-by: Aaro Koskinen 
---
 sound/aoa/fabrics/layout.c   | 8 
 sound/aoa/soundbus/i2sbus/core.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index 8f71f7e..edc8681 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -112,6 +112,7 @@ MODULE_ALIAS("sound-layout-100");
 
 MODULE_ALIAS("aoa-device-id-14");
 MODULE_ALIAS("aoa-device-id-22");
+MODULE_ALIAS("aoa-device-id-31");
 MODULE_ALIAS("aoa-device-id-35");
 MODULE_ALIAS("aoa-device-id-44");
 
@@ -362,6 +363,13 @@ static struct layout layouts[] = {
.connections = tas_connections_nolineout,
  },
},
+   /* PowerBook6,1 */
+   { .device_id = 31,
+ .codecs[0] = {
+   .name = "tas",
+   .connections = tas_connections_nolineout,
+ },
+   },
/* PowerBook6,5 */
{ .device_id = 44,
  .codecs[0] = {
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 1cbf210..000b585 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -197,7 +197,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,
 * so restrict to those we do handle for now.
 */
if (id && (*id == 22 || *id == 14 || *id == 35 ||
-  *id == 44)) {
+  *id == 31 || *id == 44)) {
snprintf(dev->sound.modalias, 32,
 "aoa-device-id-%d", *id);
ok = 1;
-- 
2.9.2

Re: [PATCH 3/3] net: fs_enet: make rx_copybreak value configurable

[Florian Fainelli]

On 08/24/2016 03:36 AM, Christophe Leroy wrote:
> Measurement shows that on a MPC8xx running at 132MHz, the optimal
> limit is 112:
> * 114 bytes packets are processed in 147 TB ticks with higher copybreak
> * 114 bytes packets are processed in 148 TB ticks with lower copybreak
> * 128 bytes packets are processed in 154 TB ticks with higher copybreak
> * 128 bytes packets are processed in 148 TB ticks with lower copybreak
> * 238 bytes packets are processed in 172 TB ticks with higher copybreak
> * 238 bytes packets are processed in 148 TB ticks with lower copybreak
> 
> However it might be different on other processors
> and/or frequencies. So it is useful to make it configurable.
> 
> Signed-off-by: Christophe Leroy 
> ---
>  drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 8 +---
>  include/linux/fs_enet_pd.h| 1 -
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
> b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index addcae6..b59bbf8 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> @@ -60,6 +60,10 @@ module_param(fs_enet_debug, int, 0);
>  MODULE_PARM_DESC(fs_enet_debug,
>"Freescale bitmapped debugging message enable value");
>  
> +static int rx_copybreak = 240;
> +module_param(rx_copybreak, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(rx_copybreak, "Receive copy threshold");

There is an ethtool tunable knob for copybreak now, which you should
prefer over a module parameter, see
drivers/net/ethernet/cisco/enic/enic_ethtool.c
-- 
Florian

Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start

[Daniel Lezcano]

On 08/24/2016 04:48 PM, Balbir Singh wrote:
> 
> 
> On 25/08/16 00:44, Daniel Lezcano wrote:
>> On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote:
>>> From: "Gautham R. Shenoy" 
>>>
>>> Currently all the idle states registered by a cpu-idle driver are
>>> enabled by default. This patch adds a mechanism which allows the
>>> driver to hint if an idle-state should start in a disabled state. The
>>> cpu-idle core will use this hint to appropriately initialize the
>>> usage->disable knob of the CPU device idle state.
>>
>> Why do you need to do that ?
>>
> 
> I think patch 2/2 explains the reason as it uses this infrastructure

Ok, let me elaborate the question, I was not clear.

Why the userspace can't setup the system environment at boot time by
disabling the state instead of adding extra code to disable it at boot
time in the kernel and then re-enable it from userspace ?

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start

[Balbir Singh]

On 25/08/16 00:44, Daniel Lezcano wrote:
> On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote:
>> From: "Gautham R. Shenoy" 
>>
>> Currently all the idle states registered by a cpu-idle driver are
>> enabled by default. This patch adds a mechanism which allows the
>> driver to hint if an idle-state should start in a disabled state. The
>> cpu-idle core will use this hint to appropriately initialize the
>> usage->disable knob of the CPU device idle state.
> 
> Why do you need to do that ?
> 

I think patch 2/2 explains the reason as it uses this infrastructure

Balbir Singh

Re: [RFC/PATCH 1/2] cpuidle: Allow idle-states to be disabled at start

[Daniel Lezcano]

On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" 
> 
> Currently all the idle states registered by a cpu-idle driver are
> enabled by default. This patch adds a mechanism which allows the
> driver to hint if an idle-state should start in a disabled state. The
> cpu-idle core will use this hint to appropriately initialize the
> usage->disable knob of the CPU device idle state.

Why do you need to do that ?

> The state can be enabled at run time by echo'ing a zero to the sysfs
> "disable" control file.

... for each cpu.


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

[PATCH -next] ibmvnic: convert to use simple_open()

[Wei Yongjun]

From: Wei Yongjun 

Remove an open coded simple_open() function and replace file
operations references to the function with simple_open()
instead.

Generated by: scripts/coccinelle/api/simple_open.cocci

Signed-off-by: Wei Yongjun 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b942108..e862530 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2779,12 +2779,6 @@ static void handle_control_ras_rsp(union ibmvnic_crq 
*crq,
}
 }
 
-static int ibmvnic_fw_comp_open(struct inode *inode, struct file *file)
-{
-   file->private_data = inode->i_private;
-   return 0;
-}
-
 static ssize_t trace_read(struct file *file, char __user *user_buf, size_t len,
  loff_t *ppos)
 {
@@ -2836,7 +2830,7 @@ static ssize_t trace_read(struct file *file, char __user 
*user_buf, size_t len,
 
 static const struct file_operations trace_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = trace_read,
 };
 
@@ -2886,7 +2880,7 @@ static ssize_t paused_write(struct file *file, const char 
__user *user_buf,
 
 static const struct file_operations paused_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = paused_read,
.write  = paused_write,
 };
@@ -2934,7 +2928,7 @@ static ssize_t tracing_write(struct file *file, const 
char __user *user_buf,
 
 static const struct file_operations tracing_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = tracing_read,
.write  = tracing_write,
 };
@@ -2987,7 +2981,7 @@ static ssize_t error_level_write(struct file *file, const 
char __user *user_buf,
 
 static const struct file_operations error_level_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = error_level_read,
.write  = error_level_write,
 };
@@ -3038,7 +3032,7 @@ static ssize_t trace_level_write(struct file *file, const 
char __user *user_buf,
 
 static const struct file_operations trace_level_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = trace_level_read,
.write  = trace_level_write,
 };
@@ -3091,7 +3085,7 @@ static ssize_t trace_buff_size_write(struct file *file,
 
 static const struct file_operations trace_size_ops = {
.owner  = THIS_MODULE,
-   .open   = ibmvnic_fw_comp_open,
+   .open   = simple_open,
.read   = trace_buff_size_read,
.write  = trace_buff_size_write,
 };

[PATCH -next] ibmvnic: fix error return code in ibmvnic_probe()

[Wei Yongjun]

From: Wei Yongjun 

Fix to return error code -ENOMEM from the dma_map_single error
handling case instead of 0, as done elsewhere in this function.

Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Wei Yongjun 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b942108..59245d0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3751,6 +3751,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
if (dma_mapping_error(>dev, adapter->stats_token)) {
if (!firmware_has_feature(FW_FEATURE_CMO))
dev_err(>dev, "Couldn't map stats buffer\n");
+   rc = -ENOMEM;
goto free_crq;
}

Re: [PATCH 1/3] net: fs_enet: merge NAPI RX and NAPI TX

[Christophe Leroy]

Le 24/08/2016 à 15:07, Eric Dumazet a écrit :

On Wed, 2016-08-24 at 12:36 +0200, Christophe Leroy wrote:

Initially, a NAPI TX routine has been implemented separately from
NAPI RX, as done on the freescale/gianfar driver.

By merging NAPI RX and NAPI TX, we reduce the amount of TX completion
interrupts.

Handling of the budget in association with TX interrupts is based on
indications provided at https://wiki.linuxfoundation.org/networking/napi

At the same time, we fix an issue in the handling of fep->tx_free:

It is only when fep->tx_free goes up to MAX_SKB_FRAGS that
we need to wake up the queue. There is no need to call
netif_wake_queue() at every packet successfully transmitted.

Signed-off-by: Christophe Leroy 
---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 259 +
 drivers/net/ethernet/freescale/fs_enet/fs_enet.h   |  16 +-
 drivers/net/ethernet/freescale/fs_enet/mac-fcc.c   |  57 ++---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c   |  57 ++---
 drivers/net/ethernet/freescale/fs_enet/mac-scc.c   |  57 ++---
 5 files changed, 153 insertions(+), 293 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 61fd486..7cd3ef9 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -79,8 +79,8 @@ static void skb_align(struct sk_buff *skb, int align)
skb_reserve(skb, align - off);
 }

-/* NAPI receive function */
-static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
+/* NAPI function */
+static int fs_enet_napi(struct napi_struct *napi, int budget)
 {
struct fs_enet_private *fep = container_of(napi, struct 
fs_enet_private, napi);
struct net_device *dev = fep->ndev;
@@ -90,9 +90,100 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int 
budget)
int received = 0;
u16 pkt_len, sc;
int curidx;
+   int dirtyidx, do_wake, do_restart;

-   if (budget <= 0)
-   return received;
+   spin_lock(>tx_lock);
+   bdp = fep->dirty_tx;
+
+   /* clear status bits for napi*/
+   (*fep->ops->napi_clear_event)(dev);
+
+   do_wake = do_restart = 0;
+   while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {


I am afraid you could live lock here on SMP.

You should make sure you do not loop forever, not assuming cpu is faster
than NIC.


This peace of code is pure move of existing code below

-static int fs_enet_tx_napi(struct napi_struct *napi, int budget)
-{
-   struct fs_enet_private *fep = container_of(napi, struct fs_enet_private,
-  napi_tx);
-   struct net_device *dev = fep->ndev;
-   cbd_t __iomem *bdp;
-   struct sk_buff *skb;
-   int dirtyidx, do_wake, do_restart;
-   u16 sc;
-   int has_tx_work = 0;
-
-   spin_lock(>tx_lock);
-   bdp = fep->dirty_tx;
-
-   /* clear TX status bits for napi*/
-   (*fep->ops->napi_clear_tx_event)(dev);
-
-   do_wake = do_restart = 0;
-   while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {
-   dirtyidx = bdp - fep->tx_bd_base;

What should be done instead (any exemple driver doing it the good way ?) 
and should that change be part of that patch or a another new one ?


Christophe






+   dirtyidx = bdp - fep->tx_bd_base;
+
+   if (fep->tx_free == fep->tx_ring)
+   break;
+
+   skb = fep->tx_skbuff[dirtyidx];
+
+   /*
+* Check for errors.
+*/
+   if (sc & (BD_ENET_TX_HB | BD_ENET_TX_LC |
+ BD_ENET_TX_RL | BD_ENET_TX_UN | BD_ENET_TX_CSL)) {
+
+   if (sc & BD_ENET_TX_HB) /* No heartbeat */
+   fep->stats.tx_heartbeat_errors++;
+   if (sc & BD_ENET_TX_LC) /* Late collision */
+   fep->stats.tx_window_errors++;
+   if (sc & BD_ENET_TX_RL) /* Retrans limit */
+   fep->stats.tx_aborted_errors++;
+   if (sc & BD_ENET_TX_UN) /* Underrun */
+   fep->stats.tx_fifo_errors++;
+   if (sc & BD_ENET_TX_CSL)/* Carrier lost */
+   fep->stats.tx_carrier_errors++;
+
+   if (sc & (BD_ENET_TX_LC | BD_ENET_TX_RL | 
BD_ENET_TX_UN)) {
+   fep->stats.tx_errors++;
+   do_restart = 1;
+   }
+   } else
+   fep->stats.tx_packets++;
+
+   if (sc & BD_ENET_TX_READY) {
+   dev_warn(fep->dev,
+"HEY! Enet xmit interrupt and TX_READY.\n");
+   }
+
+   /*
+

Re: [PATCH 1/3] net: fs_enet: merge NAPI RX and NAPI TX

[Eric Dumazet]

On Wed, 2016-08-24 at 06:07 -0700, Eric Dumazet wrote:

> I am afraid you could live lock here on SMP.
> 
> You should make sure you do not loop forever, not assuming cpu is faster
> than NIC.

You are protected by the tx_lock spinlock, but this is fragile as you
could very well remove this spinlock in the future to get lockless
transmit, like many other drivers.

Re: [PATCH 1/3] net: fs_enet: merge NAPI RX and NAPI TX

[Eric Dumazet]

On Wed, 2016-08-24 at 12:36 +0200, Christophe Leroy wrote:
> Initially, a NAPI TX routine has been implemented separately from
> NAPI RX, as done on the freescale/gianfar driver.
> 
> By merging NAPI RX and NAPI TX, we reduce the amount of TX completion
> interrupts.
> 
> Handling of the budget in association with TX interrupts is based on
> indications provided at https://wiki.linuxfoundation.org/networking/napi
> 
> At the same time, we fix an issue in the handling of fep->tx_free:
> 
> It is only when fep->tx_free goes up to MAX_SKB_FRAGS that
> we need to wake up the queue. There is no need to call
> netif_wake_queue() at every packet successfully transmitted.
> 
> Signed-off-by: Christophe Leroy 
> ---
>  .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 259 
> +
>  drivers/net/ethernet/freescale/fs_enet/fs_enet.h   |  16 +-
>  drivers/net/ethernet/freescale/fs_enet/mac-fcc.c   |  57 ++---
>  drivers/net/ethernet/freescale/fs_enet/mac-fec.c   |  57 ++---
>  drivers/net/ethernet/freescale/fs_enet/mac-scc.c   |  57 ++---
>  5 files changed, 153 insertions(+), 293 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
> b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index 61fd486..7cd3ef9 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> @@ -79,8 +79,8 @@ static void skb_align(struct sk_buff *skb, int align)
>   skb_reserve(skb, align - off);
>  }
>  
> -/* NAPI receive function */
> -static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
> +/* NAPI function */
> +static int fs_enet_napi(struct napi_struct *napi, int budget)
>  {
>   struct fs_enet_private *fep = container_of(napi, struct 
> fs_enet_private, napi);
>   struct net_device *dev = fep->ndev;
> @@ -90,9 +90,100 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int 
> budget)
>   int received = 0;
>   u16 pkt_len, sc;
>   int curidx;
> + int dirtyidx, do_wake, do_restart;
>  
> - if (budget <= 0)
> - return received;
> + spin_lock(>tx_lock);
> + bdp = fep->dirty_tx;
> +
> + /* clear status bits for napi*/
> + (*fep->ops->napi_clear_event)(dev);
> +
> + do_wake = do_restart = 0;
> + while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {

I am afraid you could live lock here on SMP.

You should make sure you do not loop forever, not assuming cpu is faster
than NIC.



> + dirtyidx = bdp - fep->tx_bd_base;
> +
> + if (fep->tx_free == fep->tx_ring)
> + break;
> +
> + skb = fep->tx_skbuff[dirtyidx];
> +
> + /*
> +  * Check for errors.
> +  */
> + if (sc & (BD_ENET_TX_HB | BD_ENET_TX_LC |
> +   BD_ENET_TX_RL | BD_ENET_TX_UN | BD_ENET_TX_CSL)) {
> +
> + if (sc & BD_ENET_TX_HB) /* No heartbeat */
> + fep->stats.tx_heartbeat_errors++;
> + if (sc & BD_ENET_TX_LC) /* Late collision */
> + fep->stats.tx_window_errors++;
> + if (sc & BD_ENET_TX_RL) /* Retrans limit */
> + fep->stats.tx_aborted_errors++;
> + if (sc & BD_ENET_TX_UN) /* Underrun */
> + fep->stats.tx_fifo_errors++;
> + if (sc & BD_ENET_TX_CSL)/* Carrier lost */
> + fep->stats.tx_carrier_errors++;
> +
> + if (sc & (BD_ENET_TX_LC | BD_ENET_TX_RL | 
> BD_ENET_TX_UN)) {
> + fep->stats.tx_errors++;
> + do_restart = 1;
> + }
> + } else
> + fep->stats.tx_packets++;
> +
> + if (sc & BD_ENET_TX_READY) {
> + dev_warn(fep->dev,
> +  "HEY! Enet xmit interrupt and TX_READY.\n");
> + }
> +
> + /*
> +  * Deferred means some collisions occurred during transmit,
> +  * but we eventually sent the packet OK.
> +  */
> + if (sc & BD_ENET_TX_DEF)
> + fep->stats.collisions++;
> +
> + /* unmap */
> + if (fep->mapped_as_page[dirtyidx])
> + dma_unmap_page(fep->dev, CBDR_BUFADDR(bdp),
> +CBDR_DATLEN(bdp), DMA_TO_DEVICE);
> + else
> + dma_unmap_single(fep->dev, CBDR_BUFADDR(bdp),
> +  CBDR_DATLEN(bdp), DMA_TO_DEVICE);
> +
> + /*
> +  * Free the sk buffer associated with this last transmit.
> +  */
> + if (skb) {
> + dev_kfree_skb(skb);
> + fep->tx_skbuff[dirtyidx] = NULL;
> + 

[PATCH 3/3] net: fs_enet: make rx_copybreak value configurable

[Christophe Leroy]

Measurement shows that on a MPC8xx running at 132MHz, the optimal
limit is 112:
* 114 bytes packets are processed in 147 TB ticks with higher copybreak
* 114 bytes packets are processed in 148 TB ticks with lower copybreak
* 128 bytes packets are processed in 154 TB ticks with higher copybreak
* 128 bytes packets are processed in 148 TB ticks with lower copybreak
* 238 bytes packets are processed in 172 TB ticks with higher copybreak
* 238 bytes packets are processed in 148 TB ticks with lower copybreak

However it might be different on other processors
and/or frequencies. So it is useful to make it configurable.

Signed-off-by: Christophe Leroy 
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 8 +---
 include/linux/fs_enet_pd.h| 1 -
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index addcae6..b59bbf8 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -60,6 +60,10 @@ module_param(fs_enet_debug, int, 0);
 MODULE_PARM_DESC(fs_enet_debug,
 "Freescale bitmapped debugging message enable value");
 
+static int rx_copybreak = 240;
+module_param(rx_copybreak, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(rx_copybreak, "Receive copy threshold");
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void fs_enet_netpoll(struct net_device *dev);
 #endif
@@ -84,7 +88,6 @@ static int fs_enet_napi(struct napi_struct *napi, int budget)
 {
struct fs_enet_private *fep = container_of(napi, struct 
fs_enet_private, napi);
struct net_device *dev = fep->ndev;
-   const struct fs_platform_info *fpi = fep->fpi;
cbd_t __iomem *bdp;
struct sk_buff *skb, *skbn;
int received = 0;
@@ -232,7 +235,7 @@ static int fs_enet_napi(struct napi_struct *napi, int 
budget)
pkt_len = CBDR_DATLEN(bdp) - 4; /* remove CRC */
fep->stats.rx_bytes += pkt_len + 4;
 
-   if (pkt_len <= fpi->rx_copybreak) {
+   if (pkt_len <= rx_copybreak) {
/* +2 to make IP header L1 cache aligned */
skbn = netdev_alloc_skb(dev, pkt_len + 2);
if (skbn != NULL) {
@@ -905,7 +908,6 @@ static int fs_enet_probe(struct platform_device *ofdev)
 
fpi->rx_ring = 32;
fpi->tx_ring = 64;
-   fpi->rx_copybreak = 240;
fpi->napi_weight = 17;
fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
if (!fpi->phy_node && of_phy_is_fixed_link(ofdev->dev.of_node)) {
diff --git a/include/linux/fs_enet_pd.h b/include/linux/fs_enet_pd.h
index 77d783f..376600e 100644
--- a/include/linux/fs_enet_pd.h
+++ b/include/linux/fs_enet_pd.h
@@ -138,7 +138,6 @@ struct fs_platform_info {
 
int rx_ring, tx_ring;   /* number of buffers on rx */
__u8 macaddr[ETH_ALEN]; /* mac address */
-   int rx_copybreak;   /* limit we copy small frames  */
int napi_weight;/* NAPI weight */
 
int use_rmii;   /* use RMII mode   */
-- 
2.1.0

[PATCH 2/3] net: fs_enet: don't unmap DMA when packet len is below copybreak

[Christophe Leroy]

When the length of the packet is below the defined copybreak limit,
the received packet is copied into a newly allocated skb in order
to reuse the skb. This is only interesting if it allow us to avoid
a new DMA mapping. We shall therefore not DMA unmap and remap the
skb->data. Instead, we invalidate the cache
with dma_sync_single_for_cpu() once the received data has been
copied into the new skb.

The following measures have been obtained on a mpc885 running at 132Mhz.
Measurement is done using the timebase with packets sent to the target
with 'ping -s 1' (packet len is 60):
* Without this patch: 182 TB ticks
* With this patch: 143 TB ticks

As a comparison, if we set the copybreak limit to 0, then we get
148 TB ticks. It means that without this patch, duration is even
worse when copying received data to a new skb instead of
allocating a new skb for next packet to be received

Signed-off-by: Christophe Leroy 
---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 36 --
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 7cd3ef9..addcae6 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -221,21 +221,10 @@ static int fs_enet_napi(struct napi_struct *napi, int 
budget)
if (sc & BD_ENET_RX_OV)
fep->stats.rx_crc_errors++;
 
-   skb = fep->rx_skbuff[curidx];
-
-   dma_unmap_single(fep->dev, CBDR_BUFADDR(bdp),
-   L1_CACHE_ALIGN(PKT_MAXBUF_SIZE),
-   DMA_FROM_DEVICE);
-
-   skbn = skb;
-
+   skbn = fep->rx_skbuff[curidx];
} else {
skb = fep->rx_skbuff[curidx];
 
-   dma_unmap_single(fep->dev, CBDR_BUFADDR(bdp),
-   L1_CACHE_ALIGN(PKT_MAXBUF_SIZE),
-   DMA_FROM_DEVICE);
-
/*
 * Process the incoming frame.
 */
@@ -251,12 +240,30 @@ static int fs_enet_napi(struct napi_struct *napi, int 
budget)
skb_copy_from_linear_data(skb,
  skbn->data, pkt_len);
swap(skb, skbn);
+   dma_sync_single_for_cpu(fep->dev,
+   CBDR_BUFADDR(bdp),
+   L1_CACHE_ALIGN(pkt_len),
+   DMA_FROM_DEVICE);
}
} else {
skbn = netdev_alloc_skb(dev, ENET_RX_FRSIZE);
 
-   if (skbn)
+   if (skbn) {
+   dma_addr_t dma;
+
skb_align(skbn, ENET_RX_ALIGN);
+
+   dma_unmap_single(fep->dev,
+   CBDR_BUFADDR(bdp),
+   L1_CACHE_ALIGN(PKT_MAXBUF_SIZE),
+   DMA_FROM_DEVICE);
+
+   dma = dma_map_single(fep->dev,
+   skbn->data,
+   L1_CACHE_ALIGN(PKT_MAXBUF_SIZE),
+   DMA_FROM_DEVICE);
+   CBDW_BUFADDR(bdp, dma);
+   }
}
 
if (skbn != NULL) {
@@ -271,9 +278,6 @@ static int fs_enet_napi(struct napi_struct *napi, int 
budget)
}
 
fep->rx_skbuff[curidx] = skbn;
-   CBDW_BUFADDR(bdp, dma_map_single(fep->dev, skbn->data,
-L1_CACHE_ALIGN(PKT_MAXBUF_SIZE),
-DMA_FROM_DEVICE));
CBDW_DATLEN(bdp, 0);
CBDW_SC(bdp, (sc & ~BD_ENET_RX_STATS) | BD_ENET_RX_EMPTY);
 
-- 
2.1.0

[PATCH 1/3] net: fs_enet: merge NAPI RX and NAPI TX

[Christophe Leroy]

Initially, a NAPI TX routine has been implemented separately from
NAPI RX, as done on the freescale/gianfar driver.

By merging NAPI RX and NAPI TX, we reduce the amount of TX completion
interrupts.

Handling of the budget in association with TX interrupts is based on
indications provided at https://wiki.linuxfoundation.org/networking/napi

At the same time, we fix an issue in the handling of fep->tx_free:

It is only when fep->tx_free goes up to MAX_SKB_FRAGS that
we need to wake up the queue. There is no need to call
netif_wake_queue() at every packet successfully transmitted.

Signed-off-by: Christophe Leroy 
---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 259 +
 drivers/net/ethernet/freescale/fs_enet/fs_enet.h   |  16 +-
 drivers/net/ethernet/freescale/fs_enet/mac-fcc.c   |  57 ++---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c   |  57 ++---
 drivers/net/ethernet/freescale/fs_enet/mac-scc.c   |  57 ++---
 5 files changed, 153 insertions(+), 293 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c 
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 61fd486..7cd3ef9 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -79,8 +79,8 @@ static void skb_align(struct sk_buff *skb, int align)
skb_reserve(skb, align - off);
 }
 
-/* NAPI receive function */
-static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
+/* NAPI function */
+static int fs_enet_napi(struct napi_struct *napi, int budget)
 {
struct fs_enet_private *fep = container_of(napi, struct 
fs_enet_private, napi);
struct net_device *dev = fep->ndev;
@@ -90,9 +90,100 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int 
budget)
int received = 0;
u16 pkt_len, sc;
int curidx;
+   int dirtyidx, do_wake, do_restart;
 
-   if (budget <= 0)
-   return received;
+   spin_lock(>tx_lock);
+   bdp = fep->dirty_tx;
+
+   /* clear status bits for napi*/
+   (*fep->ops->napi_clear_event)(dev);
+
+   do_wake = do_restart = 0;
+   while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {
+   dirtyidx = bdp - fep->tx_bd_base;
+
+   if (fep->tx_free == fep->tx_ring)
+   break;
+
+   skb = fep->tx_skbuff[dirtyidx];
+
+   /*
+* Check for errors.
+*/
+   if (sc & (BD_ENET_TX_HB | BD_ENET_TX_LC |
+ BD_ENET_TX_RL | BD_ENET_TX_UN | BD_ENET_TX_CSL)) {
+
+   if (sc & BD_ENET_TX_HB) /* No heartbeat */
+   fep->stats.tx_heartbeat_errors++;
+   if (sc & BD_ENET_TX_LC) /* Late collision */
+   fep->stats.tx_window_errors++;
+   if (sc & BD_ENET_TX_RL) /* Retrans limit */
+   fep->stats.tx_aborted_errors++;
+   if (sc & BD_ENET_TX_UN) /* Underrun */
+   fep->stats.tx_fifo_errors++;
+   if (sc & BD_ENET_TX_CSL)/* Carrier lost */
+   fep->stats.tx_carrier_errors++;
+
+   if (sc & (BD_ENET_TX_LC | BD_ENET_TX_RL | 
BD_ENET_TX_UN)) {
+   fep->stats.tx_errors++;
+   do_restart = 1;
+   }
+   } else
+   fep->stats.tx_packets++;
+
+   if (sc & BD_ENET_TX_READY) {
+   dev_warn(fep->dev,
+"HEY! Enet xmit interrupt and TX_READY.\n");
+   }
+
+   /*
+* Deferred means some collisions occurred during transmit,
+* but we eventually sent the packet OK.
+*/
+   if (sc & BD_ENET_TX_DEF)
+   fep->stats.collisions++;
+
+   /* unmap */
+   if (fep->mapped_as_page[dirtyidx])
+   dma_unmap_page(fep->dev, CBDR_BUFADDR(bdp),
+  CBDR_DATLEN(bdp), DMA_TO_DEVICE);
+   else
+   dma_unmap_single(fep->dev, CBDR_BUFADDR(bdp),
+CBDR_DATLEN(bdp), DMA_TO_DEVICE);
+
+   /*
+* Free the sk buffer associated with this last transmit.
+*/
+   if (skb) {
+   dev_kfree_skb(skb);
+   fep->tx_skbuff[dirtyidx] = NULL;
+   }
+
+   /*
+* Update pointer to next buffer descriptor to be transmitted.
+*/
+   if ((sc & BD_ENET_TX_WRAP) == 0)
+   bdp++;
+   else
+   bdp = fep->tx_bd_base;
+
+   /*
+ 

[PATCH 0/3] Optimisation of fs_enet ethernet driver

[Christophe Leroy]

This set optimises the freescale fs_enet ethernet driver:
1/ Merge of RX and TX NAPI functions in order to limit the amount of
interrupts
2/ Do not unmap DMA when packets len is below copybreak, otherwise there
is no benefit in copying the skb instead of allocating a new one
3/ Make copybreak value configurable as the optimised value is not the
same on all targets

chleroy (3):
  net: fs_enet: merge NAPI RX and NAPI TX
  net: fs_enet: don't unmap DMA when packet len is below copybreak
  net: fs_enet: make rx_copybreak value configurable

 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  | 303 +
 drivers/net/ethernet/freescale/fs_enet/fs_enet.h   |  16 +-
 drivers/net/ethernet/freescale/fs_enet/mac-fcc.c   |  57 +---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c   |  57 +---
 drivers/net/ethernet/freescale/fs_enet/mac-scc.c   |  57 +---
 include/linux/fs_enet_pd.h |   1 -
 6 files changed, 178 insertions(+), 313 deletions(-)

-- 
2.1.0

[PATCH 4/4] powerpc/mm: Update the HID bit when switching from radix to hash

[Aneesh Kumar K.V]

Power9 DD1 requires to update the hid0 register when switching from
hash to radix.

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/reg.h  |  3 +++
 arch/powerpc/mm/hash_utils_64.c | 25 +
 arch/powerpc/mm/pgtable-radix.c | 28 
 3 files changed, 56 insertions(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index f69f40f1519a..9dddabc2fced 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -475,6 +475,9 @@
 #define HID0_POWER8_1TO4LPAR   __MASK(51)
 #define HID0_POWER8_DYNLPARDIS __MASK(48)
 
+/* POWER9 HID0 bits */
+#define HID0_POWER9_RADIX  __MASK(63 - 8)
+
 #define SPRN_HID1  0x3F1   /* Hardware Implementation Register 1 */
 #ifdef CONFIG_6xx
 #define HID1_EMCP  (1<<31) /* 7450 Machine Check Pin Enable */
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 0821556e16f4..35a6721b3d25 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -711,6 +711,29 @@ int remove_section_mapping(unsigned long start, unsigned 
long end)
 }
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
+static void update_hid_for_hash(void)
+{
+   unsigned long hid0;
+   unsigned long rb = 3UL << PPC_BITLSHIFT(53); /* IS = 3 */
+
+   asm volatile("ptesync": : :"memory");
+   /* prs = 0, ric = 2, rs = 0, r = 1 is = 3 */
+   asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
+: : "r"(rb), "i"(0), "i"(0), "i"(2), "r"(0) : "memory");
+   asm volatile("eieio; tlbsync; ptesync; isync; slbia": : :"memory");
+   /*
+* now switch the HID
+*/
+   hid0  = mfspr(SPRN_HID0);
+   hid0 &= ~HID0_POWER9_RADIX;
+   mtspr(SPRN_HID0, hid0);
+   asm volatile("isync": : :"memory");
+
+   /* Wait for it to happen */
+   while ((mfspr(SPRN_HID0) & HID0_POWER9_RADIX))
+   cpu_relax();
+}
+
 static void __init hash_init_partition_table(phys_addr_t hash_table,
 unsigned long htab_size)
 {
@@ -737,6 +760,8 @@ static void __init hash_init_partition_table(phys_addr_t 
hash_table,
 */
partition_tb->patb1 = 0;
pr_info("Partition table %p\n", partition_tb);
+   if (cpu_has_feature(CPU_FTR_POWER9_DD1))
+   update_hid_for_hash();
/*
 * update partition table control register,
 * 64 K size.
diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
index af897d91d09f..8f086352e421 100644
--- a/arch/powerpc/mm/pgtable-radix.c
+++ b/arch/powerpc/mm/pgtable-radix.c
@@ -294,6 +294,32 @@ found:
return;
 }
 
+static void update_hid_for_radix(void)
+{
+   unsigned long hid0;
+   unsigned long rb = 3UL << PPC_BITLSHIFT(53); /* IS = 3 */
+
+   asm volatile("ptesync": : :"memory");
+   /* prs = 0, ric = 2, rs = 0, r = 1 is = 3 */
+   asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
+: : "r"(rb), "i"(1), "i"(0), "i"(2), "r"(0) : "memory");
+   /* prs = 1, ric = 2, rs = 0, r = 1 is = 3 */
+   asm volatile(PPC_TLBIE_5(%0, %4, %3, %2, %1)
+: : "r"(rb), "i"(1), "i"(1), "i"(2), "r"(0) : "memory");
+   asm volatile("eieio; tlbsync; ptesync; isync; slbia": : :"memory");
+   /*
+* now switch the HID
+*/
+   hid0  = mfspr(SPRN_HID0);
+   hid0 |= HID0_POWER9_RADIX;
+   mtspr(SPRN_HID0, hid0);
+   asm volatile("isync": : :"memory");
+
+   /* Wait for it to happen */
+   while (!(mfspr(SPRN_HID0) & HID0_POWER9_RADIX))
+   cpu_relax();
+}
+
 void __init radix__early_init_mmu(void)
 {
unsigned long lpcr;
@@ -345,6 +371,8 @@ void __init radix__early_init_mmu(void)
 
if (!firmware_has_feature(FW_FEATURE_LPAR)) {
radix_init_native();
+   if (cpu_has_feature(CPU_FTR_POWER9_DD1))
+   update_hid_for_radix();
lpcr = mfspr(SPRN_LPCR);
mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
radix_init_partition_table();
-- 
2.7.4

[PATCH 3/4] powerpc/mm/radix: Use different pte update sequence for different POWER9 revs

[Aneesh Kumar K.V]

POWER9 DD1 requires pte to be marked invalid (V=0) before updating
it with the new value. This makes this distinction for the different
revisions.

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/book3s/32/pgtable.h |  3 +-
 arch/powerpc/include/asm/book3s/64/pgtable.h |  5 +-
 arch/powerpc/include/asm/book3s/64/radix.h   | 75 ++--
 arch/powerpc/include/asm/nohash/32/pgtable.h |  3 +-
 arch/powerpc/include/asm/nohash/64/pgtable.h |  3 +-
 arch/powerpc/mm/pgtable-book3s64.c   |  2 +-
 arch/powerpc/mm/pgtable.c|  2 +-
 7 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h 
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 38b33dcfcc9d..6b8b2d57fdc8 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -223,7 +223,8 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct 
*mm,
 }
 
 
-static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
+static inline void __ptep_set_access_flags(struct mm_struct *mm,
+  pte_t *ptep, pte_t entry)
 {
unsigned long set = pte_val(entry) &
(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 263bf39ced40..8ec8be9495ba 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -565,10 +565,11 @@ static inline bool check_pte_access(unsigned long access, 
unsigned long ptev)
  * Generic functions with hash/radix callbacks
  */
 
-static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
+static inline void __ptep_set_access_flags(struct mm_struct *mm,
+  pte_t *ptep, pte_t entry)
 {
if (radix_enabled())
-   return radix__ptep_set_access_flags(ptep, entry);
+   return radix__ptep_set_access_flags(mm, ptep, entry);
return hash__ptep_set_access_flags(ptep, entry);
 }
 
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
b/arch/powerpc/include/asm/book3s/64/radix.h
index a2fe8fbfbd3d..2a46dea8e1b1 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -11,6 +11,11 @@
 #include 
 #endif
 
+#ifndef __ASSEMBLY__
+#include 
+#include 
+#endif
+
 /* An empty PTE can still have a R or C writeback */
 #define RADIX_PTE_NONE_MASK(_PAGE_DIRTY | _PAGE_ACCESSED)
 
@@ -105,11 +110,8 @@
 #define RADIX_PUD_TABLE_SIZE   (sizeof(pud_t) << RADIX_PUD_INDEX_SIZE)
 #define RADIX_PGD_TABLE_SIZE   (sizeof(pgd_t) << RADIX_PGD_INDEX_SIZE)
 
-static inline unsigned long radix__pte_update(struct mm_struct *mm,
-   unsigned long addr,
-   pte_t *ptep, unsigned long clr,
-   unsigned long set,
-   int huge)
+static inline unsigned long __radix_pte_update(pte_t *ptep, unsigned long clr,
+  unsigned long set)
 {
pte_t pte;
unsigned long old_pte, new_pte;
@@ -121,9 +123,39 @@ static inline unsigned long radix__pte_update(struct 
mm_struct *mm,
 
} while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
 
-   /* We already do a sync in cmpxchg, is ptesync needed ?*/
+   return old_pte;
+}
+
+
+static inline unsigned long radix__pte_update(struct mm_struct *mm,
+   unsigned long addr,
+   pte_t *ptep, unsigned long clr,
+   unsigned long set,
+   int huge)
+{
+   unsigned long old_pte;
+
+   if (cpu_has_feature(CPU_FTR_POWER9_DD1)) {
+
+   unsigned long new_pte;
+
+   old_pte = __radix_pte_update(ptep, ~0, 0);
+   asm volatile("ptesync" : : : "memory");
+   /*
+* new value of pte
+*/
+   new_pte = (old_pte | set) & ~clr;
+
+   /*
+* For now let's do heavy pid flush
+* radix__flush_tlb_page_psize(mm, addr, mmu_virtual_psize);
+*/
+   radix__flush_tlb_mm(mm);
+
+   __radix_pte_update(ptep, 0, new_pte);
+   } else
+   old_pte = __radix_pte_update(ptep, clr, set);
asm volatile("ptesync" : : : "memory");
-   /* huge pages use the old page table lock */
if (!huge)
assert_pte_locked(mm, addr);
 
@@ -134,20 +166,33 @@ static inline unsigned long radix__pte_update(struct 
mm_struct *mm,
  * Set the dirty and/or accessed bits atomically in a linux PTE, this
  * function doesn't need to invalidate tlb.
  */
-static 

[PATCH 2/4] powerpc/mm/radix: Use different RTS encoding for different POWER9 revs

[Aneesh Kumar K.V]

POWER9 DD1 uses RTS - 28 for the RTS value but other revisions use
RTS - 31.  This makes this distinction for the different revisions

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/book3s/64/radix.h | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/radix.h 
b/arch/powerpc/include/asm/book3s/64/radix.h
index df294224e280..a2fe8fbfbd3d 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -233,14 +233,19 @@ static inline unsigned long radix__get_tree_size(void)
 {
unsigned long rts_field;
/*
-* we support 52 bits, hence 52-31 = 21, 0b10101
+* We support 52 bits, hence:
+*  DD152-28 = 24, 0b11000
+*  Others 52-31 = 21, 0b10101
 * RTS encoding details
 * bits 0 - 3 of rts -> bits 6 - 8 unsigned long
 * bits 4 - 5 of rts -> bits 62 - 63 of unsigned long
 */
-   rts_field = (0x5UL << 5); /* 6 - 8 bits */
-   rts_field |= (0x2UL << 61);
-
+   if (cpu_has_feature(CPU_FTR_POWER9_DD1))
+   rts_field = (0x3UL << 61);
+   else {
+   rts_field = (0x5UL << 5); /* 6 - 8 bits */
+   rts_field |= (0x2UL << 61);
+   }
return rts_field;
 }
 #endif /* __ASSEMBLY__ */
-- 
2.7.4

[PATCH 1/4] powerpc/book3s: Add a cpu table entry for different POWER9 revs

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V 
---
 arch/powerpc/include/asm/cputable.h |  4 +++-
 arch/powerpc/kernel/cputable.c  | 19 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputable.h 
b/arch/powerpc/include/asm/cputable.h
index 82026b419341..f752e6f7cfbe 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -212,6 +212,7 @@ enum {
 #define CPU_FTR_DABRX  LONG_ASM_CONST(0x0800)
 #define CPU_FTR_PMAO_BUG   LONG_ASM_CONST(0x1000)
 #define CPU_FTR_SUBCORE
LONG_ASM_CONST(0x2000)
+#define CPU_FTR_POWER9_DD1 LONG_ASM_CONST(0x4000)
 
 #ifndef __ASSEMBLY__
 
@@ -472,6 +473,7 @@ enum {
CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY | \
CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_DAWR | \
CPU_FTR_ARCH_207S | CPU_FTR_TM_COMP | CPU_FTR_ARCH_300)
+#define CPU_FTRS_POWER9_DD1 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD1)
 #define CPU_FTRS_CELL  (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
@@ -490,7 +492,7 @@ enum {
(CPU_FTRS_POWER4 | CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
 CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
 CPU_FTRS_POWER8 | CPU_FTRS_POWER8_DD1 | CPU_FTRS_CELL | \
-CPU_FTRS_PA6T | CPU_FTR_VSX | CPU_FTRS_POWER9)
+CPU_FTRS_PA6T | CPU_FTR_VSX | CPU_FTRS_POWER9 | 
CPU_FTRS_POWER9_DD1)
 #endif
 #else
 enum {
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 74248ab18e98..6c4646ac9234 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -506,6 +506,25 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check_early= __machine_check_early_realmode_p8,
.platform   = "power8",
},
+   {   /* Power9 DD1*/
+   .pvr_mask   = 0xff00,
+   .pvr_value  = 0x004e0100,
+   .cpu_name   = "POWER9 (raw)",
+   .cpu_features   = CPU_FTRS_POWER9_DD1,
+   .cpu_user_features  = COMMON_USER_POWER9,
+   .cpu_user_features2 = COMMON_USER2_POWER9,
+   .mmu_features   = MMU_FTRS_POWER9,
+   .icache_bsize   = 128,
+   .dcache_bsize   = 128,
+   .num_pmcs   = 6,
+   .pmc_type   = PPC_PMC_IBM,
+   .oprofile_cpu_type  = "ppc64/power9",
+   .oprofile_type  = PPC_OPROFILE_INVALID,
+   .cpu_setup  = __setup_cpu_power9,
+   .cpu_restore= __restore_cpu_power9,
+   .flush_tlb  = __flush_tlb_power9,
+   .platform   = "power9",
+   },
{   /* Power9 */
.pvr_mask   = 0x,
.pvr_value  = 0x004e,
-- 
2.7.4

Re: [PATCH v3 2/5] firmware: annotate thou shalt not request fw on init or probe

[Gabriel Paubert]

On Tue, Aug 23, 2016 at 05:45:04PM -0700, mcg...@kernel.org wrote:

[snip]
> ---
>  Documentation/firmware_class/README|  20 
>  drivers/base/Kconfig   |   2 +-
>  .../request_firmware-avoid-init-probe-init.cocci   | 130 
> +
>  3 files changed, 151 insertions(+), 1 deletion(-)
>  create mode 100644 
> scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
> 
> diff --git a/Documentation/firmware_class/README 
> b/Documentation/firmware_class/README
> index cafdca8b3b15..056d1cb9d365 100644
> --- a/Documentation/firmware_class/README
> +++ b/Documentation/firmware_class/README
> @@ -93,6 +93,26 @@
> user contexts to request firmware asynchronously, but can't be called
> in atomic contexts.
>  
> +Requirements:
> +=
> +
> +You should avoid at all costs requesting firmware on both init and probe 
> paths
> +of your device driver. Reason for this is the complexity needed to ensure a
> +firmware will be available for a driver early in boot through different
> +build configurations. Consider built-in drivers needing firmware early, or
> +consider a driver assuming it will only get firmware after pivot_root().
> +
> +Drivers that really need firmware early should use stuff the firmware in

Minor grammatical nit: s/use//

> +initramfs or consider using CONFIG_EXTRA_FIRMWARE. Using initramfs is much
> +more portable to more distributions as not all distributions wish to enable
> +CONFIG_EXTRA_FIRMWARE. Should a driver require the firmware being built-in
> +it should depend on CONFIG_EXTRA_FIRMWARE. There is no current annotation for
> +requiring a firmware on initramfs.
> +
> +If you're a maintainer you can help police this with:
> +
> +$ export 
> COCCI=scripts/coccinelle/api/request_firmware-avoid-init-probe-init.cocci
> +$ make coccicheck MODE=report
>  
>   about in-kernel persistence:
>   ---

Re: [PATCH] powerpc/ptrace: Fix cppcheck issue in gpr32_set_common/gpr32_get_common.

[Simon Guo]

Hi Daniel,
On Wed, Aug 24, 2016 at 12:21:23PM +1000, Daniel Axtens wrote:
> Hi Simon,
> 
> > The ckpt_regs usage in gpr32_set_common/gpr32_get_common()
> > will lead to cppcheck error.
> >
> > [arch/powerpc/kernel/ptrace.c:2062]: (error) Uninitialized variable: 
> > ckpt_regs
> > [arch/powerpc/kernel/ptrace.c:2130]: (error) Uninitialized variable: 
> > ckpt_regs
> >
> > A straightforward fix to clean it.
> 
> I'm always happy to see cppcheck warnings fixed :)
Thanks for raising this issue :)

> 
> >  static int gpr32_get_common(struct task_struct *target,
> >  const struct user_regset *regset,
> >  unsigned int pos, unsigned int count,
> > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > void *kbuf, void __user *ubuf, bool tm_active)
> > +#else
> > +   void *kbuf, void __user *ubuf)
> > +#endif
> 
> I wonder if it might be possible to avoid some of the ifdefs and general
> churn by making the tm_active argument __maybe_unused rather than
> ifdefing around it?
> 
> In particular, it would mean the two hunks in the function definitions
> and these these two hunks at the call site would be unnecessary:
I think keeping tm_active argument for "ifndef CONFIG_PPC_TRANSACTIONAL_MEM" 
case (with __maybe_unused prefix) will be somehow strange -- Whatever
value is provided in the caller function for tm_active, programmer might be 
puzzled and cost sometime to think about it.  I don't like to use 
"__maybe_unused" to bypass this warning.

Thanks,
- Simon

Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe

[Daniel Vetter]

On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez  wrote:
> Thou shalt not make firmware calls early on init or probe.
>
> systemd already ripped support out for the usermode helper
> a while ago, there are still users that require the usermode helper,
> however systemd's use of the usermode helper exacerbated a long
> lasting issue of the fact that we have many drivers that load
> firmware on init or probe. Independent of the usermode helper
> loading firmware on init or probe is a bad idea for a few reasons.
>
> When the firmware is read directly from the filesystem by the kernel,
> if the driver is built-in to the kernel the firmware may not yet be
> available, for such uses one could use initramfs and stuff the firmware
> inside, or one also use CONFIG_EXTRA_FIRMWARE; however not all distributions
> use this, as such generally one cannot count on this. There is another
> corner cases to consider, since we are accessing the firmware directly folks
> cannot expect new found firmware on a filesystem after we switch off from
> an initramfs with pivot_root().
>
> Supporting such situations is possible today but fixing it for good is
> really complex due to the large amount of variablity in the boot up
> process.
>
> Instead just document the expectations properly and add a grammar rule to
> enable folks to check / validate / police if drivers are using the request
> firmware API early on init or probe.
>
> The SmPL rule used to check for the probe routine is loose and is
> currently defined through a regexp, that can easily be extended to any
> other known bus probe routine names. It also uses the new Python
> iteration support which allows us to backtrack from a request_firmware*()
> call back to a possible probe or init, iteratively. Without iteration
> we would only be able to get reports for callers who directly use the
> request_firmware*() API on the initial probe or init routine.
>
> There are 4 offenders at this time:
>
> mcgrof@ergon ~/linux-next (git::20160609)$ export 
> COCCI=scripts/coccinelle/api/request_firmware.cocci
> mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report
>
> drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its 
> init routine on line 321.
> drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on 
> its probe routine on line 136.
> drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its 
> probe routine on line 796.
> drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on 
> its probe routine on line 1246.

Plus all gpu drivers which need firmware. And yes we must load them at
probe because people are generally pissed when they boot their machine
and the screen goes black. On top of that a lot of people want their
gpu drivers to be built-in, but can't ship the firmware blobs in the
kernel image because gpl. Yep, there's a bit a contradiction there ...

I think what would work is loading the different subsystems of the
driver in parallel (we already do that largely), and then if one
firmware blob isn't there yet simply stall that async worker until it
shows up. But the answers I've gotten thus far from request_firmware()
folks (well at least Greg) is don't do that ...

Is that problem still somewhere on the radar? Atm there's various
wait_for_rootfs hacks out there floating in vendor/product trees.
"Avoid at all costs" sounds like upstream prefers to not care about
android/cros in those case (yes I know most arm socs don't need
firmware, which would make it a problem fo just be a subset of all
devices).
-Daniel

> I checked and verified all these are valid reports. This list also matches
> the same list as in 20150805, so we have fortunately not gotten worse.
> Let's keep it that way and also fix these drivers.
>
> v2: changes from v1 [0]:
>
> o This now supports iteration, this extends our coverage on the report
>
> o Update documentation and commit log to accept the fate of not being
>   able to remove the usermode helper.
>
> [0] 
> https://lkml.kernel.org/r/1440811107-861-1-git-send-email-mcg...@do-not-panic.com
>
> Cc: Alessandro Rubini 
> Cc: Kevin Cernekee 
> Cc: Daniel Vetter 
> Cc: Mimi Zohar 
> Cc: David Woodhouse 
> Cc: Kees Cook 
> Cc: Dmitry Torokhov 
> Cc: Ming Lei 
> Cc: Jonathan Corbet 
> Cc: Julia Lawall 
> Cc: Gilles Muller 
> Cc: Nicolas Palix 
> Cc: Thierry Martinez 
> Cc: Michal Marek 
> Cc: co...@systeme.lip6.fr
> Cc: Alessandro Rubini 
> Cc: Kevin Cernekee 
> Cc: Greg Kroah-Hartman 
> Cc: Jiri Slaby 
> Cc: linux-ser...@vger.kernel.org
> Cc: