Re: [PATCH] Enhanced support for MPC8xx/8xxx watchdog
Hi Wim, Le 27/02/2013 20:52, Wim Van Sebroeck a écrit : The rest of the code is OK and when above comments are corrected, I will add the patch to improve the userspace experience. Kind regards, Wim. Ok, I'll fix and re-submit my patch according to your comments. Best regards Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Unset LANG in setlocalversion script
Le 21/02/2013 22:14, Michal Marek a écrit : Dne 21.2.2013 13:49, Christophe Leroy napsal(a): This patch allows the use of setlocalversion script regardless of the LANG parameter. Otherwise, the `svn info 2>/dev/null | grep '^Last Changed Rev'` returns nothing because for instance, in French the text 'Last Changed Rev' is replaced by 'Révision de la derničre modification' Unsetting LANG might not be enough. You should unset LC_ALL and set LC_MESSAGES=C. But - is this a problem for other commands in the script? From a quick look, we only parse English output of svn, so I suggest to only set the env var for the svn command. Michal Ok, I will propose a new patch. I confirm my only issue at the time being is with the svn command. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hwmon/lm70: adding support for NS LM74 chip
Le 26/08/2012 19:47, Guenter Roeck a écrit : On Thu, Aug 23, 2012 at 11:13:17AM -0700, Guenter Roeck wrote: On Thu, Aug 23, 2012 at 05:32:12PM +0200, Christophe Leroy wrote: Hello, Hi Christophe, Hi again, [ ... ] Hi Guenter, - /* 3-wire link (shared SI/SO) for LM70 */ - if (chip == LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE)) + /* 3-wire link (shared SI/SO) for LM70 and LM74 */ + if ((chip == LM70_CHIP_LM70 || chip == LM70_CHIP_LM74) + && !(spi->mode & SPI_3WIRE)) Ok for now, but I'll have to check this. The driver does not really write anything to the chip, so it should be irrelevant which mode the SPI master controller supports (MOSI does not have to be connected). Besides, from the chip specification it looks like it is possible to connect the chip to a standard SPI interface by adding a 10k resistor between MOSI and MISO. After looking into the above, the proper change or fix is really to remove the above check for SPI_3WIRE entirely. The driver works perfectly fine if a LM70 is connected to a controller which does not explicitly support SPI_3WIRE; all that needs to be done is to connect MOSI through a resistor or not at all. This is, however, a board design decision, not a SPI master chip limitation. Ok, then I'll prepare a patch for this. With that, we don't need to explicitly add any LM74 specific code to the driver, since the TMP121 code does the trick. All we need to do is to update the documentation, adding a note that LM74 is supported as well, and that it can be selected with 'tmp121'. Yes, could be the solution, but I'm however a bit puzzled with doing this. The driver is originaly for NS LM70 component. Later on an extension has been added for TMP121 which is from a different family, even if compatible. Now we are talking about the LM74 which is in the same family as the original LM70. Therefore I would expect to be able to handle the LM74 directly from this driver, not through a tricky compatibility with the 'tmp121'. I would expect to handle it the other way round, ie consider the 'tmp121' as compatible with the LM74. One of the reasons for that is that the driver reports the component ID through /sys/class/hwmon/hwmonX/name As the 'tmp121' is already in the driver, it is not a good idea to replace it by LM74, however, I would prefer having the LM74 explicitly addressable by the driver, in additional to the 'tmp121' I'm preparing a patch to include support for both LM71 and LM74 in the LM70 driver, in order to have a driver handling the complete family. Do you see a problem for that ? It would look strange that the LM70 driver handles LM70 and LM71, but LM74 through the compatible 'tmp121' Nb, refer to your first answer, implementation for 'tmp122/124' is another story because those have a programmable precision, not a fixed one. Thanks, Guenter Thanks Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] lxt PHY: Support for the buggy LXT973 rev A2
Le 10/09/2012 20:17, Lutz Jaenicke a écrit : On Mon, Sep 10, 2012 at 05:45:49PM +0200, Christophe Leroy wrote: This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding precautions linked to ERRATA Item 4: Item 4: MDIO Interface and Repeated Polling Problem: Repeated polling of odd-numbered registers via the MDIO interface randomly returns the contents of the previous even register. Implication: Managed applications may not obtain the correct register contents when a particular register is monitored for device status. Workaround: None. Status: This erratum has been previously fixed (in rev A3) Hmm. Were did you get the hardware from? We have been using LXT973 in our products and the A2 was replaced by A3 in 2003. Best regards, Lutz They are custom boards that my company manufactures. Most boards manufactured before 2006 or 2007 have this buggy chip. Regards Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] lxt PHY: Support for the buggy LXT973 rev A2
Le 24/09/2012 16:13, David Laight a écrit : This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding precautions linked to ERRATA Item 4: Revision A2 of LXT973 chip randomly returns the contents of the previous even register when you read a odd register regularly Does reading the PHY registers involve bit-banging an MII interface? If so this code is likely to stall the system for significant periods (ready phy registers at all can be a problem). I know some ethernet mac have hardware blocks for reading MII and even polling one MII register for changes. Maybe some of this code ought to be using async software bit-bang - especially when just polling for link status change. I'm sure it ought to be possible to do one bit-bang action per clock tick instead of spinning for the required delays. David Not sure I understand what you mean. We have been using this code without any problem for about 2 years on our Hardware. It does almost same as genphy_read_status() except that it also reads the BMCR register (which is the register preceeding the BMSR) in order to detect the unlikely happening of the bug reported by the ERRATA. In case it happens (which is really seldom), it does a re-read. We are not spinning on any delays here. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] lxt PHY: Support for the buggy LXT973 rev A2
Le 24/09/2012 20:30, Richard Cochran a écrit :
On Mon, Sep 24, 2012 at 04:00:58PM +0200, Christophe Leroy wrote:
diff -u a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
--- a/drivers/net/phy/lxt.c 2012-09-23 03:08:48.0 +0200
+++ b/drivers/net/phy/lxt.c 2012-09-23 03:18:00.0 +0200
...
@@ -175,6 +292,16 @@
.driver = { .owner = THIS_MODULE,},
}, {
.phy_id = 0x00137a10,
+ .name = "LXT973-A2",
+ .phy_id_mask= 0x,
+ .features = PHY_BASIC_FEATURES,
+ .flags = 0,
+ .probe = lxt973_probe,
+ .config_aneg= lxt973_config_aneg,
+ .read_status= lxt973a2_read_status,
I like this way of matching the A2 chips much better than what you had
before. But are you sure this will work correctly?
Apparently it does.
What do A3 chips have in the last nibble of phy_id?
A2 chip has phy_id 0x00137a10
A3 chip has phy_id 0x00137a11
Christophe
+ .driver = { .owner = THIS_MODULE,},
+}, {
+ .phy_id = 0x00137a10,
.name = "LXT973",
.phy_id_mask= 0xfff0,
.features = PHY_BASIC_FEATURES,
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IIO AD7923 iio_consumer support
Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
On 10/05/2013 10:21 AM, Christophe Leroy wrote:
+ .consumer_channel = "channel_0",
+ .adc_channel_label = "0",
+ },
+ {
+ .consumer_dev_name = AD7923_NAME,
+ .consumer_channel = "channel_1",
+ .adc_channel_label = "1",
+ },
+ {
+ .consumer_dev_name = AD7923_NAME,
+ .consumer_channel = "channel_2",
+ .adc_channel_label = "2",
+ },
+ {
+ .consumer_dev_name = AD7923_NAME,
+ .consumer_channel = "channel_3",
+ .adc_channel_label = "3",
+ },
+ { }
+};
This is a mapping between channel names of the provider between the channel
names of the consumer. So it is specific to a certain combination of
consumer and provider and usually depend on how things are physically wired
on your board. As such there can be no generic mapping and this needs to go
into your machine/board driver. The mapping is usually passed to the IIO
driver via its platform data.
So e.g. imagine you have a provider like this driver and you have a consumer
that has a "voltage" channel. And on your board channel 3 of the ADC is what
you want to route to that consumer. Then your mapping would look like this:
{
.consumer_dev_name = "your_consumer_device.1",
.consumer_channel = "voltage",
.adc_channel_label = "AIN3",
}
And in your consumer driver you'd do:
channel = iio_channel_get(dev, "voltage");
Thanks for the explanation.
Can the mapping be retrieved via of_platform ?
Indeed, the only exemple I found was in the lp8788_adc driver, which
includes iio/machine.h and declares a default mapping, but it is based
on platform_data, not of_platform.
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] IIO AD7923 iio_consumer support
Le 05/10/2013 11:35, Lars-Peter Clausen a écrit :
On 10/05/2013 11:18 AM, leroy christophe wrote:
Le 05/10/2013 10:41, Lars-Peter Clausen a écrit :
On 10/05/2013 10:21 AM, Christophe Leroy wrote:
+.consumer_channel = "channel_0",
+.adc_channel_label = "0",
+},
+{
+.consumer_dev_name = AD7923_NAME,
+.consumer_channel = "channel_1",
+.adc_channel_label = "1",
+},
+{
+.consumer_dev_name = AD7923_NAME,
+.consumer_channel = "channel_2",
+.adc_channel_label = "2",
+},
+{
+.consumer_dev_name = AD7923_NAME,
+.consumer_channel = "channel_3",
+.adc_channel_label = "3",
+},
+{ }
+};
This is a mapping between channel names of the provider between the channel
names of the consumer. So it is specific to a certain combination of
consumer and provider and usually depend on how things are physically wired
on your board. As such there can be no generic mapping and this needs to go
into your machine/board driver. The mapping is usually passed to the IIO
driver via its platform data.
So e.g. imagine you have a provider like this driver and you have a consumer
that has a "voltage" channel. And on your board channel 3 of the ADC is what
you want to route to that consumer. Then your mapping would look like this:
{
.consumer_dev_name = "your_consumer_device.1",
.consumer_channel = "voltage",
.adc_channel_label = "AIN3",
}
And in your consumer driver you'd do:
channel = iio_channel_get(dev, "voltage");
Thanks for the explanation.
Can the mapping be retrieved via of_platform ?
Indeed, the only exemple I found was in the lp8788_adc driver, which
includes iio/machine.h and declares a default mapping, but it is based on
platform_data, not of_platform.
If you are using device tree you can specify the mapping inside the
devicetree. Have a look at
Documentation/devicetree/bindings/iio/iio-bindings.txt
Yes, I saw that. Is there anything that shall be done in the driver to
support it, or is it automatic ?
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc 8xx: Fixing issue with CONFIG_PIN_TLB
Le 16/09/2013 23:02, Scott Wood a écrit : On Fri, 2013-09-13 at 07:04 +0200, leroy christophe wrote: Le 12/09/2013 20:44, Scott Wood a écrit : On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote: This is a reorganisation of the setup of the TLB at kernel startup, in order to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866 and MPC885 reference manuals. Signed-off-by: Christophe Leroy diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S 2013-09-02 22:46:10.0 +0200 +++ linux-3.11/arch/powerpc/kernel/head_8xx.S 2013-09-09 11:28:54.0 +0200 @@ -785,27 +785,24 @@ * these mappings is mapped by page tables. */ initial_mmu: - tlbia /* Invalidate all TLB entries */ -/* Always pin the first 8 MB ITLB to prevent ITLB - misses while mucking around with SRR0/SRR1 in asm -*/ - lis r8, MI_RSV4I@h - ori r8, r8, 0x1c00 - + lis r8, MI_RESETVAL@h mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ -#ifdef CONFIG_PIN_TLB - lis r10, (MD_RSV4I | MD_RESETVAL)@h - ori r10, r10, 0x1c00 - mr r8, r10 -#else lis r10, MD_RESETVAL@h -#endif #ifndef CONFIG_8xx_COPYBACK orisr10, r10, MD_WTDEF@h #endif mtspr SPRN_MD_CTR, r10/* Set data TLB control */ + tlbia /* Invalidate all TLB entries */ Is this change to make sure we invalidate everything even if the bootloader set RSV4I? Most probably. It is step 2 of the process defined in MPC866 and MPC885 Reference Manuals: §8.10.3 Loading Locked TLB Entries: The process of loading a single reserved entry in the TLB is as follows: To minimize code churn we should just fix actual problems, rather than shuffle things around to conform to a suggested sequence. After all, we're not just trying to load a single entry. Ok, I'll try again. + ori r8, r8, 0x1c00 + mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ +#ifdef CONFIG_PIN_TLB + ori r10, r10, 0x1c00 + mtspr SPRN_MD_CTR, r10/* Set data TLB control */ +#endif Still 0x1c00? Yes, I kept the same entries in order to limit modifications: * 28 = First 8Mbytes page * 29 = IMMR * 30 = Second 8Mbytes page * 31 = Third 8Mbytes page If you actually want to program them in increasing order then it looks like you're still missing a write to CTR between the last two 8M entries -- thus you'll overwrite the IMMR with the last 8M entry. That was the same problem that v1 fixed -- did that change get lost accidentally? Oops, no, in fact I diffed from the version which was including it already. My mistake. The hardware wants to decrement; why fight it? I see your point. However it is not clear in the documentation if the decrement is done really after the update, or at xTLB interrupt. So I propose to still set the CTR ourself as described in the reference Manual and not assume that the HW decrements it. /* Now map the lower 8 Meg into the TLBs. For this quick hack, * we can load the instruction and data TLB registers with the * same values. @@ -825,6 +822,12 @@ mtspr SPRN_MI_AP, r8 mtspr SPRN_MD_AP, r8 + /* Always pin the first 8 MB ITLB to prevent ITLB +* misses while mucking around with SRR0/SRR1 in asm +*/ + lis r8, (MI_RSV4I | MI_RESETVAL)@h + mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ Entry 0 is not pinnable. Here we are not trying to pin entry 0. Sorry, misread the patch. We are at step 8, we are setting MI_RSV4I. At the same time, we set MD_CTR to 0 which is off the pinned range, to be sure that we won't overwrite one of the pinned entries. The main difference compared to the previous implementation is that before, we were setting the RSV4I bit before loading the TLB entries. Now, as defined in the Reference Manuals, we are doing it at the end. Have you seen any evidence that it matters? Not really. Ok, propose a new patch in a few minutes. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc 8xx: Fixing issue with CONFIG_PIN_TLB
Le 12/09/2013 02:15, Benjamin Herrenschmidt a écrit : On Wed, 2013-09-11 at 17:36 -0500, Scott Wood wrote: I wonder why we don't start from entry 31 so we can actually make use of that autodecrement. What will happen when we load the first normal TLB entry later on? I don't see any setting of SPRN_MD_CTR after this code, so won't it overwrite entry 30 (the middle 8M) in the CONFIG_PIN_TLB case? Ben, would patches like this be considered bugfixes as far as merging goes, or would they be for next given that it's something that's never really worked right and hasn't been touched in years? Since they don't affect anything outside of 8xx, I'm happy to take them until around -rc2 or 3. But it's your call really. Scott, you're right, I didn't see that other consequence. I'll come with a more complete patch this afternoon. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc 8xx: Fixing issue with CONFIG_PIN_TLB
Le 12/09/2013 20:44, Scott Wood a écrit : On Thu, 2013-09-12 at 20:25 +0200, Christophe Leroy wrote: This is a reorganisation of the setup of the TLB at kernel startup, in order to handle the CONFIG_PIN_TLB case in accordance with chapter 8.10.3 of MPC866 and MPC885 reference manuals. Signed-off-by: Christophe Leroy diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S --- linux-3.11.org/arch/powerpc/kernel/head_8xx.S 2013-09-02 22:46:10.0 +0200 +++ linux-3.11/arch/powerpc/kernel/head_8xx.S 2013-09-09 11:28:54.0 +0200 @@ -785,27 +785,24 @@ * these mappings is mapped by page tables. */ initial_mmu: - tlbia /* Invalidate all TLB entries */ -/* Always pin the first 8 MB ITLB to prevent ITLB - misses while mucking around with SRR0/SRR1 in asm -*/ - lis r8, MI_RSV4I@h - ori r8, r8, 0x1c00 - + lis r8, MI_RESETVAL@h mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ -#ifdef CONFIG_PIN_TLB - lis r10, (MD_RSV4I | MD_RESETVAL)@h - ori r10, r10, 0x1c00 - mr r8, r10 -#else lis r10, MD_RESETVAL@h -#endif #ifndef CONFIG_8xx_COPYBACK orisr10, r10, MD_WTDEF@h #endif mtspr SPRN_MD_CTR, r10/* Set data TLB control */ + tlbia /* Invalidate all TLB entries */ Is this change to make sure we invalidate everything even if the bootloader set RSV4I? Most probably. It is step 2 of the process defined in MPC866 and MPC885 Reference Manuals: §8.10.3 Loading Locked TLB Entries: The process of loading a single reserved entry in the TLB is as follows: 1. Disable the TLB by clearing MSR[IR] or MSR[DR] as needed. 2. Clear MI_CTR[RSV4I] (MD_CTR[RSV4D]). 3. Invalidate the EA of the reserved page by using tlbia or tlbie. 4. Set MI_CTR[ITLB_INDX] (MD_CTR[DTLB_INDX]) to the appropriate value (between 27 and 31). 5. Load Mx_EPN with the effective page number, the ASID of the reserved page, and set EV. 6. Run software tablewalk code to load the appropriate entry into the translation lookaside buffer. See Section 8.10.1.1, “Translation Reload Examples.” 7. Repeat steps 4–6 to load other TLB entries. 8. Set MI_CTR[RSV4I] (MD_CTR[RSV4D]) + ori r8, r8, 0x1c00 + mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ +#ifdef CONFIG_PIN_TLB + ori r10, r10, 0x1c00 + mtspr SPRN_MD_CTR, r10/* Set data TLB control */ +#endif Still 0x1c00? Yes, I kept the same entries in order to limit modifications: * 28 = First 8Mbytes page * 29 = IMMR * 30 = Second 8Mbytes page * 31 = Third 8Mbytes page /* Now map the lower 8 Meg into the TLBs. For this quick hack, * we can load the instruction and data TLB registers with the * same values. @@ -825,6 +822,12 @@ mtspr SPRN_MI_AP, r8 mtspr SPRN_MD_AP, r8 + /* Always pin the first 8 MB ITLB to prevent ITLB +* misses while mucking around with SRR0/SRR1 in asm +*/ + lis r8, (MI_RSV4I | MI_RESETVAL)@h + mtspr SPRN_MI_CTR, r8 /* Set instruction MMU control */ Entry 0 is not pinnable. Here we are not trying to pin entry 0. We are at step 8, we are setting MI_RSV4I. At the same time, we set MD_CTR to 0 which is off the pinned range, to be sure that we won't overwrite one of the pinned entries. The main difference compared to the previous implementation is that before, we were setting the RSV4I bit before loading the TLB entries. Now, as defined in the Reference Manuals, we are doing it at the end. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc 8xx: Fixing issue with CONFIG_PIN_TLB
Le 20/09/2013 23:22, Scott Wood a écrit : The hardware wants to decrement; why fight it? >I see your point. >However it is not clear in the documentation if the decrement is done >really after the update, or at xTLB interrupt. So I propose to still set >the CTR ourself as described in the reference Manual and not assume that >the HW decrements it. It says "every update" -- do you have any reason to believe that's wrong? It could be tested... Ok. I just test it, and I observe the following: As we have set the RSV4x bit, the CPU sets Mx_CTR to a value below 0x1c after each update: * After writing entry 0x1c, Mx_CTR has value 0x1b * After writing entry 0x1d, Mx_CTR has value 0x18 * After writing entry 0x1e, Mx_CTR has value 0x19 * After writing entry 0x1f, Mx_CTR has value 0x1a Indeed the first version of my patch was complete, only the description was not fully correct. So, in order to minimise code churn, I will re-submit my initial patch with a modified description. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
Le 16/08/2012 17:21, Alan Cox a écrit : MAX_IDL: Maximum idle characters. When a character is received, the receiver begins counting idle characters. If MAX_IDL idle characters are received before the next data character, an idle timeout occurs and the buffer is closed, generating a maskable interrupt request to the core to receive the data from the buffer. Thus, MAX_IDL offers a way to demarcate frames. To disable the feature, clear MAX_IDL. The bit length of an idle character is calculated as follows: 1 + data length (5–9) + 1 (if parity is used) + number of stop bits (1–2). For 8 data bits, no parity, and 1 stop bit, the character length is 10 bits So if you have slightly bursty high speed data as its quite typical before your change you would get one interrupt per buffer of 32 bytes, with it you'll get a lot more interrupts. You have two available hints about the way to set this - one of them is the baud rate (low baud rates mean the fifo isn't a big win and the latency is high), the other is the low_latency flag if the driver supports the low latency feature (and arguably you can still use a request for it as a hint even if you refuse the actual feature). So I think a reasonable approach would be set the idle timeout down for low baud rates or if low_latency is requested. generated if there is at least one word in the FIFO and for a time equivalent to the transmission of four characters Which is a bit more reasonable than one, although problematic at low speed (hence the fifo on/off). What would then thing about: * a value of 1 for all rates below 2400 (On 8250, fifo is set to 1 for such rates) * a value of 2 for 2400 and 4800 * a value of 4 for 9600 (which is the default on the 8250 for all rates above 2400) * a value of 8 for 19200 * a value of 16 for 38400 and above (on UCC_UART, maxidl is set to 16, never 32) Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2] Enhanced support for MPC8xx/8xxx watchdog
Le 26/06/2013 01:04, Scott Wood a écrit :
On Thu, Feb 28, 2013 at 09:52:22AM +0100, LEROY Christophe wrote:
This patch modifies the behaviour of the MPC8xx/8xxx watchdog. On the MPC8xx,
at 133Mhz, the maximum timeout of the watchdog timer is 1s, which means it must
be pinged twice a second. This is not in line with the Linux watchdog concept
which is based on a default watchdog timeout around 60s.
This patch introduces an intermediate layer between the CPU and the userspace.
The kernel pings the watchdog at the required frequency at the condition that
userspace tools refresh it regularly.
Existing parameter 'timeout' is renamed 'hw_time'.
The new parameter 'timeout' allows to set up the userspace timeout.
The driver also implements the WDIOC_SETTIMEOUT ioctl.
Signed-off-by: Christophe Leroy
diff -ur linux-3.7.9/drivers/watchdog/mpc8xxx_wdt.c
linux/drivers/watchdog/mpc8xxx_wdt.c
--- linux-3.7.9/drivers/watchdog/mpc8xxx_wdt.c 2013-02-17 19:53:32.0
+0100
+++ linux/drivers/watchdog/mpc8xxx_wdt.c2013-02-27 16:00:07.0
+0100
@@ -52,10 +52,17 @@
static struct mpc8xxx_wdt __iomem *wd_base;
static int mpc8xxx_wdt_init_late(void);
-static u16 timeout = 0x;
-module_param(timeout, ushort, 0);
+#define WD_TIMO 10 /* Default timeout = 10 seconds */
If the default Linux watchdog timeout is normally 60 seconds, why is it 10
here?
Looks like each driver has its own default value, but agreed, I change
it to 60 seconds
+static uint timeout = WD_TIMO;
+module_param(timeout, uint, 0);
MODULE_PARM_DESC(timeout,
- "Watchdog timeout in ticks. (0
hw_timeout would be more legibile -- this is a public interface.
Ok
static bool reset = 1;
module_param(reset, bool, 0);
@@ -72,10 +79,12 @@
* to 0
*/
static int prescale = 1;
-static unsigned int timeout_sec;
+static unsigned int hw_timo_sec;
+static int wdt_auto = 1;
bool, and add a comment indicating what this means.
Ok
static unsigned long wdt_is_open;
static DEFINE_SPINLOCK(wdt_spinlock);
+static unsigned long wdt_last_ping;
static void mpc8xxx_wdt_keepalive(void)
{
@@ -91,9 +100,20 @@
static void mpc8xxx_wdt_timer_ping(unsigned long arg)
{
- mpc8xxx_wdt_keepalive();
- /* We're pinging it twice faster than needed, just to be sure. */
- mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
+ if (wdt_auto)
+ wdt_last_ping = jiffies;
+
+ if (jiffies - wdt_last_ping <= timeout * HZ) {
So timeout cannot be more than UINT_MAX / HZ... Might want to check for
that, just in case.
Ok.
What happens if there's a race? If another CPU updates wdt_last_ping in
parallel, then you could see wdt_last_ping greater than the value you
read for jiffies. Since this is an unsigned comparison, it will fail to
call keepalive. You might get saved by pinging it twice as often as
necessary, but you shouldn't rely on that.
Euh ... This watchdog is integrated inside the CPU, so there is no
chance that any external CPU get access to it.
+ mpc8xxx_wdt_keepalive();
+ /* We're pinging it twice faster than needed, to be sure. */
+ mod_timer(&wdt_timer, jiffies + HZ * hw_timo_sec / 2);
+ }
+}
+
+static void mpc8xxx_wdt_sw_keepalive(void)
+{
+ wdt_last_ping = jiffies;
+ mpc8xxx_wdt_timer_ping(0);
}
This isn't new with this patch, but it looks like
mpc8xxx_wdt_keepalive() can be called either from timer context, or with
interrupts enabled... yet it uses a bare spin_lock() rather than an
irq-safe version. This should be fixed.
Ok, I'll propose another patch for that. Indeed, is the spin_lock needed
at all ? If we get two writes interleaved, it will make it anyway.
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Adds support for Open Firmware in MAX730x GPIO Driver
Le 01/03/2013 01:43, Linus Walleij a écrit : On Fri, Feb 22, 2013 at 10:26 AM, Christophe Leroy wrote: This patch allows the use of the MAX730x Driver on systems using the Open Firmware platform format Signed-off-by: Patrick Vasseur Signed-off-by: Christophe Leroy (...) /* bits_per_word cannot be configured in platform data */ - spi->bits_per_word = 16; + if (spi->dev.platform_data) + spi->bits_per_word = 16; What about just fixing so you *can* specify that instead? The comment looks more like a FIXME to me. Euh, ok, why not. But here the purpose of my patch is to allow using this driver with of_platform in addition to platform. This FIXME is not mine, it was already existing in that driver. As of_platform can configure bits per word, the only thing I did is to add a test in order to not apply this FIXME on the of_platform case. Do you think my patch is not acceptable like this ? Regards Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Adds support for Open Firmware in MAX730x GPIO Driver
Le 02/03/2013 22:16, Grant Likely a écrit : I would like to know /why/ this specific hunk is necessary. I cannot tell from the context. That's the sort of thing that is very helpful to have in the commit description. Otherwise the patch looks fine. g. Ok, I will resubmit with a note to it in the commit description. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Huge performance degradation for UDP between 2.4.17 and 2.6
Hi I'm having a big issue with UDP. Using a powerpc board (MPC860). With our board running kernel 2.4.17, I'm able to send 16 voice packets (UDP, 96 bytes per packet) in 11 seconds. With the same board running either Kernel 2.6.35.14 or Kernel 3.4.7, I need 55 seconds to send the same amount of packets. Is there anything to tune in order to get same output rate as with Kernel 2.4 ? Regards Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Huge performance degradation for UDP between 2.4.17 and 2.6
Le 02/08/2012 16:13, Eric Dumazet a écrit : On Thu, 2012-08-02 at 14:27 +0200, leroy christophe wrote: Hi I'm having a big issue with UDP. Using a powerpc board (MPC860). With our board running kernel 2.4.17, I'm able to send 16 voice packets (UDP, 96 bytes per packet) in 11 seconds. With the same board running either Kernel 2.6.35.14 or Kernel 3.4.7, I need 55 seconds to send the same amount of packets. Is there anything to tune in order to get same output rate as with Kernel 2.4 ? kernel size is probably too big for your old / slow cpu. Maybe you added too many features on your 3.4.7 kernel. (netfilter ? SLUB debugging ...) Its hard to say, 2.4.17 had less features and was faster. Thanks for your answer. Yes I have netfilter as I need it. However, I tried without it and still need about 37 seconds to send the 16 packets I was sending in 11 seconds with 2.4.17 I don't think there is any problem with size of the kernel. I still have plenty of memory available. All debugging is turned off, and I'm not using SLUB but SLOB. I have 32Mbytes of RAM. Would SLUB be more performant than SLOB ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Huge performance degradation for UDP between 2.4.17 and 2.6
Le 05/08/2012 10:28, Eric Dumazet a écrit : On Sun, 2012-08-05 at 10:16 +0200, LEROY christophe wrote: Le 02/08/2012 16:13, Eric Dumazet a écrit : On Thu, 2012-08-02 at 14:27 +0200, leroy christophe wrote: Hi I'm having a big issue with UDP. Using a powerpc board (MPC860). With our board running kernel 2.4.17, I'm able to send 16 voice packets (UDP, 96 bytes per packet) in 11 seconds. With the same board running either Kernel 2.6.35.14 or Kernel 3.4.7, I need 55 seconds to send the same amount of packets. Is there anything to tune in order to get same output rate as with Kernel 2.4 ? kernel size is probably too big for your old / slow cpu. Maybe you added too many features on your 3.4.7 kernel. (netfilter ? SLUB debugging ...) Its hard to say, 2.4.17 had less features and was faster. Thanks for your answer. Yes I have netfilter as I need it. However, I tried without it and still need about 37 seconds to send the 16 packets I was sending in 11 seconds with 2.4.17 I don't think there is any problem with size of the kernel. I still have plenty of memory available. I believe you misunderstood me. I was referring to cpu caches ( dcache & icache ) All debugging is turned off, and I'm not using SLUB but SLOB. I have 32Mbytes of RAM. Would SLUB be more performant than SLOB ? I never used SLOB I cannot comment Please provide (on 3.4.7) cat /proc/cpuinfo lsmod dmesg Ok, I have recompiled with SLUB. Find attached cpuinfo, lsmod and dmesg. I do not have any modules loaded. Module Size Used byNot tainted processor : 0 cpu : 8xx clock : 132.00MHz revision: 0.0 (pvr 0050 ) bogomips: 16.50 timebase: 825 platform: CMPC885 model : MIAE Memory : 128 MB [0.00] Using CMPC885 machine description [0.00] Linux version 3.4.7-s3k-3.8.3+-svn2796 (root@localhost.localdomain) (gcc version 4.4.4 (GCC) ) #19 PREEMPT Sun Aug 5 06:43:31 CEST 2012 [0.00] Top of RAM: 0x800, Total RAM: 0x800 [0.00] Memory hole size: 0MB [0.00] Zone PFN ranges: [0.00] DMA 0x -> 0x8000 [0.00] Normal empty [0.00] Movable zone start PFN for each node [0.00] Early memory PFN ranges [0.00] 0: 0x -> 0x8000 [0.00] On node 0 totalpages: 32768 [0.00] free_area_init_node: node 0, pgdat c03f4be8, node_mem_map c0433000 [0.00] DMA zone: 256 pages used for memmap [0.00] DMA zone: 0 pages reserved [0.00] DMA zone: 32512 pages, LIFO batch:7 [0.00] MMU: Allocated 72 bytes of context maps for 16 contexts [0.00] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 32512 [0.00] Kernel command line: console=ttyCPM0,115200N8 ip=172.25.231.39:172.25.231.59::255.0.0.0:miae:eth0:off [0.00] PID hash table entries: 512 (order: -1, 2048 bytes) [0.00] Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) [0.00] Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) [0.00] Memory: 125432k/131072k available (3972k kernel code, 5640k reserved, 140k data, 178k bss, 488k init) [0.00] Kernel virtual memory layout: [0.00] * 0xfffdf000..0xf000 : fixmap [0.00] * 0xfde0..0xfe00 : consistent mem [0.00] * 0xfddf6000..0xfde0 : early ioremap [0.00] * 0xc900..0xfddf6000 : vmalloc & ioremap [0.00] SLUB: Genslabs=14, HWalign=16, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 [0.00] NR_IRQS:512 nr_irqs:512 16 [0.00] Decrementer Frequency = 0x7de290 [0.00] time_init: decrementer frequency = 8.25 MHz [0.00] time_init: processor frequency = 132.00 MHz [0.00] clocksource: timebase mult[79364d93] shift[24] registered [0.00] clockevent: decrementer mult[21cac08] shift[32] cpu[0] [0.00] console [ttyCPM0] enabled [0.144102] pid_max: default: 32768 minimum: 301 [0.149489] Mount-cache hash table entries: 512 [0.175086] devtmpfs: initialized [0.178575] device: 'platform': device_add [0.179079] bus: 'platform': registered [0.179451] bus: 'cpu': registered [0.179551] device: 'cpu': device_add [0.181718] NET: Registered protocol family 16 [0.186374] device class 'bdi': registering [0.188054] device class 'gpio': registering [0.189423] device class 'tty': registering [0.191855] bus: 'spi': registered [0.191932] device class 'spi_master': registering [0.193842] gpiochip_find_base: found new base at 1008 [0.193974] device: 'gpiochip1008': device_add [0.195387] gpiochip_add: regist
Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
Le 14/08/2012 16:52, Alan Cox a écrit : On Tue, 14 Aug 2012 16:26:28 +0200 Christophe Leroy wrote: Hello, I'm not sure who to address this Patch to either It fixes a delay issue with CPM UART driver on Powerpc MPC8xx. The problem is that with the actual code, the driver waits 32 IDLE patterns before returning the received data to the upper level. It means for instance about 1 second at 300 bauds. This fix limits to one byte the waiting period. Take a look how the 8250 does it - I think you want to set the value based upon the data rate. Your patch will break it for everyone doing high seed I/O. Alan I'm not sure I understand what you mean. As far as I can see 8250/16550 is working a bit different, as it is based on a fifo and triggers an interrupt as soon as a given number of bytes is received. I also see that in case this amount is not reached, there is a receive-timeout which goes on after no byte is received for a duration of more than 4 bytes. The PowerPC CPM is working differently. It doesn't use a fifo but buffers. Buffers are handed to the microprocessor only when they are full or after a timeout period which is adjustable. In the driver, the buffers are configured with a size of 32 bytes. And the timeout is set to the size of the buffer. That is this timeout that I'm reducing to 1 byte in my proposed patch. I can't see what it would break for high speed I/O. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Powerpc 8xx CPM_UART delay in receive
Le 16/08/2012 16:29, Alan Cox a écrit : The PowerPC CPM is working differently. It doesn't use a fifo but buffers. Buffers are handed to the microprocessor only when they are full or after a timeout period which is adjustable. In the driver, the Which is different how - remembering we empty the FIFO on an IRQ buffers are configured with a size of 32 bytes. And the timeout is set to the size of the buffer. That is this timeout that I'm reducing to 1 byte in my proposed patch. I can't see what it would break for high speed I/O. How can a timeout be measured in "bytes". Can we have a bit more clarity on how the hardware works and take it from there ? Alan The reference manual of MPC885 says the following about the MAX_IDL parameter: MAX_IDL: Maximum idle characters. When a character is received, the receiver begins counting idle characters. If MAX_IDL idle characters are received before the next data character, an idle timeout occurs and the buffer is closed, generating a maskable interrupt request to the core to receive the data from the buffer. Thus, MAX_IDL offers a way to demarcate frames. To disable the feature, clear MAX_IDL. The bit length of an idle character is calculated as follows: 1 + data length (5–9) + 1 (if parity is used) + number of stop bits (1–2). For 8 data bits, no parity, and 1 stop bit, the character length is 10 bits If the UART is receiving data and gets an idle character (all ones), the channel begins counting consecutive idle characters received. If MAX_IDL is reached, the buffer is closed and an RX interrupt is generated if not masked. If no buffer is open, this event does not generate an interrupt or any status information. The internal idle counter (IDLC) is reset every time a character is received. To disable the idle sequence function, clear MAX_IDL The datasheet of the 16550 UART says: Besides, for FIFO mode operation a time out mechanism is implemented. Independently of the trigger level of the FIFO, an interrupt will be generated if there is at least one word in the FIFO and for a time equivalent to the transmission of four characters - no new character has been received and - the microprocessor has not read the RHR To compute the time out, the current total number of bits (start, data, parity and stop(s)) is used, together with the current baud rate (i.e., it depends on the contents of the LCR, DLL, DLM and PSD registers). Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] IIO ADC support for AD7923
Le 12/02/2013 19:54, Lars-Peter Clausen a écrit : On 02/12/2013 06:10 PM, Christophe Leroy wrote: This patch adds support for Analog Devices AD7923 ADC in the IIO Subsystem. Signed-off-by: Patrick Vasseur Signed-off-by: Christophe Leroy Looks good to me except for one small, but important issue. Oh, many thanks. Good points . I'll fix it and resubmit. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/7] powerpc/traps: Return early in show_signal_msg()
Murilo Opsfelder Araujo a écrit :
Modify logic of show_signal_msg() to return early, if possible. Replace
printk_ratelimited() by printk() and a default rate limit burst to limit
displaying unhandled signals messages.
Can you explain more the benefits of this change ?
Christophe
Signed-off-by: Murilo Opsfelder Araujo
---
arch/powerpc/kernel/traps.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index cbd3dc365193..4faab4705774 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -301,6 +301,13 @@ void user_single_step_siginfo(struct task_struct *tsk,
info->si_addr = (void __user *)regs->nip;
}
+static bool show_unhandled_signals_ratelimited(void)
+{
+ static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
+ DEFAULT_RATELIMIT_BURST);
+ return show_unhandled_signals && __ratelimit(&rs);
+}
+
static void show_signal_msg(int signr, struct pt_regs *regs, int code,
unsigned long addr)
{
@@ -309,11 +316,12 @@ static void show_signal_msg(int signr, struct
pt_regs *regs, int code,
const char fmt64[] = KERN_INFO "%s[%d]: unhandled signal %d " \
"at %016lx nip %016lx lr %016lx code %x\n";
- if (show_unhandled_signals && unhandled_signal(current, signr)) {
- printk_ratelimited(regs->msr & MSR_64BIT ? fmt64 : fmt32,
- current->comm, current->pid, signr,
- addr, regs->nip, regs->link, code);
- }
+ if (!unhandled_signal(current, signr))
+ return;
+
+ printk(regs->msr & MSR_64BIT ? fmt64 : fmt32,
+ current->comm, current->pid, signr,
+ addr, regs->nip, regs->link, code);
}
void _exception_pkey(int signr, struct pt_regs *regs, int code,
@@ -326,7 +334,8 @@ void _exception_pkey(int signr, struct pt_regs
*regs, int code,
return;
}
- show_signal_msg(signr, regs, code, addr);
+ if (show_unhandled_signals_ratelimited())
+ show_signal_msg(signr, regs, code, addr);
if (arch_irqs_disabled() && !arch_irq_disabled_regs(regs))
local_irq_enable();
--
2.17.1
Re: [PATCH 6/7] powerpc/traps: Print signal name for unhandled signals
Murilo Opsfelder Araujo a écrit :
This adds a human-readable name in the unhandled signal message.
Before this patch, a page fault looked like:
Jul 11 16:04:11 localhost kernel: pandafault[6303]: unhandled
signal 11 at 17d0 nip 161c lr
7fff93c55100 code 2 in pandafault[1000+1]
After this patch, a page fault looks like:
Jul 11 18:14:48 localhost kernel: pandafault[6352]: segfault
(11) at 00013a2a09f8 nip 00013a2a086c lr 7fffb63e5100
code 2 in pandafault[13a2a+1]
Signed-off-by: Murilo Opsfelder Araujo
---
arch/powerpc/kernel/traps.c | 43 +
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index e6c43ef9fb50..e55ee639d010 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -96,6 +96,41 @@ EXPORT_SYMBOL(__debugger_fault_handler);
#define TM_DEBUG(x...) do { } while(0)
#endif
+static const char *signames[SIGRTMIN + 1] = {
+ "UNKNOWN",
+ "SIGHUP", // 1
+ "SIGINT", // 2
+ "SIGQUIT",// 3
+ "SIGILL", // 4
+ "unhandled trap", // 5 = SIGTRAP
+ "SIGABRT",// 6 = SIGIOT
+ "bus error", // 7 = SIGBUS
+ "floating point exception", // 8 = SIGFPE
+ "illegal instruction",// 9 = SIGILL
+ "SIGUSR1",// 10
+ "segfault", // 11 = SIGSEGV
+ "SIGUSR2",// 12
+ "SIGPIPE",// 13
+ "SIGALRM",// 14
+ "SIGTERM",// 15
+ "SIGSTKFLT", // 16
+ "SIGCHLD",// 17
+ "SIGCONT",// 18
+ "SIGSTOP",// 19
+ "SIGTSTP",// 20
+ "SIGTTIN",// 21
+ "SIGTTOU",// 22
+ "SIGURG", // 23
+ "SIGXCPU",// 24
+ "SIGXFSZ",// 25
+ "SIGVTALRM", // 26
+ "SIGPROF",// 27
+ "SIGWINCH", // 28
+ "SIGIO", // 29 = SIGPOLL = SIGLOST
+ "SIGPWR", // 30
+ "SIGSYS", // 31 = SIGUNUSED
+};
+
/*
* Trap & Exception support
*/
@@ -314,10 +349,10 @@ static void show_signal_msg(int signr, struct
pt_regs *regs, int code,
if (!unhandled_signal(current, signr))
return;
- pr_info("%s[%d]: unhandled signal %d at "REG_FMT \
- " nip "REG_FMT" lr "REG_FMT" code %x",
- current->comm, current->pid, signr, addr,
- regs->nip, regs->link, code);
+ pr_info("%s[%d]: %s (%d) at "REG_FMT" nip "REG_FMT \
+ " lr "REG_FMT" code %x",
+ current->comm, current->pid, signames[signr],
+ signr, addr, regs->nip, regs->link, code);
Are we sure that signr is always within the limits of the table ?
Christophe
print_vma_addr(KERN_CONT " in ", regs->nip);
--
2.17.1
Re: [PATCH 7/7] powerpc/traps: Show instructions on exceptions
Murilo Opsfelder Araujo a écrit :
Move show_instructions() declaration to arch/powerpc/include/asm/stacktrace.h
and include asm/stracktrace.h in arch/powerpc/kernel/process.c,
which contains
the implementation.
Modify show_instructions() not to call __kernel_text_address(), allowing
userspace instruction dump. probe_kernel_address(), which returns -EFAULT if
something goes wrong, is still being called.
Call show_instructions() in arch/powerpc/kernel/traps.c to dump
instructions at
faulty location, useful to debugging.
Shouldn't this part be in a second patch ?
Wouldn't it be better to also see regs in addition if we want to use
this to understand what happened ?
So you could call show_regs() instead of show_instructions() ?
Christophe
Before this patch, an unhandled signal message looked like:
Jul 24 09:57:00 localhost kernel: pandafault[10524]: segfault
(11) at 17d0 nip 161c lr 7fffbd295100
code 2 in pandafault[1000+1]
After this patch, it looks like:
Jul 24 09:57:00 localhost kernel: pandafault[10524]: segfault
(11) at 17d0 nip 161c lr 7fffbd295100
code 2 in pandafault[1000+1]
Jul 24 09:57:00 localhost kernel: Instruction dump:
Jul 24 09:57:00 localhost kernel: 4bfffeec 4bfffee8 3c401002
38427f00 fbe1fff8 f821ffc1 7c3f0b78 3d22fffe
Jul 24 09:57:00 localhost kernel: 392988d0 f93f0020 e93f0020
39400048 <9949> 3920 7d234b78 383f0040
Signed-off-by: Murilo Opsfelder Araujo
---
arch/powerpc/include/asm/stacktrace.h | 7 +++
arch/powerpc/kernel/process.c | 6 +++---
arch/powerpc/kernel/traps.c | 3 +++
3 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/include/asm/stacktrace.h
diff --git a/arch/powerpc/include/asm/stacktrace.h
b/arch/powerpc/include/asm/stacktrace.h
new file mode 100644
index ..46e5ef451578
--- /dev/null
+++ b/arch/powerpc/include/asm/stacktrace.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_POWERPC_STACKTRACE_H
+#define _ASM_POWERPC_STACKTRACE_H
+
+void show_instructions(struct pt_regs *regs);
+
+#endif /* _ASM_POWERPC_STACKTRACE_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index b1af3390249c..ee1d63e03c52 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1261,7 +1262,7 @@ struct task_struct *__switch_to(struct
task_struct *prev,
static int instructions_to_print = 16;
-static void show_instructions(struct pt_regs *regs)
+void show_instructions(struct pt_regs *regs)
{
int i;
unsigned long pc = regs->nip - (instructions_to_print * 3 / 4 *
@@ -1283,8 +1284,7 @@ static void show_instructions(struct pt_regs *regs)
pc = (unsigned long)phys_to_virt(pc);
#endif
- if (!__kernel_text_address(pc) ||
-probe_kernel_address((unsigned int __user *)pc, instr)) {
+ if (probe_kernel_address((unsigned int __user *)pc, instr)) {
pr_cont(" ");
} else {
if (regs->nip == pc)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index e55ee639d010..3beca17ac1b1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -70,6 +70,7 @@
#include
#include
#include
+#include
#if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC_CORE)
int (*__debugger)(struct pt_regs *regs) __read_mostly;
@@ -357,6 +358,8 @@ static void show_signal_msg(int signr, struct
pt_regs *regs, int code,
print_vma_addr(KERN_CONT " in ", regs->nip);
pr_cont("\n");
+
+ show_instructions(regs);
}
void _exception_pkey(int signr, struct pt_regs *regs, int code,
--
2.17.1
Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
Mathieu Malaterre a écrit : On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy wrote: Le 22/03/2018 à 21:20, Mathieu Malaterre a écrit : Some functions prototypes were missing for the non-altivec code. Add the missing prototypes directly in xor_vmx, fix warnings treated as errors with W=1: arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototype for ‘xor_altivec_2’ [-Werror=missing-prototypes] arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototype for ‘xor_altivec_3’ [-Werror=missing-prototypes] arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototype for ‘xor_altivec_4’ [-Werror=missing-prototypes] arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototype for ‘xor_altivec_5’ [-Werror=missing-prototypes] Signed-off-by: Mathieu Malaterre --- arch/powerpc/lib/xor_vmx.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx.h index 5c2b0839b179..2173e3c84151 100644 --- a/arch/powerpc/lib/xor_vmx.h +++ b/arch/powerpc/lib/xor_vmx.h @@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsigned long *v1_in, void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in, unsigned long *v2_in, unsigned long *v3_in, unsigned long *v4_in, unsigned long *v5_in); + +void xor_altivec_2(unsigned long bytes, unsigned long *v1_in, +unsigned long *v2_in); + Only used in one place, should be static instead of adding it in a .h Same for the other ones. $ git grep xor_altivec_2 [...] arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2); Are you sure I can change this function to static ? Yes you are right. But in fact those fonctions are already defined in asm/xor. h So you just need to add the missing #include Christophe Christophe +void xor_altivec_3(unsigned long bytes, unsigned long *v1_in, +unsigned long *v2_in, unsigned long *v3_in); + +void xor_altivec_4(unsigned long bytes, unsigned long *v1_in, +unsigned long *v2_in, unsigned long *v3_in, +unsigned long *v4_in); + +void xor_altivec_5(unsigned long bytes, unsigned long *v1_in, +unsigned long *v2_in, unsigned long *v3_in, +unsigned long *v4_in, unsigned long *v5_in); --- L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. https://www.avast.com/antivirus
Re: [PATCH 01/16] initrd: Add generic code path for common initrd unloading logic.
Shea Levy a écrit :
Signed-off-by: Shea Levy
---
init/initramfs.c | 7 +++
usr/Kconfig | 4
2 files changed, 11 insertions(+)
diff --git a/init/initramfs.c b/init/initramfs.c
index 7e99a0038942..de5ce873eb5a 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -526,6 +526,13 @@ extern unsigned long __initramfs_size;
#include
#include
+#ifdef CONFIG_INITRAMFS_GENERIC_UNLOAD
+void free_initrd_mem(unsigned long start, unsigned long end)
+{
+ free_reserved_area((void *)start, (void *)end, -1, "initrd");
+}
+#endif
In powerpc this was an __init function. Why not also put the generic
one in __init section ?
Christophe
+
static void __init free_initrd(void)
{
#ifdef CONFIG_KEXEC_CORE
diff --git a/usr/Kconfig b/usr/Kconfig
index 43658b8a975e..fd79d4d6fa26 100644
--- a/usr/Kconfig
+++ b/usr/Kconfig
@@ -233,3 +233,7 @@ config INITRAMFS_COMPRESSION
default ".lzma" if RD_LZMA
default ".bz2" if RD_BZIP2
default ""
+
+# Arches can select this for a generic initrd unloading codepath
+config INITRAMFS_GENERIC_UNLOAD
+ bool
--
2.16.2
Re: [PATCH v4 03/19] powerpc: Mark variable `l` as unused, remove `path`
Mathieu Malaterre a écrit :
Add gcc attribute unused for `l` variable, replace `path` variable directly
with prom_scratch. Fix warnings treated as errors with W=1:
arch/powerpc/kernel/prom_init.c:607:6: error: variable ‘l’ set but
not used [-Werror=unused-but-set-variable]
arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set
but not used [-Werror=unused-but-set-variable]
Suggested-by: Michael Ellerman
Signed-off-by: Mathieu Malaterre
---
v4: redo v3 since path variable can be avoided
v3: really move path within ifdef DEBUG_PROM
v2: move path within ifdef DEBUG_PROM
arch/powerpc/kernel/prom_init.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c
b/arch/powerpc/kernel/prom_init.c
index f8a9a50ff9b5..4b223a9470be 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -604,7 +604,7 @@ static void __init early_cmdline_parse(void)
const char *opt;
char *p;
- int l = 0;
+ int l __maybe_unused = 0;
Instead of hiding the problem with __maybe_unused, I think we could
replace the
#ifdef CONFIG_CMDLINE
by a
if (IS_ENABLED(CONFIG_CMDLINE_BOOL))
This is recommanded by Linux codying style
Christophe
prom_cmd_line[0] = 0;
p = prom_cmd_line;
@@ -1386,7 +1386,7 @@ static void __init reserve_mem(u64 base, u64 size)
static void __init prom_init_mem(void)
{
phandle node;
- char *path, type[64];
+ char type[64];
unsigned int plen;
cell_t *p, *endp;
__be32 val;
@@ -1407,7 +1407,6 @@ static void __init prom_init_mem(void)
prom_debug("root_size_cells: %x\n", rsc);
prom_debug("scanning memory:\n");
- path = prom_scratch;
for (node = 0; prom_next_node(&node); ) {
type[0] = 0;
@@ -1432,9 +1431,9 @@ static void __init prom_init_mem(void)
endp = p + (plen / sizeof(cell_t));
#ifdef DEBUG_PROM
- memset(path, 0, PROM_SCRATCH_SIZE);
- call_prom("package-to-path", 3, 1, node, path,
PROM_SCRATCH_SIZE-1);
- prom_debug(" node %s :\n", path);
+ memset(prom_scratch, 0, PROM_SCRATCH_SIZE);
+ call_prom("package-to-path", 3, 1, node, prom_scratch,
PROM_SCRATCH_SIZE - 1);
+ prom_debug(" node %s :\n", prom_scratch);
#endif /* DEBUG_PROM */
while ((endp - p) >= (rac + rsc)) {
--
2.11.0
Re: [PATCH AUTOSEL 4.14 05/30] powerpc/traps: restore recoverability of machine_check interrupts
Looks like this patch didn't make its way into 4.14 yet. Is there a
problem with this patch ?
Christophe
Sasha Levin a écrit :
From: Christophe Leroy
[ Upstream commit daf00ae71dad8aa05965713c62558aeebf2df48e ]
commit b96672dd840f ("powerpc: Machine check interrupt is a non-
maskable interrupt") added a call to nmi_enter() at the beginning of
machine check restart exception handler. Due to that, in_interrupt()
always returns true regardless of the state before entering the
exception, and die() panics even when the system was not already in
interrupt.
This patch calls nmi_exit() before calling die() in order to restore
the interrupt state we had before calling nmi_enter()
Fixes: b96672dd840f ("powerpc: Machine check interrupt is a
non-maskable interrupt")
Signed-off-by: Christophe Leroy
Reviewed-by: Nicholas Piggin
Signed-off-by: Michael Ellerman
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/traps.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index ac2e5e56a9f0..a5f2b7593976 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -694,12 +694,17 @@ void machine_check_exception(struct pt_regs *regs)
if (check_io_access(regs))
goto bail;
- die("Machine check", regs, SIGBUS);
-
/* Must die if the interrupt is not recoverable */
if (!(regs->msr & MSR_RI))
nmi_panic(regs, "Unrecoverable Machine check");
+ if (!nested)
+ nmi_exit();
+
+ die("Machine check", regs, SIGBUS);
+
+ return;
+
bail:
if (!nested)
nmi_exit();
--
2.17.1
Re: [PATCH v5] soc/fsl/qe: fix err handling of ucc_of_parse_tdm
Peng Hao a écrit :
From: Wen Yang
Currently there are some issues with the ucc_of_parse_tdm function:
1, a possible null pointer dereference in ucc_of_parse_tdm,
detected by the semantic patch deref_null.cocci,
with the following warning:
drivers/soc/fsl/qe/qe_tdm.c:177:21-24: ERROR: pdev is NULL but dereferenced.
2, dev gets modified, so in any case that devm_iounmap() will fail
even when the new pdev is valid, because the iomap was done with a
different pdev.
3, there is no driver bind with the "fsl,t1040-qe-si" or
"fsl,t1040-qe-siram" device. So allocating resources using devm_*()
with these devices won't provide a cleanup path for these resources
when the caller fails.
This patch fixes them.
Suggested-by: Li Yang
Suggested-by: Christophe LEROY
Signed-off-by: Wen Yang
Reviewed-by: Peng Hao
CC: Julia Lawall
CC: Zhao Qiang
CC: David S. Miller
CC: net...@vger.kernel.org
CC: linuxppc-...@lists.ozlabs.org
CC: linux-kernel@vger.kernel.org
---
In order to ease review, could add the list of changes between each
version of the patch ? Usually we do it at this place so that it is
available for reviewers but not part of the commit text.
Christophe
drivers/net/wan/fsl_ucc_hdlc.c | 62
+-
drivers/soc/fsl/qe/qe_tdm.c| 55 -
2 files changed, 61 insertions(+), 56 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 839fa77..f30a040 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1057,6 +1057,54 @@ static const struct net_device_ops uhdlc_ops = {
.ndo_tx_timeout = uhdlc_tx_timeout,
};
+static int hdlc_map_iomem(char *name, int init_flag, void __iomem **ptr)
+{
+ struct device_node *np;
+ struct platform_device *pdev;
+ struct resource *res;
+ static int siram_init_flag;
+ int ret = 0;
+
+ np = of_find_compatible_node(NULL, NULL, name);
+ if (!np)
+ return -EINVAL;
+
+ pdev = of_find_device_by_node(np);
+ if (!pdev) {
+ pr_err("%pOFn: failed to lookup pdev\n", np);
+ of_node_put(np);
+ return -EINVAL;
+ }
+
+ of_node_put(np);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ ret = -EINVAL;
+ goto error_put_device;
+ }
+ *ptr = ioremap(res->start, resource_size(res));
+ if (!*ptr) {
+ ret = -ENOMEM;
+ goto error_put_device;
+ }
+
+ /* We've remapped the addresses, and we don't need the device any
+* more, so we should release it.
+*/
+ put_device(&pdev->dev);
+
+ if (init_flag && siram_init_flag == 0) {
+ memset_io(*ptr, 0, resource_size(res));
+ siram_init_flag = 1;
+ }
+ return 0;
+
+error_put_device:
+ put_device(&pdev->dev);
+
+ return ret;
+}
+
static int ucc_hdlc_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1151,6 +1199,15 @@ static int ucc_hdlc_probe(struct
platform_device *pdev)
ret = ucc_of_parse_tdm(np, utdm, ut_info);
if (ret)
goto free_utdm;
+
+ ret = hdlc_map_iomem("fsl,t1040-qe-si", 0,
+(void __iomem **)&utdm->si_regs);
+ if (ret)
+ goto free_utdm;
+ ret = hdlc_map_iomem("fsl,t1040-qe-siram", 1,
+(void __iomem **)&utdm->siram);
+ if (ret)
+ goto unmap_si_regs;
}
if (of_property_read_u16(np, "fsl,hmask", &uhdlc_priv->hmask))
@@ -1159,7 +1216,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
ret = uhdlc_init(uhdlc_priv);
if (ret) {
dev_err(&pdev->dev, "Failed to init uhdlc\n");
- goto free_utdm;
+ goto undo_uhdlc_init;
}
dev = alloc_hdlcdev(uhdlc_priv);
@@ -1188,6 +1245,9 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
free_dev:
free_netdev(dev);
undo_uhdlc_init:
+ iounmap(utdm->siram);
+unmap_si_regs:
+ iounmap(utdm->si_regs);
free_utdm:
if (uhdlc_priv->tsa)
kfree(utdm);
diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c
index f78c346..76480df 100644
--- a/drivers/soc/fsl/qe/qe_tdm.c
+++ b/drivers/soc/fsl/qe/qe_tdm.c
@@ -44,10 +44,6 @@ int ucc_of_parse_tdm(struct device_node *np,
struct ucc_tdm *utdm,
const char *sprop;
int ret = 0;
u32 val;
- struct resource *res;
- struct device_node *np2;
- static int siram_init_flag;
- struct platform_device *pdev;
sprop = of_get_property(np, "fsl,rx-sync-clock", NULL);
if (sprop) {
@@ -124,57 +120,6 @@ int ucc_of_parse_tdm(struct devic
Re: [PATCH] Remove 'type' argument from access_ok() function
Mathieu Malaterre a écrit :
In commit 05a4ab823983 ("powerpc/uaccess: fix warning/error with
access_ok()") an attempt was made to remove a warning by referencing the
variable `type`, however in commit 96d4f267e40f ("Remove 'type' argument
from access_ok() function") the variable `type` has been removed.
Revert commit 05a4ab823983 ("powerpc/uaccess: fix warning/error with
access_ok()") to fix the following compilation error:
arch/powerpc/include/asm/uaccess.h:66:32: error: ‘type’ undeclared
(first use in this function)
Signed-off-by: Mathieu Malaterre
Should you add a Fixes: 96d4f267e40f ?
Note that sparc arch will have the same issue.
Christophe
---
arch/powerpc/include/asm/uaccess.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index b31bf45eebd4..5f0c98e511a0 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -63,7 +63,7 @@ static inline int __access_ok(unsigned long addr,
unsigned long size,
#endif
#define access_ok(addr, size) \
- (__chk_user_ptr(addr), (void)(type),\
+ (__chk_user_ptr(addr), \
__access_ok((__force unsigned long)(addr), (size), get_fs()))
/*
--
2.19.2
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Finn Thain a écrit :
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.
m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
"select NVRAM".
Adopt the powerpc convention on m68k to avoid surprises.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
---
This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.
---
drivers/char/Kconfig | 5 +
drivers/scsi/Kconfig | 6 +++---
drivers/scsi/atari_scsi.c | 7 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 9d03b2ff5df6..5b54595dfe30 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
config NVRAM
tristate "/dev/nvram support"
- depends on ATARI || X86 || GENERIC_NVRAM
+ depends on X86 || GENERIC_NVRAM
---help---
If you say Y here and create a character special file /dev/nvram
with major number 10 and minor number 144 using mknod ("man mknod"),
@@ -254,9 +254,6 @@ config NVRAM
should NEVER idly tamper with it. See Ralf Brown's interrupt list
for a guide to the use of CMOS bytes by your BIOS.
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
To compile this driver as a module, choose M here: the
module will be called nvram.
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 640cd1b31a18..924eb69e7fc4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1381,14 +1381,14 @@ config ATARI_SCSI
tristate "Atari native SCSI support"
depends on ATARI && SCSI
select SCSI_SPI_ATTRS
- select NVRAM
---help---
If you have an Atari with built-in NCR5380 SCSI controller (TT,
Falcon, ...) say Y to get it supported. Of course also, if you have
a compatible SCSI controller (e.g. for Medusa).
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
This driver supports both styles of NCR integration into the
system: the TT style (separate DMA), and the Falcon style (via
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
unsigned char b = nvram_read_byte(16);
@@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (b & 0x80)
atari_scsi_template.this_id = b & 7;
}
- }
+#endif
/* If running on a Falcon and if there's TT-Ram (i.e., more than one
* memory block, since there's always ST-Ram in a Falcon), then
--
2.19.2
Re: [PATCH v8 02/25] m68k/atari: Move Atari-specific code out of drivers/char/nvram.c
Finn Thain a écrit :
Move the m68k-specific code out of the driver to make the driver generic.
I've used 'SPDX-License-Identifier: GPL-2.0+' for the new file because the
old file is covered by MODULE_LICENSE("GPL").
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
Acked-by: Geert Uytterhoeven
---
Changed since v7:
- Added SPDX-License-Identifier.
---
arch/m68k/atari/Makefile | 2 +
arch/m68k/atari/nvram.c | 243 +
drivers/char/nvram.c | 280 +--
3 files changed, 280 insertions(+), 245 deletions(-)
create mode 100644 arch/m68k/atari/nvram.c
diff --git a/arch/m68k/atari/Makefile b/arch/m68k/atari/Makefile
index 0cac723306f9..0b86bb6cfa87 100644
--- a/arch/m68k/atari/Makefile
+++ b/arch/m68k/atari/Makefile
@@ -6,3 +6,5 @@ obj-y := config.o time.o debug.o ataints.o stdma.o \
atasound.o stram.o
obj-$(CONFIG_ATARI_KBD_CORE) += atakeyb.o
+
+obj-$(CONFIG_NVRAM:m=y)+= nvram.o
diff --git a/arch/m68k/atari/nvram.c b/arch/m68k/atari/nvram.c
new file mode 100644
index ..3e620ee955ba
--- /dev/null
+++ b/arch/m68k/atari/nvram.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * CMOS/NV-RAM driver for Atari. Adapted from drivers/char/nvram.c.
+ * Copyright (C) 1997 Roman Hodek
+ * idea by and with help from Richard Jelinek
+ * Portions copyright (c) 2001,2002 Sun Microsystems (thoc...@sun.com)
+ * Further contributions from Cesar Barros, Erik Gilling, Tim Hockin and
+ * Wim Van Sebroeck.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NVRAM_BYTES50
+
+/* It is worth noting that these functions all access bytes of general
+ * purpose memory in the NVRAM - that is to say, they all add the
+ * NVRAM_FIRST_BYTE offset. Pass them offsets into NVRAM as if you did not
+ * know about the RTC cruft.
+ */
+
+/* Note that *all* calls to CMOS_READ and CMOS_WRITE must be done with
+ * rtc_lock held. Due to the index-port/data-port design of the RTC, we
+ * don't want two different things trying to get to it at once. (e.g. the
+ * periodic 11 min sync from kernel/time/ntp.c vs. this driver.)
+ */
+
+unsigned char __nvram_read_byte(int i)
+{
+ return CMOS_READ(NVRAM_FIRST_BYTE + i);
+}
+
+unsigned char nvram_read_byte(int i)
+{
+ unsigned long flags;
+ unsigned char c;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ c = __nvram_read_byte(i);
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ return c;
+}
+EXPORT_SYMBOL(nvram_read_byte);
+
+/* This races nicely with trying to read with checksum checking */
+void __nvram_write_byte(unsigned char c, int i)
+{
+ CMOS_WRITE(c, NVRAM_FIRST_BYTE + i);
+}
+
+void nvram_write_byte(unsigned char c, int i)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ __nvram_write_byte(c, i);
+ spin_unlock_irqrestore(&rtc_lock, flags);
+}
+
+/* On Ataris, the checksum is over all bytes except the checksum bytes
+ * themselves; these are at the very end.
+ */
+#define ATARI_CKS_RANGE_START 0
+#define ATARI_CKS_RANGE_END47
+#define ATARI_CKS_LOC 48
+
+int __nvram_check_checksum(void)
+{
+ int i;
+ unsigned char sum = 0;
+
+ for (i = ATARI_CKS_RANGE_START; i <= ATARI_CKS_RANGE_END; ++i)
+ sum += __nvram_read_byte(i);
+ return (__nvram_read_byte(ATARI_CKS_LOC) == (~sum & 0xff)) &&
+ (__nvram_read_byte(ATARI_CKS_LOC + 1) == (sum & 0xff));
+}
+
+int nvram_check_checksum(void)
+{
+ unsigned long flags;
+ int rv;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ rv = __nvram_check_checksum();
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ return rv;
+}
+EXPORT_SYMBOL(nvram_check_checksum);
+
+static void __nvram_set_checksum(void)
+{
+ int i;
+ unsigned char sum = 0;
+
+ for (i = ATARI_CKS_RANGE_START; i <= ATARI_CKS_RANGE_END; ++i)
+ sum += __nvram_read_byte(i);
+ __nvram_write_byte(~sum, ATARI_CKS_LOC);
+ __nvram_write_byte(sum, ATARI_CKS_LOC + 1);
+}
+
+#ifdef CONFIG_PROC_FS
+static struct {
+ unsigned char val;
+ const char *name;
+} boot_prefs[] = {
+ { 0x80, "TOS" },
+ { 0x40, "ASV" },
+ { 0x20, "NetBSD (?)" },
+ { 0x10, "Linux" },
+ { 0x00, "unspecified" },
+};
+
+static const char * const languages[] = {
+ "English (US)",
+ "German",
+ "French",
+ "English (UK)",
+ "Spanish",
+ "Italian",
+ "6 (undefined)",
+ "Swiss (French)",
+ "Swiss (German)",
+};
+
+static const char * const dateformat[] = {
+ "MM%cDD%cYY",
+ "DD%cMM%cYY",
+ "YY%cMM%cDD",
+ "YY%cDD%cMM",
+ "4 (undefined)",
+ "5 (undefined)",
+ "6 (undefined)",
+ "7 (undefined)",
+};
+
+static const char * const colors[] = {
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Michael Schmitz a écrit :
Hi Finn,
Am 29.12.2018 um 14:06 schrieb Finn Thain:
On Fri, 28 Dec 2018, LEROY Christophe wrote:
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
I don't like #ifdefs either. However, as the maintainer of this file, I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m are
treaded differently by drivers. Therefore, IS_ENABLED would be incorrect.
IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest.
Doesn't #ifdef means either y or m ? So the same as IS_ENABLED() ?
Christophe
Or (really going out on a limb here):
IS_BUILTIN(CONFIG_NVRAM) ||
( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) )
Not that I'd advocate that, for this series.
Cheers,
Michael
Re: [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte()
Finn Thain a écrit :
Make use of arch_nvram_ops in device drivers so that the nvram_* function
exports can be removed.
Since they are no longer global symbols, rename the PPC32 nvram_* functions
appropriately.
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/setup_32.c | 8
drivers/char/generic_nvram.c | 4 ++--
drivers/video/fbdev/controlfb.c| 4 ++--
drivers/video/fbdev/imsttfb.c | 4 ++--
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
drivers/video/fbdev/platinumfb.c | 4 ++--
drivers/video/fbdev/valkyriefb.c | 4 ++--
7 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index e0d045677472..bdbe6acbef11 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -152,20 +152,18 @@ __setup("l3cr=", ppc_setup_l3cr);
#ifdef CONFIG_GENERIC_NVRAM
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
{
if (ppc_md.nvram_read_val)
return ppc_md.nvram_read_val(addr);
return 0xff;
}
-EXPORT_SYMBOL(nvram_read_byte);
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
{
if (ppc_md.nvram_write_val)
ppc_md.nvram_write_val(addr, val);
}
-EXPORT_SYMBOL(nvram_write_byte);
static ssize_t ppc_nvram_get_size(void)
{
@@ -182,6 +180,8 @@ static long ppc_nvram_sync(void)
}
const struct nvram_ops arch_nvram_ops = {
+ .read_byte = ppc_nvram_read_byte,
+ .write_byte = ppc_nvram_write_byte,
.get_size = ppc_nvram_get_size,
.sync = ppc_nvram_sync,
};
diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
index f32d5663de95..41b76bf9614e 100644
--- a/drivers/char/generic_nvram.c
+++ b/drivers/char/generic_nvram.c
@@ -48,7 +48,7 @@ static ssize_t read_nvram(struct file *file, char
__user *buf,
if (*ppos >= nvram_len)
return 0;
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
- if (__put_user(nvram_read_byte(i), p))
+ if (__put_user(arch_nvram_ops.read_byte(i), p))
Instead of modifying all drivers (in this patch and previous ones
related to other arches), wouldn't it be better to add helpers like
the following in nvram.h:
Static inline unsigned char nvram_read_byte(int addr)
{
return arch_nvram_ops.read_byte(addr);
}
Christophe
return -EFAULT;
*ppos = i;
return p - buf;
@@ -68,7 +68,7 @@ static ssize_t write_nvram(struct file *file,
const char __user *buf,
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
if (__get_user(c, p))
return -EFAULT;
- nvram_write_byte(c, i);
+ arch_nvram_ops.write_byte(c, i);
}
*ppos = i;
return p - buf;
diff --git a/drivers/video/fbdev/controlfb.c
b/drivers/video/fbdev/controlfb.c
index 9cb0ef7ac29e..27ff33ccafcb 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -413,7 +413,7 @@ static int __init init_control(struct fb_info_control *p)
/* Try to pick a video mode out of NVRAM if we have one. */
#ifdef CONFIG_NVRAM
if (default_cmode == CMODE_NVRAM) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if(cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
} else
@@ -421,7 +421,7 @@ static int __init init_control(struct fb_info_control *p)
cmode=default_cmode;
#ifdef CONFIG_NVRAM
if (default_vmode == VMODE_NVRAM) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode < 1 || vmode > VMODE_MAX ||
control_mac_modes[vmode - 1].m[full] < cmode) {
sense = read_control_sense(p);
diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c
index 8d231591ff0e..e9f3b8914145 100644
--- a/drivers/video/fbdev/imsttfb.c
+++ b/drivers/video/fbdev/imsttfb.c
@@ -1393,12 +1393,12 @@ static void init_imstt(struct fb_info *info)
int vmode = init_vmode, cmode = init_cmode;
if (vmode == -1) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode <= 0 || vmode > VMODE_MAX)
vmode = VMODE_640_480_67;
}
if (cmode == -1) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if (cmode < CMODE_8 || cmode > CMODE_32)
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Arnd Bergmann a écrit : On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz wrote: Hi Finn, Am 29.12.2018 um 15:34 schrieb Finn Thain: > On Sat, 29 Dec 2018, Michael Schmitz wrote: > >> >> IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest. >> >> Or (really going out on a limb here): >> >> IS_BUILTIN(CONFIG_NVRAM) || >> ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) >> >> Not that I'd advocate that, for this series. >> > > Well, you are a maintainer for atari_scsi.c. > > Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of > ifdef? No, just pointing out that there would be a way to avoid the ifdef without messing up driver behaviour. I'm fine with the ifdef - not least because it clearly eliminates code that would be unreachable. (On second thought - I don't want to speculate whether there's weird compiler options that could result in the nvram_check_checksum and nvram_read_bytes symbols to still be referenced in the final link, even though IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best leave this as-is.) As far as I know, it's totally reliable with the supported compilers (gcc-4.6+). In the older compilers (e.g. 4.1), there was a corner case, where it could have failed to eliminate a function that was only referenced through a pointer from a discarded variable, but a plain IS_ENABLED() check like the one here was still ok, and lots of code relies on that. Other than that, I agree either way is totally fine here, so no objections to using the #ifdef. As far as I know, kernel codying style promotes the use of IS_ENABLED() etc. instead of #ifdefs when possible. Christophe Arnd
Re: [PATCH v8 13/25] m68k: Dispatch nvram_ops calls to Atari or Mac functions
Finn Thain a écrit :
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
On Wed, Dec 26, 2018 at 1:43 AM Finn Thain
wrote:
> +
> +static ssize_t m68k_nvram_get_size(void)
> +{
> + if (MACH_IS_ATARI)
> + return atari_nvram_get_size();
> + else if (MACH_IS_MAC)
> + return mac_pram_get_size();
> + return -ENODEV;
> +}
> +
> +/* Atari device drivers call .read (to get checksum validation) whereas
> + * Mac and PowerMac device drivers just use .read_byte.
> + */
> +const struct nvram_ops arch_nvram_ops = {
> +#ifdef CONFIG_MAC
> + .read_byte = m68k_nvram_read_byte,
> + .write_byte = m68k_nvram_write_byte,
> +#endif
> +#ifdef CONFIG_ATARI
> + .read = m68k_nvram_read,
> + .write = m68k_nvram_write,
> + .set_checksum = m68k_nvram_set_checksum,
> + .initialize = m68k_nvram_initialize,
> +#endif
> + .get_size = m68k_nvram_get_size,
> +};
> +EXPORT_SYMBOL(arch_nvram_ops);
Since the operations are almost entirely distinct, why not have two
separate 'nvram_ops' instances here that each refer to just
the set they actually need?
The reason for that is that I am alergic to code duplication. But I'll
change it if you think it matters. BTW, this patch has already been acked
by Geert.
I agree it would be cleaner, as it would also avoid this
m68k_nvram_get_size() wouldn't it ?
I don't see potential code duplication here, do you ?
Christophe
I was actually expecting one more patch here that would make the
arch_nvram_ops a pointer to one of multiple structures, which would
be easier to do with multiple copies, but I suppose there is no need
for that here (there might be on ppc, I have to look again).
Yes, I considered that too. I picked the variation that makes everything
const.
--
Arnd
Re: [RESEND PATCH v5 0/4] Add perf interface to expose nvdimm
Hi Le 16/11/2021 à 05:49, Kajol Jain a écrit : > Patchset adds performance stats reporting support for nvdimm. > Added interface includes support for pmu register/unregister > functions. A structure is added called nvdimm_pmu to be used for > adding arch/platform specific data such as cpumask, nvdimm device > pointer and pmu event functions like event_init/add/read/del. > User could use the standard perf tool to access perf events > exposed via pmu. > > Interface also defines supported event list, config fields for the > event attributes and their corresponding bit values which are exported > via sysfs. Patch 3 exposes IBM pseries platform nmem* device > performance stats using this interface. You resending your v5 series ? Is there any news since your submittion last month that's awaiting in patchwork here at https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=264422 ? Christophe > > Result from power9 pseries lpar with 2 nvdimm device: > > Ex: List all event by perf list > > command:# perf list nmem > >nmem0/cache_rh_cnt/[Kernel PMU event] >nmem0/cache_wh_cnt/[Kernel PMU event] >nmem0/cri_res_util/[Kernel PMU event] >nmem0/ctl_res_cnt/ [Kernel PMU event] >nmem0/ctl_res_tm/ [Kernel PMU event] >nmem0/fast_w_cnt/ [Kernel PMU event] >nmem0/host_l_cnt/ [Kernel PMU event] >nmem0/host_l_dur/ [Kernel PMU event] >nmem0/host_s_cnt/ [Kernel PMU event] >nmem0/host_s_dur/ [Kernel PMU event] >nmem0/med_r_cnt/ [Kernel PMU event] >nmem0/med_r_dur/ [Kernel PMU event] >nmem0/med_w_cnt/ [Kernel PMU event] >nmem0/med_w_dur/ [Kernel PMU event] >nmem0/mem_life/[Kernel PMU event] >nmem0/poweron_secs/[Kernel PMU event] >... >nmem1/mem_life/[Kernel PMU event] >nmem1/poweron_secs/[Kernel PMU event] > > Patch1: > Introduces the nvdimm_pmu structure > Patch2: > Adds common interface to add arch/platform specific data > includes nvdimm device pointer, pmu data along with > pmu event functions. It also defines supported event list > and adds attribute groups for format, events and cpumask. > It also adds code for cpu hotplug support. > Patch3: > Add code in arch/powerpc/platform/pseries/papr_scm.c to expose > nmem* pmu. It fills in the nvdimm_pmu structure with pmu name, > capabilities, cpumask and event functions and then registers > the pmu by adding callbacks to register_nvdimm_pmu. > Patch4: > Sysfs documentation patch > > Changelog > --- > v4 -> v5: > - Remove multiple variables defined in nvdimm_pmu structure include >name and pmu functions(event_int/add/del/read) as they are just >used to copy them again in pmu variable. Now we are directly doing >this step in arch specific code as suggested by Dan Williams. > > - Remove attribute group field from nvdimm pmu structure and >defined these attribute groups in common interface which >includes format, event list along with cpumask as suggested by >Dan Williams. >Since we added static defination for attrbute groups needed in >common interface, removes corresponding code from papr. > > - Add nvdimm pmu event list with event codes in the common interface. > > - Remove Acked-by/Reviewed-by/Tested-by tags as code is refactored >to handle review comments from Dan. > > - Make nvdimm_pmu_free_hotplug_memory function static as reported >by kernel test robot, also add corresponding Reported-by tag. > > - Link to the patchset v4: https://lkml.org/lkml/2021/9/3/45 > > v3 -> v4 > - Rebase code on top of current papr_scm code without any logical >changes. > > - Added Acked-by tag from Peter Zijlstra and Reviewed by tag >from Madhavan Srinivasan. > > - Link to the patchset v3: https://lkml.org/lkml/2021/6/17/605 > > v2 -> v3 > - Added Tested-by tag. > > - Fix nvdimm mailing list in the ABI Documentation. > > - Link to the patchset v2: https://lkml.org/lkml/2021/6/14/25 > > v1 -> v2 > - Fix hotplug code by adding pmu migration call >incase current designated cpu got offline. As >pointed by Peter Zijlstra. > > - Removed the retun -1 part from cpu hotplug offline >function. > > - Link to the patchset v1: https://lkml.org/lkml/2021/6/8/500 > > Kajol Jain (4): >drivers/nvdimm: Add nvdimm pmu structure >drivers/nvdimm: Add perf interface
Re: [PATCH v3 2/2] powerpc/8xx: use _PAGE_RO instead of _PAGE_RW
Le 05/01/2015 19:12, Joakim Tjernlund a écrit : On Mon, 2014-12-22 at 11:14 +0100, Christophe Leroy wrote: On powerpc 8xx, in TLB entries, 0x400 bit is set to 1 for read-only pages and is set to 0 for RW pages. So we should use _PAGE_RO instead of _PAGE_RW Signed-off-by: Christophe Leroy Hi Christophe, been meaning to look over all you recent 8xx MMU/TLB patches but got so little time :( This is very cool (not sure if there will be a performance gain) but .. I think every saved cycle is worth it. Before I did any modification: * ITLBMiss was 28 instructions. * DTLBMiss was 32 instructions. Now, (No MODULES, no CPU6, no CPU15): * ITLBMiss is 15 instructions * DTLBMiss is 24 instructions diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h index caf094a..b4e0c3b 100644 --- a/arch/powerpc/include/asm/pgtable-ppc32.h +++ b/arch/powerpc/include/asm/pgtable-ppc32.h @@ -178,9 +178,10 @@ static inline unsigned long pte_update(pte_t *p, andc%1,%0,%5\n\ or %1,%1,%6\n\ /* 0x200 == Extended encoding, bit 22 */ \ - /* Bit 22 has to be 1 if neither _PAGE_USER nor _PAGE_RW are set */ \ + /* Bit 22 has to be 1 when _PAGE_USER is unset and _PAGE_RO is set */ \ rlwimi %1,%1,32-2,0x200\n /* get _PAGE_USER */ \ - rlwinm %3,%1,32-1,0x200\n /* get _PAGE_RW */ \ + rlwinm %3,%1,32-1,0x200\n /* get _PAGE_RO */ \ + xori%3,%3,0x200\n \ or %1,%3,%1\n\ xori%1,%1,0x200\n" " stwcx. %1,0,%4\n\ ... here I expected to loose the existing xori insn instead of adding one? Well, I could have xored the PAGE_USER bit instead, but in that case, it is not anymore an 'or' but an 'and' that has to be performed between the bits, and then all other bits must be set to 1, or the result of the 'and' shall be inserted using 'rlwimi'. So it would be more modifications than just adding an xori, and not less instructions. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 09/11] powerpc/8xx: dont save CR in SCRATCH registers
Le 05/01/2015 19:30, Joakim Tjernlund a écrit : On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote: CR only needs to be preserved when checking if we are handling a kernel address. So we can preserve CR in a register: - In ITLBMiss, check is done only when CONFIG_MODULES is defined. Otherwise we don't need to do anything at all with CR. - If CONFIG_8xx_CPU6 is defined, we have r3 available for saving CR - Otherwise, we use r10, then we reload SRR0/MD_EPN into r10 when CR is restored I think the #ifdef CPU6 code makes for too much maintenance. Can you loose the #ifdef CPU6 and adjust code so it works for both cases? I wanted to take the opportunity to save one cycle, but we are wasting so many cycles with CPU6 that it is propably not worth it. Ok, I will remove the special handling for CPU6. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/head_8xx.S | 53 +- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index c89aed9..a073918 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -308,14 +308,10 @@ SystemCall: #endif InstructionTLBMiss: + EXCEPTION_PROLOG_0 #ifdef CONFIG_8xx_CPU6 mtspr SPRN_DAR, r3 #endif - EXCEPTION_PROLOG_0 - mfcrr10 - mtspr SPRN_SPRG_SCRATCH2, r10 - mfspr r10, SPRN_SRR0/* Get effective address of fault */ - DO_8xx_CPU15(r11, r10) /* If we are faulting a kernel address, we have to use the * kernel page tables. @@ -323,14 +319,33 @@ InstructionTLBMiss: #ifdef CONFIG_MODULES /* Only modules will cause ITLB Misses as we always * pin the first 8MB of kernel memory */ +#ifdef CONFIG_8xx_CPU6 + mfspr r10, SPRN_SRR0/* Get effective address of fault */ + DO_8xx_CPU15(r11, r10) + mfcrr3 andis. r11, r10, 0x8000/* Address >= 0x8000 */ +#else + mfspr r11, SPRN_SRR0/* Get effective address of fault */ + DO_8xx_CPU15(r10, r11) + mfcrr10 + andis. r11, r11, 0x8000/* Address >= 0x8000 */ #endif mfspr r11, SPRN_M_TW/* Get level 1 table base address */ -#ifdef CONFIG_MODULES beq 3f lis r11, (swapper_pg_dir-PAGE_OFFSET)@ha 3: +#ifdef CONFIG_8xx_CPU6 + mtcrr3 +#else + mtcrr10 + mfspr r10, SPRN_SRR0/* Get effective address of fault */ #endif +#else /* CONFIG_MODULES */ + mfspr r10, SPRN_SRR0/* Get effective address of fault */ + DO_8xx_CPU15(r11, r10) + mfspr r11, SPRN_M_TW/* Get level 1 table base address */ +#endif /* CONFIG_MODULES */ + /* Insert level 1 index */ rlwimi r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) << 1, 29 lwz r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level 1 entry */ @@ -362,29 +377,37 @@ InstructionTLBMiss: mfspr r3, SPRN_DAR mtspr SPRN_DAR, r11/* Tag DAR */ #endif - mfspr r10, SPRN_SPRG_SCRATCH2 - mtcrr10 EXCEPTION_EPILOG_0 rfi . = 0x1200 DataStoreTLBMiss: -#ifdef CONFIG_8xx_CPU6 - mtspr SPRN_DAR, r3 -#endif EXCEPTION_PROLOG_0 - mfcrr10 - mtspr SPRN_SPRG_SCRATCH2, r10 - mfspr r10, SPRN_MD_EPN /* If we are faulting a kernel address, we have to use the * kernel page tables. */ +#ifdef CONFIG_8xx_CPU6 + mtspr SPRN_DAR, r3 + mfcrr3 + mfspr r10, SPRN_MD_EPN andis. r11, r10, 0x8000 +#else + mfcrr10 + mfspr r11, SPRN_MD_EPN + andis. r11, r11, 0x8000 +#endif mfspr r11, SPRN_M_TW/* Get level 1 table base address */ beq 3f lis r11, (swapper_pg_dir-PAGE_OFFSET)@ha 3: +#ifdef CONFIG_8xx_CPU6 + mtcrr3 +#else + mtcrr10 + mfspr r10, SPRN_MD_EPN +#endif + /* Insert level 1 index */ rlwimi r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) << 1, 29 lwz r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level 1 entry */ @@ -441,8 +464,6 @@ DataStoreTLBMiss: mfspr r3, SPRN_DAR #endif mtspr SPRN_DAR, r11/* Tag DAR */ - mfspr r10, SPRN_SPRG_SCRATCH2 - mtcrr10 EXCEPTION_EPILOG_0 rfi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 05/11] powerpc/8xx: Optimise access to swapper_pg_dir
Le 06/01/2015 13:08, David Laight a écrit : On Tue, 2014-12-16 at 16:03 +0100, Christophe Leroy wrote: All accessed to PGD entries are done via 0(r11). By using lower part of swapper_pg_dir as load index to r11, we can remove the ori instruction. Signed-off-by: Christophe Leroy Nice :) Acked-by: Joakim Tjernlund --- arch/powerpc/kernel/head_8xx.S | 22 ++ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index ae05f28..aa45225 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -322,13 +322,12 @@ InstructionTLBMiss: mfspr r11, SPRN_M_TW/* Get level 1 table base address */ #ifdef CONFIG_MODULES beq 3f - lis r11, (swapper_pg_dir-PAGE_OFFSET)@h - ori r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l + lis r11, (swapper_pg_dir-PAGE_OFFSET)@ha 3: #endif /* Insert level 1 index */ rlwimi r11, r10, 32 - ((PAGE_SHIFT - 2) << 1), (PAGE_SHIFT - 2) << 1, 29 - lwz r11, 0(r11)/* Get the level 1 entry */ + lwz r11, (swapper_pg_dir-PAGE_OFFSET)@l(r11)/* Get the level 1 entry */ On the face of it that fragment doesn't look right when CONFIG_MODULES is undefined. David I'm not sure I understand what you mean. The other part of the patch adds the following: +lir5, (swapper_pg_dir-PAGE_OFFSET)@l +subr4, r4, r5 r4 is the value put into SPRN_M_TW, so I don't see what may be wrong. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: algif_hash: splice of data > 2**16
Le 20/12/2014 07:37, Stephan Mueller a écrit : Am Donnerstag, 18. Dezember 2014, 13:22:20 schrieb leroy christophe: Hi Christophe, Le 18/12/2014 13:15, Stephan Mueller a écrit : Hi Herbert, While testing the vmsplice/splice interface of algif_hash I was made aware of the problem that data blobs larger than 16 pages do not seem to be hashed properly. For testing, a file is mmap()ed and handed to vmsplice / splice. If the file is smaller than 2**16, the interface returns the proper hash. However, when the file is larger, only the first 2**16 bytes seem to be used. When adding printk's to hash_sendpage, I see that this function is invoked exactly 16 times where the first 15 invocations have the MSG_MORE flag set and the last invocation does not have MSG_MORE. Hi Stephan, I have already noticed the same issue and proposed a patch, but I never got any feedback and it has never been merged, allthought I pinged it a few times. See https://lkml.org/lkml/2014/4/18/276 After testing, this patch does not work for me. The operation still stops after 16 pages. Yes, it looks like the function I fixed is exclusively used by sendfile() system call. So there is probably the same kind of fix to be done in another function. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: algif_hash: splice of data > 2**16
Le 24/12/2014 10:03, Stephan Mueller a écrit : Am Dienstag, 23. Dezember 2014, 18:16:01 schrieb leroy christophe: Hi leroy, Le 20/12/2014 07:37, Stephan Mueller a écrit : Am Donnerstag, 18. Dezember 2014, 13:22:20 schrieb leroy christophe: Hi Christophe, Le 18/12/2014 13:15, Stephan Mueller a écrit : Hi Herbert, While testing the vmsplice/splice interface of algif_hash I was made aware of the problem that data blobs larger than 16 pages do not seem to be hashed properly. For testing, a file is mmap()ed and handed to vmsplice / splice. If the file is smaller than 2**16, the interface returns the proper hash. However, when the file is larger, only the first 2**16 bytes seem to be used. When adding printk's to hash_sendpage, I see that this function is invoked exactly 16 times where the first 15 invocations have the MSG_MORE flag set and the last invocation does not have MSG_MORE. Hi Stephan, I have already noticed the same issue and proposed a patch, but I never got any feedback and it has never been merged, allthought I pinged it a few times. See https://lkml.org/lkml/2014/4/18/276 After testing, this patch does not work for me. The operation still stops after 16 pages. Yes, it looks like the function I fixed is exclusively used by sendfile() system call. So there is probably the same kind of fix to be done in another function. I do not believe that is the case. IMHO the blocking issue is found in the following code: splice_from_pipe_feed walks the pipe->nrbufs. And vmsplice_to_pipe defines the maximum number of nrbufs as PIPE_DEF_BUFFERS -- which is 16. As subsequent functions allocate memory based on PIPE_DEF_BUFFERS, there is no trivial way to increase the number of pages to be processed. Thus I see that the vmsplice/splice combo can at most operate on a chunk of 16 pages. Thus, you have to segment your input buffer into chunks of that size and invoke the vmsplice/splice syscalls for each segment. Yes your are probably right. There splice needs to be called with SPLICE_F_MORE flag, hope that works. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: powerpc32: fix warning from include/linux/mm.h
Le 21/03/2015 00:52, Scott Wood a écrit : On Fri, Dec 05, 2014 at 12:20:20PM +0100, LEROY Christophe wrote: include/linux/mm.h: In function 'is_vmalloc_addr': include/linux/mm.h:367:14: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] return addr >= VMALLOC_START && addr < VMALLOC_END; ^ Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/pgtable-ppc32.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) That warning doesn't appear to be enabled. What config are you seeing this with? I'm used to adding EXTRA_CFLAGS=-Wextra when checking my own drivers, as it helps finding additional bugs. When doing that, the only warnings I get outside of my own code are this one, and the other one in my proposed patch identified "powerpc32: fix warning from include/asm-generic/termios-base.h" I though it would be worth fixing those two warnings in order to get a perfectly clean code. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 11/11] powerpc/8xx: Add support for TASK_SIZE greater than 0x80000000
Le 21/03/2015 01:47, Scott Wood a écrit : On Tue, 2015-01-20 at 10:57 +0100, Christophe Leroy wrote: By default, TASK_SIZE is set to 0x8000 for PPC_8xx, which is most likely sufficient for most cases. However, kernel configuration allows to set TASK_SIZE to another value, so the 8xx shall handle it. Signed-off-by: Christophe Leroy --- v2: no change arch/powerpc/kernel/head_8xx.S | 25 +++-- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index ead6448..92a6456 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -48,6 +48,19 @@ mtspr spr, reg #endif +/* Macro to test if an address is a kernel address */ +#if CONFIG_TASK_SIZE <= 0x8000 +#define IS_KERNEL(tmp, addr) \ + andis. tmp, addr, 0x8000 /* Address >= 0x8000 */ +#define BRANCH_UNLESS_KERNEL(label)beq label +#else This works if CONFIG_TASK_SIZE == 0x8000, but what if it's less, and you have a kernel address < 0x8000? You are right, I didn't realise that PAGE_OFFSET was also configurable and could be different from 0xC000. andis. (which is what is always used in the current kernel) can be used if CONFIG_TASK_SIZE <= 0x8000 AND PAGE_OFFSET >= 0x8000. I will resubmit the patch accordingly. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 00/17] crypto: talitos - Add support for SEC1
Oops, this is the first time I use directly the output of git format-patch into sendmail, and it looks like the mails are dated with the commit date, not today's date. I will resend now with today's date. Sorry for the noise. Christophe Le 17/04/2015 15:47, Christophe Leroy a écrit : The purpose of this set of patchs is to add to talitos crypto driver the support for the SEC1 version of the security engine, which is found in mpc885 and mpc8272 processors. v3 is a complete rework of the patchset. Since a kernel can be built with support for both MPC82xx and MPC83xx at the same time, talitos driver shall support both SEC1 and SEC2+ at the same time. Based on cryptodev-2.6 tree Christophe Leroy (17): crypto: talitos - Use zero entry to init descriptors ptrs to zero crypto: talitos - Refactor the sg in/out chain allocation crypto: talitos - talitos_ptr renamed ptr for more lisibility crypto: talitos - Add a helper function to clear j_extent field crypto: talitos - remove param 'extent' in map_single_talitos_ptr() crypto: talitos - helper function for ptr len crypto: talitos - enhanced talitos_desc struct for SEC1 crypto: talitos - add sub-choice in talitos CONFIG for SEC1 crypto: talitos - Add a feature to tag SEC1 crypto: talitos - fill in talitos descriptor iaw SEC1 or SEC2+ crypto: talitos - adaptation of talitos_submit() for SEC1 crypto: talitos - base address for Execution Units crypto: talitos - adapt interrupts and reset functions to SEC1 crypto: talitos - implement scatter/gather copy for SEC1 crypto: talitos - SEC1 bugs on 0 data hash crypto: talitos - Add fsl,sec1.0 compatible crypto: talitos - Update DT bindings with SEC1 .../devicetree/bindings/crypto/fsl-sec2.txt| 6 +- drivers/crypto/Kconfig | 18 + drivers/crypto/talitos.c | 727 +++-- drivers/crypto/talitos.h | 153 +++-- 4 files changed, 644 insertions(+), 260 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 03/17] crypto: talitos - talitos_ptr renamed ptr for more lisibility
Le 17/04/2015 17:14, David Laight a écrit :
From: Christophe Leroy
Linux CodyingStyle recommends to use short variables for local
variables. ptr is just good enough for those 3 lines functions.
It helps keep single lines shorter than 80 characters.
...
-static void to_talitos_ptr(struct talitos_ptr *talitos_ptr, dma_addr_t
dma_addr)
+static void to_talitos_ptr(struct talitos_ptr *ptr, dma_addr_t dma_addr)
{
- talitos_ptr->ptr = cpu_to_be32(lower_32_bits(dma_addr));
- talitos_ptr->eptr = upper_32_bits(dma_addr);
+ ptr->ptr = cpu_to_be32(lower_32_bits(dma_addr));
+ ptr->eptr = upper_32_bits(dma_addr);
}
...
Maybe, but 'ptr' isn't a good choice.
Any suggestion ?
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/11] powerpc8xx: Further optimisation of TLB handling
Le 20/04/2015 07:26, Christophe Leroy a écrit : This patchset provides a further optimisation of TLB handling in the 8xx. Main changes are based on: - Using processor handling of PGD/PTE Validity bits instead of testing ourselves the entries validity - Aligning PGD address to allow direct bit manipulation - Not saving registers like CR when not needed It also adds support to any TASK_SIZE Patchset: 01 - powerpc/8xx: remove remaining unnecessary code in FixupDAR 02 - powerpc/8xx: remove tests on PGDIR entry validity 03 - powerpc32: Use kmem_cache memory for PGDIR 04 - powerpc/8xx: Take benefit of aligned PGDIR 05 - powerpc/8xx: Optimise access to swapper_pg_dir 06 - powerpc/8xx: Remove duplicated code in set_context() 07 - powerpc/8xx: macro for handling CPU15 errata 08 - powerpc/8xx: Handle CR out of exception PROLOG/EPILOG 09 - powerpc/8xx: dont save CR in SCRATCH registers 10 - powerpc/8xx: Use SPRG2 instead of DAR for saving r3 11 - powerpc/8xx: Add support for TASK_SIZE greater than 0x8000 All changes have been successfully tested on MPC885 Signed-off-by: Christophe Leroy Tested-by: Christophe Leroy Sorry, forget than one, my finger slipped and I resent a very old one already applied Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/11] powerpc/8xx: macro for handling CPU15 errata
Le 20/04/2015 13:40, David Laight a écrit : From: Christophe Leroy Sent: 20 April 2015 06:27 Having a macro will help keep clear code. ... * We have to use the MD_xxx registers for the tablewalk because the * equivalent MI_xxx registers only perform the attribute functions. */ + +#ifdef CONFIG_8xx_CPU15 +#define DO_8xx_CPU15(tmp, addr)\ + additmp, addr, PAGE_SIZE; \ + tlbie tmp;\ + additmp, addr, PAGE_SIZE; \ + tlbie tmp +#else +#define DO_8xx_CPU15(tmp, addr) +#endif I'm sure I've spotted the same obvious error in the above before. I'd also suggest calling it 'invalidate_adjacent_pages' - since that it what it does. I also guess that the execution time of 'tlbie' is non-trivial. So you might as well get rid of the temporary register and put an 'addi' to reset 'addr' at the end. David Forget it, I did a big mistake this morning, involontarily resent an old patch. Sorry for the noise. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v3, 01/11] powerpc/8xx: remove remaining unnecessary code in FixupDAR
Le 26/03/2015 22:32, Scott Wood a écrit :
On Tue, Feb 03, 2015 at 12:38:16PM +0100, LEROY Christophe wrote:
Since commit 33fb845a6f01 ("powerpc/8xx: Don't use MD_TWC for walk"), MD_EPN and
MD_TWC are not writen anymore in FixupDAR so saving r3 has become useless.
Signed-off-by: Christophe Leroy
---
v2: no change
v3: no change
This doesn't apply cleanly.
You already applied part of that patchset it in your next tree,
including that one (commit 2374d0a).
You told me to re-submit a patchset with only the remaining ones,
therefore I sent v4 on the 4th of Feb, based on your tree.
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [v3, 01/11] powerpc/8xx: remove remaining unnecessary code in FixupDAR
Le 13/04/2015 22:26, Scott Wood a écrit :
On Sun, 2015-04-12 at 18:16 +0200, leroy christophe wrote:
Le 26/03/2015 22:32, Scott Wood a écrit :
On Tue, Feb 03, 2015 at 12:38:16PM +0100, LEROY Christophe wrote:
Since commit 33fb845a6f01 ("powerpc/8xx: Don't use MD_TWC for walk"), MD_EPN and
MD_TWC are not writen anymore in FixupDAR so saving r3 has become useless.
Signed-off-by: Christophe Leroy
---
v2: no change
v3: no change
This doesn't apply cleanly.
You already applied part of that patchset it in your next tree,
including that one (commit 2374d0a).
You told me to re-submit a patchset with only the remaining ones,
therefore I sent v4 on the 4th of Feb, based on your tree.
OK. I applied v2, and didn't remember that when I came across v3 in
patchwork.
What about v4 (the remaining ones) ? You got comments on the last one of
the set, have you applied the other ones or shall I re-sumbit a full v5 ?
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [BUG] mpc8323_rdb platform doesn't boot since kernel 3.16
Le 10/06/2015 20:17, Rob Herring a écrit : On Wed, Jun 10, 2015 at 10:12 AM, leroy christophe wrote: Le 06/06/2015 17:32, Rob Herring a écrit : On Sat, Jun 6, 2015 at 6:20 AM, christophe leroy wrote: I've got a MPC8323 RDB evaluation platform from freescale kernel 4.0 doesn't boot kernel 3.16 doesn't boot kernel 3.15 boots ok I disected the issue down to your commit "of/fdt: Convert FDT functions to use libfdt" (e6a6928c3ea1d0195ed75a091e345696b916c09b) Do you have an idea of what the issue could be ? Is your dtb older version of the dtb format (before 0x10)? libfdt doesn't work on the older versions. Yes, dtb has version 0x11, which seems to be the issue (see below) In that your bootloader doesn't understand 0x11. If not, do you have logs with debug enabled in drivers/of/fdt.c? I get "unflatten: error -11 processing FDT" Can I get the full debug prints. Here it is: -> unflatten_device_tree() Unflattening device tree: magic: d00dfeed size: 20b5 version: 0011 unflatten: error -11 processing FDT unflatten: error -11 processing FDT size is 44dc, allocating... unflattening c3ff0b20... fixed up name for -> fixed up name for aliases -> aliases fixed up name for cpus -> cpus fixed up name for PowerPC,8323@0 -> PowerPC,8323 fixed up name for memory -> memory fixed up name for soc8323@e000 -> soc8323 fixed up name for wdt@200 -> wdt fixed up name for power@b00 -> power fixed up name for i2c@3000 -> i2c fixed up name for serial@4500 -> serial fixed up name for serial@4600 -> serial fixed up name for dma@82a8 -> dma fixed up name for dma-channel@0 -> dma-channel fixed up name for dma-channel@80 -> dma-channel fixed up name for dma-channel@100 -> dma-channel fixed up name for dma-channel@180 -> dma-channel fixed up name for crypto@3 -> crypto fixed up name for pic@700 -> pic fixed up name for par_io@1400 -> par_io fixed up name for gpio-controller@1448 -> gpio-controller fixed up name for ucc_pin@02 -> ucc_pin fixed up name for ucc_pin@03 -> ucc_pin fixed up name for qe@e010 -> qe fixed up name for muram@1 -> muram fixed up name for data-only@0 -> data-only fixed up name for spi@4c0 -> spi fixed up name for mmc-slot@0 -> mmc-slot fixed up name for spi@500 -> spi fixed up name for ucc@3000 -> ucc fixed up name for ucc@2200 -> ucc fixed up name for mdio@3120 -> mdio fixed up name for ethernet-phy@00 -> ethernet-phy fixed up name for ethernet-phy@04 -> ethernet-phy fixed up name for interrupt-controller@80 -> interrupt-controller fixed up name for pci@e0008500 -> pci unflatten: error -11 processing FDT unflatten: error -11 processing FDT <- unflatten_device_tree() Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BUG] mpc8323_rdb platform doesn't boot since kernel 3.16
Le 06/06/2015 17:32, Rob Herring a écrit : On Sat, Jun 6, 2015 at 6:20 AM, christophe leroy wrote: I've got a MPC8323 RDB evaluation platform from freescale kernel 4.0 doesn't boot kernel 3.16 doesn't boot kernel 3.15 boots ok I disected the issue down to your commit "of/fdt: Convert FDT functions to use libfdt" (e6a6928c3ea1d0195ed75a091e345696b916c09b) Do you have an idea of what the issue could be ? Is your dtb older version of the dtb format (before 0x10)? libfdt doesn't work on the older versions. Yes, dtb has version 0x11, which seems to be the issue (see below) If not, do you have logs with debug enabled in drivers/of/fdt.c? I get "unflatten: error -11 processing FDT" Indeed, Uboot (version 1.1.6 - installed on that evaluation board) adds a node called "chosen" at the end of the BLOB. But Uboot doesn't update the FDT len in the BLOB header, therefore the following test fails in fdt_offset_ptr() if (fdt_version(fdt) >= 0x11) if (((offset + len) < offset) || ((offset + len) > fdt_size_dt_struct(fdt))) return NULL; If I comment this test out, Linux 3.16 to 4.0 work properly on my evaluation board. Can we find a proper workaround for this issue ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Oops in 3.18.14 in destroy_inode()
[46796.501487] Unable to handle kernel paging request for data at
address 0x02dd
[46796.514365] Faulting instruction address: 0xc00c5978
[46796.524217] Oops: Kernel access of bad area, sig: 11 [#1]
[46796.529351] PREEMPT CMPC885
[46796.532144] CPU: 0 PID: 1107 Comm: snmpd Not tainted 3.18.14 #43
[46796.539790] task: c682d340 ti: c6728000 task.ti: c6728000
[46796.545119] NIP: c00c5978 LR: c00c5974 CTR: c00efeb4
[46796.550033] REGS: c6729e00 TRAP: 0300 Not tainted (3.18.14)
[46796.557497] MSR: 9032 CR: 24042424 XER: 2000
[46796.564043] DAR: 02dd DSISR: c000
[46796.564043] GPR00: c00c5974 c6729eb0 c682d340 c5a02734
0003 00851d4a
[46796.564043] GPR08: 05ae 02b9 9032 01e4 24042424
1001c8cc 7fc835f8 100ad378
[46796.564043] GPR16: 7fc835f0 7fc835e8 7fc835e0 7fc835d8
7fc835d0 7fc835c8 7fc835c0
[46796.564043] GPR24: 0fe59f14 02ac c6a44b48 c6056110 c5e03168
c5a026e0 c6728000 c1a026e0
[46796.596017] NIP [c00c5978] destroy_inode+0x38/0x84
[46796.600736] LR [c00c5974] destroy_inode+0x34/0x84
[46796.605344] Call Trace:
[46796.607793] [c6729eb0] [c00c5974] destroy_inode+0x34/0x84 (unreliable)
[46796.614271] [c6729ec0] [c00c1d90] __dentry_kill+0x2a8/0x304
[46796.619763] [c6729ee0] [c00c27c8] dput+0xd0/0x1d8
[46796.624416] [c6729f00] [c00adf54] __fput+0x134/0x1fc
[46796.629319] [c6729f20] [c002de28] task_work_run+0xac/0xf4
[46796.634655] [c6729f40] [c000bba4] do_user_signal+0x74/0xc4
[46796.640023] Instruction dump:
[46796.642955] 39430078 93e1000c 90010014 7c7f1b78 81230078 7d295278
7d290034 5529d97e
[46796.650612] 69290001 0f09 4b45 813f0014 <81290024> 81290004
2f89 419e0020
[46796.658466] ---[ end trace 0abe99599a8bf31d ]---
c00c5940 :
struct inode *inode = container_of(head, struct inode, i_rcu);
kmem_cache_free(inode_cachep, inode);
}
static void destroy_inode(struct inode *inode)
{
c00c5940:7c 08 02 a6 mflrr0
c00c5944:94 21 ff f0 stwur1,-16(r1)
BUG_ON(!list_empty(&inode->i_lru));
c00c5948:39 43 00 78 addir10,r3,120
struct inode *inode = container_of(head, struct inode, i_rcu);
kmem_cache_free(inode_cachep, inode);
}
static void destroy_inode(struct inode *inode)
{
c00c594c:93 e1 00 0c stw r31,12(r1)
c00c5950:90 01 00 14 stw r0,20(r1)
c00c5954:7c 7f 1b 78 mr r31,r3
BUG_ON(!list_empty(&inode->i_lru));
c00c5958:81 23 00 78 lwz r9,120(r3)
c00c595c:7d 29 52 78 xor r9,r9,r10
c00c5960:7d 29 00 34 cntlzw r9,r9
c00c5964:55 29 d9 7e rlwinm r9,r9,27,5,31
c00c5968:69 29 00 01 xorir9,r9,1
c00c596c:0f 09 00 00 twnei r9,0
__destroy_inode(inode);
c00c5970:4b ff ff 45 bl c00c58b4 <__destroy_inode>
if (inode->i_sb->s_op->destroy_inode)
c00c5974:81 3f 00 14 lwz r9,20(r31)
==> c00c5978:81 29 00 24 lwz r9,36(r9)
c00c597c:81 29 00 04 lwz r9,4(r9)
c00c5980:2f 89 00 00 cmpwi cr7,r9,0
c00c5984:41 9e 00 20 beq cr7,c00c59a4
inode->i_sb->s_op->destroy_inode(inode);
else
call_rcu(&inode->i_rcu, i_callback);
}
c00c5988:80 01 00 14 lwz r0,20(r1)
Looks like inode->i_sb (apparently contained in r9) has value 0x2b9
which is obviously wrong, hence the bad access at 0x2dd when trying to
get inode->i_sb->s_op
What else can I look at to investigate this issue ?
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 03/19] powerpc: Mark variables as unused
Michael Ellerman a écrit :
LEROY Christophe writes:
Mathieu Malaterre a écrit :
Add gcc attribute unused for two variables. Fix warnings treated as errors
with W=1:
arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set
but not used [-Werror=unused-but-set-variable]
Suggested-by: Christophe Leroy
Signed-off-by: Mathieu Malaterre
---
v2: move path within ifdef DEBUG_PROM
arch/powerpc/kernel/prom_init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c
b/arch/powerpc/kernel/prom_init.c
index acf4b2e0530c..4163b11abb6c 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
const char *opt;
char *p;
- int l = 0;
+ int l __maybe_unused = 0;
prom_cmd_line[0] = 0;
p = prom_cmd_line;
@@ -1385,7 +1385,7 @@ static void __init reserve_mem(u64 base, u64 size)
static void __init prom_init_mem(void)
{
phandle node;
- char *path, type[64];
+ char *path __maybe_unused, type[64];
You should enclose that in an ifdef DEBUG_PROM instead of hiding the warning
I disagree, the result is horrible:
static void __init prom_init_mem(void)
{
phandle node;
- char *path, type[64];
+#ifdef DEBUG_PROM
+ char *path;
+#endif
+ char type[64];
unsigned int plen;
cell_t *p, *endp;
__be32 val;
The right fix is to move the debug logic into a helper, and put the path
in there, eg. something like (not tested):
diff --git a/arch/powerpc/kernel/prom_init.c
b/arch/powerpc/kernel/prom_init.c
index f9d6befb55a6..b02fa2ccc70b 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1389,6 +1389,18 @@ static void __init reserve_mem(u64 base, u64 size)
mem_reserve_cnt = cnt + 1;
}
+#ifdef DEBUG_PROM
+static void prom_debug_path(phandle node)
+{
+ char *path;
+ path = prom_scratch;
+ memset(path, 0, PROM_SCRATCH_SIZE);
+ call_prom("package-to-path", 3, 1, node, path, PROM_SCRATCH_SIZE-1);
+ prom_debug(" node %s :\n", path);
+}
+#else
+static void prom_debug_path(phandle node) { }
Or put the ifdef inside the function to avoid double definition ?
+#endif /* DEBUG_PROM */
/*
* Initialize memory allocation mechanism, parse "memory" nodes and
* obtain that way the top of memory and RMO to setup out local allocator
@@ -1441,11 +1453,7 @@ static void __init prom_init_mem(void)
p = regbuf;
endp = p + (plen / sizeof(cell_t));
-#ifdef DEBUG_PROM
- memset(path, 0, PROM_SCRATCH_SIZE);
- call_prom("package-to-path", 3, 1, node, path,
PROM_SCRATCH_SIZE-1);
- prom_debug(" node %s :\n", path);
-#endif /* DEBUG_PROM */
+ prom_debug_path(node);
while ((endp - p) >= (rac + rsc)) {
unsigned long base, size;
Although that also begs the question of why the hell do we need path at
all, and not just use prom_scratch directly?
Wondering the same, why not use prom_scratch directly
Christophe
cheers
Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
Mathieu Malaterre a écrit :
Christophe,
On Sat, Mar 24, 2018 at 9:10 PM, LEROY Christophe
wrote:
Mathieu Malaterre a écrit :
On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy
wrote:
Le 22/03/2018 à 21:20, Mathieu Malaterre a écrit :
Some functions prototypes were missing for the non-altivec code. Add the
missing prototypes directly in xor_vmx, fix warnings treated as errors
with
W=1:
arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototype
for
‘xor_altivec_2’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototype
for
‘xor_altivec_3’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototype
for
‘xor_altivec_4’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototype
for
‘xor_altivec_5’ [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre
---
arch/powerpc/lib/xor_vmx.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx.h
index 5c2b0839b179..2173e3c84151 100644
--- a/arch/powerpc/lib/xor_vmx.h
+++ b/arch/powerpc/lib/xor_vmx.h
@@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsigned
long
*v1_in,
void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
unsigned long *v2_in, unsigned long *v3_in,
unsigned long *v4_in, unsigned long
*v5_in);
+
+void xor_altivec_2(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in);
+
Only used in one place, should be static instead of adding it in a .h
Same for the other ones.
$ git grep xor_altivec_2
[...]
arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2);
Are you sure I can change this function to static ?
Yes you are right. But in fact those fonctions are already defined in
asm/xor. h
So you just need to add the missing #include
I originally tried it, but this leads to:
CC arch/powerpc/lib/xor_vmx_glue.o
In file included from arch/powerpc/lib/xor_vmx_glue.c:16:0:
./arch/powerpc/include/asm/xor.h:39:15: error: variable
‘xor_block_altivec’ has initializer but incomplete type
static struct xor_block_template xor_block_altivec = {
^~
./arch/powerpc/include/asm/xor.h:40:2: error: unknown field ‘name’
specified in initializer
.name = "altivec",
^
[...]
The file (powerpc) is pretty much expected to be included
after .
I did not want to tweak to test for #ifdef _XOR_H just before
#ifdef _XOR_H
static struct xor_block_template xor_block_altivec = {
[...]
since this seems like a hack to me.
Is this ok to test for #ifdef _XOR_H in ?
What about including linux/raid/xor.h in asm/xor.h ?
Christophe
Christophe
Christophe
+void xor_altivec_3(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long
*v3_in);
+
+void xor_altivec_4(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long *v3_in,
+unsigned long *v4_in);
+
+void xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long *v3_in,
+unsigned long *v4_in, unsigned long
*v5_in);
---
L'absence de virus dans ce courrier électronique a été vérifiée par le
logiciel antivirus Avast.
https://www.avast.com/antivirus
Re: [PATCH 14/19] powerpc/altivec: Add missing prototypes for altivec
LEROY Christophe a écrit :
Mathieu Malaterre a écrit :
Christophe,
On Sat, Mar 24, 2018 at 9:10 PM, LEROY Christophe
wrote:
Mathieu Malaterre a écrit :
On Fri, Mar 23, 2018 at 1:19 PM, christophe leroy
wrote:
Le 22/03/2018 à 21:20, Mathieu Malaterre a écrit :
Some functions prototypes were missing for the non-altivec code. Add the
missing prototypes directly in xor_vmx, fix warnings treated as errors
with
W=1:
arch/powerpc/lib/xor_vmx_glue.c:18:6: error: no previous prototype
for
‘xor_altivec_2’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:29:6: error: no previous prototype
for
‘xor_altivec_3’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:40:6: error: no previous prototype
for
‘xor_altivec_4’ [-Werror=missing-prototypes]
arch/powerpc/lib/xor_vmx_glue.c:52:6: error: no previous prototype
for
‘xor_altivec_5’ [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre
---
arch/powerpc/lib/xor_vmx.h | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/lib/xor_vmx.h b/arch/powerpc/lib/xor_vmx.h
index 5c2b0839b179..2173e3c84151 100644
--- a/arch/powerpc/lib/xor_vmx.h
+++ b/arch/powerpc/lib/xor_vmx.h
@@ -19,3 +19,17 @@ void __xor_altivec_4(unsigned long bytes, unsigned
long
*v1_in,
void __xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
unsigned long *v2_in, unsigned long *v3_in,
unsigned long *v4_in, unsigned long
*v5_in);
+
+void xor_altivec_2(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in);
+
Only used in one place, should be static instead of adding it in a .h
Same for the other ones.
$ git grep xor_altivec_2
[...]
arch/powerpc/lib/xor_vmx_glue.c:EXPORT_SYMBOL(xor_altivec_2);
Are you sure I can change this function to static ?
Yes you are right. But in fact those fonctions are already defined in
asm/xor. h
So you just need to add the missing #include
I originally tried it, but this leads to:
CC arch/powerpc/lib/xor_vmx_glue.o
In file included from arch/powerpc/lib/xor_vmx_glue.c:16:0:
./arch/powerpc/include/asm/xor.h:39:15: error: variable
‘xor_block_altivec’ has initializer but incomplete type
static struct xor_block_template xor_block_altivec = {
^~
./arch/powerpc/include/asm/xor.h:40:2: error: unknown field ‘name’
specified in initializer
.name = "altivec",
^
[...]
The file (powerpc) is pretty much expected to be included
after .
I did not want to tweak to test for #ifdef _XOR_H just before
#ifdef _XOR_H
static struct xor_block_template xor_block_altivec = {
[...]
since this seems like a hack to me.
Is this ok to test for #ifdef _XOR_H in ?
What about including linux/raid/xor.h in asm/xor.h ?
Or better: including linux/raid/xor.h then asm/xor.h in xor_vmx_glue.c ?
Christophe
Christophe
Christophe
Christophe
+void xor_altivec_3(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long
*v3_in);
+
+void xor_altivec_4(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long *v3_in,
+unsigned long *v4_in);
+
+void xor_altivec_5(unsigned long bytes, unsigned long *v1_in,
+unsigned long *v2_in, unsigned long *v3_in,
+unsigned long *v4_in, unsigned long
*v5_in);
---
L'absence de virus dans ce courrier électronique a été vérifiée par le
logiciel antivirus Avast.
https://www.avast.com/antivirus
Re: [PATCH 11/19] powerpc/powermac: Move pmac_pfunc_base_install prototype to header file
Mathieu Malaterre a écrit : On Fri, Mar 23, 2018 at 1:13 PM, christophe leroy wrote: Le 22/03/2018 à 21:19, Mathieu Malaterre a écrit : The pmac_pfunc_base_install prototype was declared in powermac/smp.c since function was used there, move it to pmac_pfunc.h header to be visible in pfunc_base.c. Fix a warning treated as error with W=1: arch/powerpc/platforms/powermac/pfunc_base.c:330:12: error: no previous prototype for ‘pmac_pfunc_base_install’ [-Werror=missing-prototypes] Signed-off-by: Mathieu Malaterre --- arch/powerpc/include/asm/pmac_pfunc.h | 1 + arch/powerpc/platforms/powermac/smp.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/pmac_pfunc.h b/arch/powerpc/include/asm/pmac_pfunc.h index 73bd8f28f2a8..99f7a288789a 100644 --- a/arch/powerpc/include/asm/pmac_pfunc.h +++ b/arch/powerpc/include/asm/pmac_pfunc.h @@ -245,6 +245,7 @@ extern void pmf_put_function(struct pmf_function *func); extern int pmf_call_one(struct pmf_function *func, struct pmf_args *args); +extern int pmac_pfunc_base_install(void); extern keyword is not needed I understand; but for consistency every single protoypes in this header file actually use the extern keyword. Is there a guide/best practice to refer to in this case ? Consistancy is not a valid reason to continue bad practice. Every single modufication is an opportunity to clean things up You should run script/checkpatch.pl --strict on all your patches before submitting. And follow as much as possible the linux codying style Christophe Christophe /* Suspend/resume code called by via-pmu directly for now */ extern void pmac_pfunc_base_suspend(void); diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c index 95275e0e2efa..447da6db450a 100644 --- a/arch/powerpc/platforms/powermac/smp.c +++ b/arch/powerpc/platforms/powermac/smp.c @@ -65,7 +65,6 @@ #endif extern void __secondary_start_pmac_0(void); -extern int pmac_pfunc_base_install(void); static void (*pmac_tb_freeze)(int freeze); static u64 timebase; --- L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. https://www.avast.com/antivirus
Re: [PATCH 15/19] powerpc: Add missing prototype
Mathieu Malaterre a écrit : On Fri, Mar 23, 2018 at 1:20 PM, christophe leroy wrote: Le 22/03/2018 à 21:20, Mathieu Malaterre a écrit : Add one missing prototype for function rh_dump_blk. Fix warning treated as error in W=1: arch/powerpc/lib/rheap.c:740:6: error: no previous prototype for ‘rh_dump_blk’ [-Werror=missing-prototypes] Signed-off-by: Mathieu Malaterre --- arch/powerpc/include/asm/rheap.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/powerpc/include/asm/rheap.h b/arch/powerpc/include/asm/rheap.h index 172381769cfc..e75d96de19a0 100644 --- a/arch/powerpc/include/asm/rheap.h +++ b/arch/powerpc/include/asm/rheap.h @@ -83,6 +83,9 @@ extern int rh_get_stats(rh_info_t * info, int what, int max_stats, /* Simple dump of remote heap info */ extern void rh_dump(rh_info_t * info); +/* Simple dump of remote info block */ +extern void rh_dump_blk(rh_info_t *info, rh_block_t *blk); + Only used in one place, should be static Well here is what I see over here: $ git grep rh_dump_blk ... arch/powerpc/lib/rheap.c:EXPORT_SYMBOL_GPL(rh_dump_blk); If it was really used by anybody in a module, it would already be in a .h so I think we should simply delete the function Christophe Christophe /* Set owner of taken block */ extern int rh_set_owner(rh_info_t * info, unsigned long start, const char *owner); --- L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. https://www.avast.com/antivirus
Re: [PATCH v2 01/19] powerpc/powermac: Mark variable x as unused
Mathieu Malaterre a écrit :
Since the value of x is never intended to be read, declare it with gcc
attribute as unused. Fix warning treated as error with W=1:
arch/powerpc/platforms/powermac/bootx_init.c:471:21: error:
variable ‘x’ set but not used [-Werror=unused-but-set-variable]
Signed-off-by: Mathieu Malaterre
---
v2: move x variable within its local scope
arch/powerpc/platforms/powermac/bootx_init.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powermac/bootx_init.c
b/arch/powerpc/platforms/powermac/bootx_init.c
index c3c9bbb3573a..d44e8571c1ec 100644
--- a/arch/powerpc/platforms/powermac/bootx_init.c
+++ b/arch/powerpc/platforms/powermac/bootx_init.c
@@ -468,7 +468,7 @@ void __init bootx_init(unsigned long r3,
unsigned long r4)
boot_infos_t *bi = (boot_infos_t *) r4;
unsigned long hdr;
unsigned long space;
- unsigned long ptr, x;
+ unsigned long ptr;
char *model;
unsigned long offset = reloc_offset();
@@ -562,6 +562,7 @@ void __init bootx_init(unsigned long r3,
unsigned long r4)
* MMU switched OFF, so this should not be useful anymore.
*/
if (bi->version < 4) {
+ unsigned long x __maybe_unused;
bootx_printf("Touching pages...\n");
Stylewise, there should be an empty line after your declaration.
But I believe you should remove that ugly loop and replace it by a
call to fault_in_pages_readable()
Christophe
/*
--
2.11.0
Re: [PATCH v2 02/19] powerpc/powermac: Mark variable x as unused
Mathieu Malaterre a écrit : Since the value of x is never intended to be read, remove it. Fix warning treated as error with W=1: arch/powerpc/platforms/powermac/udbg_scc.c:76:9: error: variable ‘x’ set but not used [-Werror=unused-but-set-variable] Suggested-by: Christophe Leroy Signed-off-by: Mathieu Malaterre Reviewed-by: Christophe Leroy --- v2: remove x completely arch/powerpc/platforms/powermac/udbg_scc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/powermac/udbg_scc.c b/arch/powerpc/platforms/powermac/udbg_scc.c index d83135a9830e..8901973ed683 100644 --- a/arch/powerpc/platforms/powermac/udbg_scc.c +++ b/arch/powerpc/platforms/powermac/udbg_scc.c @@ -73,7 +73,7 @@ void udbg_scc_init(int force_scc) struct device_node *stdout = NULL, *escc = NULL, *macio = NULL; struct device_node *ch, *ch_def = NULL, *ch_a = NULL; const char *path; - int i, x; + int i; escc = of_find_node_by_name(NULL, "escc"); if (escc == NULL) @@ -120,7 +120,7 @@ void udbg_scc_init(int force_scc) mb(); for (i = 2; i != 0; --i) - x = in_8(sccc); + in_8(sccc); out_8(sccc, 0x09); /* reset A or B side */ out_8(sccc, 0xc0); -- 2.11.0
Re: [PATCH v2 03/19] powerpc: Mark variables as unused
Mathieu Malaterre a écrit :
Add gcc attribute unused for two variables. Fix warnings treated as errors
with W=1:
arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set
but not used [-Werror=unused-but-set-variable]
Suggested-by: Christophe Leroy
Signed-off-by: Mathieu Malaterre
---
v2: move path within ifdef DEBUG_PROM
arch/powerpc/kernel/prom_init.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c
b/arch/powerpc/kernel/prom_init.c
index acf4b2e0530c..4163b11abb6c 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
const char *opt;
char *p;
- int l = 0;
+ int l __maybe_unused = 0;
prom_cmd_line[0] = 0;
p = prom_cmd_line;
@@ -1385,7 +1385,7 @@ static void __init reserve_mem(u64 base, u64 size)
static void __init prom_init_mem(void)
{
phandle node;
- char *path, type[64];
+ char *path __maybe_unused, type[64];
You should enclose that in an ifdef DEBUG_PROM instead of hiding the warning
Christophe
unsigned int plen;
cell_t *p, *endp;
__be32 val;
@@ -1406,7 +1406,6 @@ static void __init prom_init_mem(void)
prom_debug("root_size_cells: %x\n", rsc);
prom_debug("scanning memory:\n");
- path = prom_scratch;
for (node = 0; prom_next_node(&node); ) {
type[0] = 0;
@@ -1431,6 +1430,7 @@ static void __init prom_init_mem(void)
endp = p + (plen / sizeof(cell_t));
#ifdef DEBUG_PROM
+ path = prom_scratch;
memset(path, 0, PROM_SCRATCH_SIZE);
call_prom("package-to-path", 3, 1, node, path,
PROM_SCRATCH_SIZE-1);
prom_debug(" node %s :\n", path);
--
2.11.0
Re: [PATCH v2 05/19] powerpc/chrp/setup: Add attribute unused and make some functions static
The subject of the patch should be updated as well
Christophe
Mathieu Malaterre a écrit :
Remove variable declaration idu_size and associated code since not used.
These functions can all be static, make it so. Fix warnings treated as
errors with W=1:
arch/powerpc/platforms/chrp/setup.c:97:6: error: no previous
prototype for ‘chrp_show_cpuinfo’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:302:13: error: no previous
prototype for ‘chrp_setup_arch’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:385:16: error: variable
‘idu_size’ set but not used [-Werror=unused-but-set-variable]
arch/powerpc/platforms/chrp/setup.c:526:13: error: no previous
prototype for ‘chrp_init_IRQ’ [-Werror=missing-prototypes]
arch/powerpc/platforms/chrp/setup.c:559:1: error: no previous
prototype for ‘chrp_init2’ [-Werror=missing-prototypes]
Suggested-by: Christophe Leroy
Signed-off-by: Mathieu Malaterre
---
v2: Remove totally variable idu_size
arch/powerpc/platforms/chrp/setup.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/chrp/setup.c
b/arch/powerpc/platforms/chrp/setup.c
index 481ed133e04b..d6d8ffc0271e 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -94,7 +94,7 @@ static const char *chrp_names[] = {
"Total Impact Briq"
};
-void chrp_show_cpuinfo(struct seq_file *m)
+static void chrp_show_cpuinfo(struct seq_file *m)
{
int i, sdramen;
unsigned int t;
@@ -299,7 +299,7 @@ static __init void chrp_init(void)
of_node_put(node);
}
-void __init chrp_setup_arch(void)
+static void __init chrp_setup_arch(void)
{
struct device_node *root = of_find_node_by_path("/");
const char *machine = NULL;
@@ -382,7 +382,7 @@ static void __init chrp_find_openpic(void)
{
struct device_node *np, *root;
int len, i, j;
- int isu_size, idu_size;
+ int isu_size;
const unsigned int *iranges, *opprop = NULL;
int oplen = 0;
unsigned long opaddr;
@@ -427,11 +427,9 @@ static void __init chrp_find_openpic(void)
}
isu_size = 0;
- idu_size = 0;
if (len > 0 && iranges[1] != 0) {
printk(KERN_INFO "OpenPIC irqs %d..%d in IDU\n",
iranges[0], iranges[0] + iranges[1] - 1);
- idu_size = iranges[1];
}
if (len > 1)
isu_size = iranges[3];
@@ -523,7 +521,7 @@ static void __init chrp_find_8259(void)
}
}
-void __init chrp_init_IRQ(void)
+static void __init chrp_init_IRQ(void)
{
#if defined(CONFIG_VT) && defined(CONFIG_INPUT_ADBHID) &&
defined(CONFIG_XMON)
struct device_node *kbd;
@@ -555,7 +553,7 @@ void __init chrp_init_IRQ(void)
#endif
}
-void __init
+static void __init
chrp_init2(void)
{
#ifdef CONFIG_NVRAM
--
2.11.0
Re: [PATCH v2 04/19] powerpc/kvm: Prefer fault_in_pages_readable function
Mathieu Malaterre a écrit :
Directly use fault_in_pages_readable instead of manual __get_user code. Fix
warning treated as error with W=1:
arch/powerpc/kernel/kvm.c:675:6: error: variable ‘tmp’ set but not
used [-Werror=unused-but-set-variable]
Suggested-by: Christophe Leroy
Signed-off-by: Mathieu Malaterre
Reviewed-by: Christophe Leroy
---
v2: use fault_in_pages_readable instead
arch/powerpc/kernel/kvm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
index 9ad37f827a97..124c51030b75 100644
--- a/arch/powerpc/kernel/kvm.c
+++ b/arch/powerpc/kernel/kvm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
@@ -672,14 +673,13 @@ static void kvm_use_magic_page(void)
{
u32 *p;
u32 *start, *end;
- u32 tmp;
u32 features;
/* Tell the host to map the magic page to -4096 on all CPUs */
on_each_cpu(kvm_map_magic_page, &features, 1);
/* Quick self-test to see if the mapping works */
- if (__get_user(tmp, (u32*)KVM_MAGIC_PAGE)) {
+ if (!fault_in_pages_readable((const char *)KVM_MAGIC_PAGE,
sizeof(u32))) {
kvm_patching_worked = false;
return;
}
--
2.11.0
Re: [PATCH] powerpc 8xx: Fixing memory init issue with CONFIG_PIN_TLB
Le 15/10/2013 22:33, Scott Wood a écrit : On Tue, 2013-10-15 at 18:27 +0200, leroy christophe wrote: Le 11/10/2013 17:13, Joakim Tjernlund a écrit : "Linuxppc-dev" wrote on 2013/10/11 14:56:40: Activating CONFIG_PIN_TLB allows access to the 24 first Mbytes of memory at bootup instead of 8. It is needed for "big" kernels for instance when activating CONFIG_LOCKDEP_SUPPORT. This needs to be taken into account in init_32 too, otherwise memory allocation soon fails after startup. Signed-off-by: Christophe Leroy diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S --- linux-3.11.org/arch/powerpc/mm/init_32.c 2013-09-02 22:46:10.0 +0200 +++ linux-3.11/arch/powerpc/mm/init_32.c 2013-09-09 11:28:54.0 +0200 @@ -213,7 +213,12 @@ */ BUG_ON(first_memblock_base != 0); +#ifdef CONFIG_PIN_TLB + /* 8xx can only access 24MB at the moment */ + memblock_set_current_limit(min_t(u64, first_memblock_size, 0x0180)); +#else /* 8xx can only access 8MB at the moment */ memblock_set_current_limit(min_t(u64, first_memblock_size, 0x0080)); +#endif } #endif /* CONFIG_8xx */ hmm, I think you should always map 24 MB (or less if RAM < 24 MB) and do the same in head_8xx.S. Or to keep it simple, just always map at least 16 MB here and in head_8xx.S, assuming that 16 MB is min RAM for any 8xx system running 3.x kernels. Yes we could do a more elaborated modification in the future. However it also has an impact on the boot loader, so I'm not sure we should make it the default without thinking twice. In the meantime, my patch does take into account the existing situation where you have 8Mb by default and 24Mb when you activate CONFIG_PIN_TLB. I see it as a bug fix and I believe we should include it at least in order to allow including in the stable releases. Do you see any issue with this approach ? The patch is fine, but I don't think it's stable material (BTW, if it were, you should have marked it as such when submitting). If I understand the situation correctly, there's no regression, and nothing fails to work with CONFIG_PIN_TLB that would have worked without it. It's just making CONFIG_PIN_TLB more useful. Yes the patch is definitly stable. How should I have mark it ? The situation is that in 2010, I discovered that I was not able to start a big Kernel because of the 8Mb limit. You told me (see attached mail) that in order to get rid of that limit I shall use CONFIG_PIN_TLB: it was the first step, it helped pass the memory zeroize at init, but it was not enough as I then got problems with the Device Tree being erased because being put inside the first 8Mb area too. Then I temporarely gave up at that time. Recently I started again. After fixing my bootloader to get the device tree somewhere else, I discovered this 8Mb limit hardcoded in mm/init_32.c for the 8xx With the patch I submitted I can now boot a kernel which is bigger than 8Mb. So, I'm a bit lost here on what to do. --- Begin Message --- On Tue, 2013-10-15 at 18:27 +0200, leroy christophe wrote: > Le 11/10/2013 17:13, Joakim Tjernlund a écrit : > > "Linuxppc-dev" > > > > wrote on 2013/10/11 14:56:40: > >> Activating CONFIG_PIN_TLB allows access to the 24 first Mbytes of memory > > at > >> bootup instead of 8. It is needed for "big" kernels for instance when > > activating > >> CONFIG_LOCKDEP_SUPPORT. This needs to be taken into account in init_32 > > too, > >> otherwise memory allocation soon fails after startup. > >> > >> Signed-off-by: Christophe Leroy > >> > >> diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S > > linux-3.11/arch/powerpc/kernel/head_8xx.S > >> --- linux-3.11.org/arch/powerpc/mm/init_32.c 2013-09-02 > > 22:46:10.0 +0200 > >> +++ linux-3.11/arch/powerpc/mm/init_32.c 2013-09-09 11:28:54.0 > > +0200 > >> @@ -213,7 +213,12 @@ > >> */ > >> BUG_ON(first_memblock_base != 0); > >> > >> +#ifdef CONFIG_PIN_TLB > >> + /* 8xx can only access 24MB at the moment */ > >> + memblock_set_current_limit(min_t(u64, first_memblock_size, > > 0x0180)); > >> +#else > >> /* 8xx can only access 8MB at the moment */ > >> memblock_set_current_limit(min_t(u64, first_memblock_size, > > 0x0080)); > >> +#endif > >> } > >> #endif /* CONFIG_8xx */ > > hmm, I think you should always map 24 MB (or less if RAM < 24 MB) and do > > the same > > in head_8xx.S. > > > > Or to keep it simple, just always map at least 16 MB here and in > > head_8xx.S, assuming > > that 16 MB is min RAM for any 8xx syst
BUG: Patch "Convert some mftb/mftbu into mfspr" breaks MPC885
Scott, The patch "Convert some mftb/mftbu into mfspr" (beb2dc0a7a84be003ce54e98b95d65cc66e6e536) breaks startup on MPC885. The CPU traps (SoftwareEmulation trap) at sched_clock() when trying to read TBU with mfspr. Reverting the patch solves the issue. What's the prefered way to fix this ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc 8xx: Fixing memory init issue with CONFIG_PIN_TLB
Le 11/10/2013 17:13, Joakim Tjernlund a écrit : "Linuxppc-dev" wrote on 2013/10/11 14:56:40: Activating CONFIG_PIN_TLB allows access to the 24 first Mbytes of memory at bootup instead of 8. It is needed for "big" kernels for instance when activating CONFIG_LOCKDEP_SUPPORT. This needs to be taken into account in init_32 too, otherwise memory allocation soon fails after startup. Signed-off-by: Christophe Leroy diff -ur linux-3.11.org/arch/powerpc/kernel/head_8xx.S linux-3.11/arch/powerpc/kernel/head_8xx.S --- linux-3.11.org/arch/powerpc/mm/init_32.c 2013-09-02 22:46:10.0 +0200 +++ linux-3.11/arch/powerpc/mm/init_32.c 2013-09-09 11:28:54.0 +0200 @@ -213,7 +213,12 @@ */ BUG_ON(first_memblock_base != 0); +#ifdef CONFIG_PIN_TLB + /* 8xx can only access 24MB at the moment */ + memblock_set_current_limit(min_t(u64, first_memblock_size, 0x0180)); +#else /* 8xx can only access 8MB at the moment */ memblock_set_current_limit(min_t(u64, first_memblock_size, 0x0080)); +#endif } #endif /* CONFIG_8xx */ hmm, I think you should always map 24 MB (or less if RAM < 24 MB) and do the same in head_8xx.S. Or to keep it simple, just always map at least 16 MB here and in head_8xx.S, assuming that 16 MB is min RAM for any 8xx system running 3.x kernels. Yes we could do a more elaborated modification in the future. However it also has an impact on the boot loader, so I'm not sure we should make it the default without thinking twice. In the meantime, my patch does take into account the existing situation where you have 8Mb by default and 24Mb when you activate CONFIG_PIN_TLB. I see it as a bug fix and I believe we should include it at least in order to allow including in the stable releases. Do you see any issue with this approach ? Much of the need for pinning would go away if you adopted my 8MB pages from 2.4 to 3.x Jocke Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] watchdog: mpc8xxx_wdt convert to watchdog core
Le 01/12/2013 20:38, Guenter Roeck a écrit :
On 11/30/2013 07:33 AM, Christophe Leroy wrote:
Convert mpc8xxx_wdt.c to the new watchdog API.
Signed-off-by: Christophe Leroy
diff -ur a/drivers/watchdog/mpc8xxx_wdt.c
b/drivers/watchdog/mpc8xxx_wdt.c
--- a/drivers/watchdog/mpc8xxx_wdt.c2013-05-11 22:57:46.0
+0200
+++ b/drivers/watchdog/mpc8xxx_wdt.c2013-11-30 16:14:53.803495472
+0100
@@ -72,28 +72,31 @@
* to 0
*/
static int prescale = 1;
-static unsigned int timeout_sec;
-static unsigned long wdt_is_open;
static DEFINE_SPINLOCK(wdt_spinlock);
-static void mpc8xxx_wdt_keepalive(void)
+static int mpc8xxx_wdt_ping(struct watchdog_device *w)
{
/* Ping the WDT */
spin_lock(&wdt_spinlock);
out_be16(&wd_base->swsrr, 0x556c);
out_be16(&wd_base->swsrr, 0xaa39);
spin_unlock(&wdt_spinlock);
+return 0;
The return code is never checked, so you can make this function void.
watchdog .h expects an int function.
Wouldn't it be an error to make it void, if for instance in the future
the return code is checked by the core ?
}
+static struct watchdog_device mpc8xxx_wdt_dev;
static void mpc8xxx_wdt_timer_ping(unsigned long arg);
-static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0, 0);
+static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0,
+(unsigned long)&mpc8xxx_wdt_dev);
static void mpc8xxx_wdt_timer_ping(unsigned long arg)
{
-mpc8xxx_wdt_keepalive();
+struct watchdog_device *w = (struct watchdog_device *)arg;
+
+mpc8xxx_wdt_ping(w);
/* We're pinging it twice faster than needed, just to be sure. */
-mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
+mod_timer(&wdt_timer, jiffies + HZ * w->timeout / 2);
}
static void mpc8xxx_wdt_pr_warn(const char *msg)
@@ -102,19 +105,9 @@
reset ? "reset" : "machine check exception");
}
-static ssize_t mpc8xxx_wdt_write(struct file *file, const char
__user *buf,
- size_t count, loff_t *ppos)
-{
-if (count)
-mpc8xxx_wdt_keepalive();
-return count;
-}
-
-static int mpc8xxx_wdt_open(struct inode *inode, struct file *file)
+static int mpc8xxx_wdt_start(struct watchdog_device *w)
{
u32 tmp = SWCRR_SWEN;
-if (test_and_set_bit(0, &wdt_is_open))
-return -EBUSY;
/* Once we start the watchdog we can't stop it */
if (nowayout)
This code and the subsequent module_get can be removed; it is handled
by the infrastructure.
@@ -132,59 +125,30 @@
del_timer_sync(&wdt_timer);
-return nonseekable_open(inode, file);
+return 0;
}
-static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
+static int mpc8xxx_wdt_stop(struct watchdog_device *w)
{
-if (!nowayout)
-mpc8xxx_wdt_timer_ping(0);
-else
-mpc8xxx_wdt_pr_warn("watchdog closed");
-clear_bit(0, &wdt_is_open);
+mpc8xxx_wdt_timer_ping((unsigned long)w);
I really dislike this typecasting back and forth.
I think it would be better to move the mod_timer() call from
mpc8xxx_wdt_timer_ping
into mpc8xxx_wdt_ping and call mpc8xxx_wdt_ping directly whenever
possible.
From mpc8xxx_wdt_timer_ping you can then just call mpc8xxx_wdt_ping
with the
typecasted parameter.
I'm not sure I understand what you mean.
mpc8xxx_wdt_ping() is called by the core when the user app writes into
/dev/watchdog.
We don't want the set the timer in that case.
The timer is only used for when no user app has opened /dev/watchdog yet.
return 0;
}
-static long mpc8xxx_wdt_ioctl(struct file *file, unsigned int cmd,
-unsigned long arg)
-{
-void __user *argp = (void __user *)arg;
-int __user *p = argp;
-static const struct watchdog_info ident = {
-.options = WDIOF_KEEPALIVEPING,
-.firmware_version = 1,
-.identity = "MPC8xxx",
-};
-
-switch (cmd) {
-case WDIOC_GETSUPPORT:
-return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
-case WDIOC_GETSTATUS:
-case WDIOC_GETBOOTSTATUS:
-return put_user(0, p);
-case WDIOC_KEEPALIVE:
-mpc8xxx_wdt_keepalive();
-return 0;
-case WDIOC_GETTIMEOUT:
-return put_user(timeout_sec, p);
-default:
-return -ENOTTY;
-}
-}
+static struct watchdog_info mpc8xxx_wdt_info = {
+.options = WDIOF_KEEPALIVEPING,
+.firmware_version = 1,
+.identity = "MPC8xxx",
+};
-static const struct file_operations mpc8xxx_wdt_fops = {
-.owner= THIS_MODULE,
-.llseek= no_llseek,
-.write= mpc8xxx_wdt_write,
-.unlocked_ioctl= mpc8xxx_wdt_ioctl,
-.open= mpc8xxx_wdt_open,
-.release= mpc8xxx_wdt_release,
+static struct watchdog_ops mpc8xxx_wdt_ops = {
+.owner = THIS_MODULE,
+.start = mpc8xxx_wdt_start,
+.stop = mpc8xxx_wdt_stop,
};
-static struct miscdevice mpc8xxx_wdt_miscdev = {
-.minor= WATCHDOG_MINOR,
-.name= "watchdog",
-.fops
Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB
Le 16/12/2013 23:57, Scott Wood a écrit : On Wed, 2013-12-11 at 00:36 +0100, leroy christophe wrote: Le 11/12/2013 00:18, Scott Wood a écrit : There wasn't previously an ifdef specifically around the setting of SPRN_MD_CTR. That's new. There was an ifdef around the entire block, which has gone away because you are now trying to map more than 8M regardless of CONFIG_PIN_TLB, but that has nothing to do with whether there should be an ifdef around SPRN_MD_CTR. Euh, ok, but then we have to fix it in the whole function, not only in this block. Do you think it is worth doing it ? Fix what in the whole function? I was asking what harm there would be if you just remove all the CONFIG_PIN_TLB ifdefs except around the actual RSV4I setting -- do we really care what value goes in MD_CTR for the non-pinned case, as long as it's different for each entry? MD_CTR is decremented after each entry added. However, the function populates entry 28, then 29, then 30, then 31. At the end MD_CTR has then value 30, ready to overide entry 30 then 29 then 28 then 27 . So I will remove all the CONFIG_PIN_TLB, but I'll also have to fix the value set in MD_CTR to start from 31, won't I ? Do you have any comment/recommendation on my tentative v3 patch where I have tried to implement based on the use of r7 as you recommended ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 03/21] powerpc/8xx: exception InstructionAccess does not exist on MPC8xx
Le 18/09/2014 17:15, Joakim Tjernlund a écrit : Christophe Leroy wrote on 2014/09/17 18:36:57: Exception InstructionAccess does not exist on MPC8xx. No need to branch there from somewhere else. Handling can be done directly in InstructionTLBError Exception. Signed-off-by: Christophe Leroy --- Changes in v2: - None Changes in v3: - arch/powerpc/mm/fault.c uses the vector number, so make sure it understand the new ones. arch/powerpc/kernel/head_8xx.S | 17 +++-- arch/powerpc/mm/fault.c| 1 + 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index 3af6db1..fbe5d10 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -234,15 +234,9 @@ DataAccess: EXC_XFER_LITE(0x300, handle_page_fault) /* Instruction access exception. - * This is "never generated" by the MPC8xx. We jump to it for other - * translation errors. + * This is "never generated" by the MPC8xx. */ - . = 0x400 -InstructionAccess: - EXCEPTION_PROLOG - mr r4,r12 - mr r5,r9 - EXC_XFER_LITE(0x400, handle_page_fault) + EXCEPTION(0x400, InstructionAccess, unknown_exception, EXC_XFER_STD) /* External interrupt */ EXCEPTION(0x500, HardwareInterrupt, do_IRQ, EXC_XFER_LITE) @@ -382,7 +376,7 @@ InstructionTLBMiss: #endif mfspr r10, SPRN_SPRG_SCRATCH2 EXCEPTION_EPILOG_0 - b InstructionAccess + b InstructionTLBError . = 0x1200 DataStoreTLBMiss: @@ -477,7 +471,10 @@ DataStoreTLBMiss: */ . = 0x1300 InstructionTLBError: - b InstructionAccess + EXCEPTION_PROLOG + mr r4,r12 + mr r5,r9 + EXC_XFER_LITE(0x1300, handle_page_fault) Still don't like that you change the vector number, every other ppc uses the standard number. Can you not just lie here(EXC_XFER_LITE(0x400, handle_page_fault))? Move the code to InstructionAccess too and let TLBError branch there. My issue was that if I do EXC_XFER_LITE(0x400, handle_page_fault), I can't leave the EXCEPTION(0x400, InstructionAccess, unknown_exception, EXC_XFER_STD) at address .400 Otherwise, I get twice the same label. What about the following patch then ? Would this be acceptable ? @@ -234,15 +234,13 @@ DataAccess: EXC_XFER_LITE(0x300, handle_page_fault) /* Instruction access exception. - * This is "never generated" by the MPC8xx. We jump to it for other - * translation errors. + * This is "never generated" by the MPC8xx. */ . = 0x400 InstructionAccess: EXCEPTION_PROLOG -mrr4,r12 -mrr5,r9 -EXC_XFER_LITE(0x400, handle_page_fault) +addir3,r1,STACK_FRAME_OVERHEAD +EXC_XFER_STD(0x401, unknown_exception) /* External interrupt */ EXCEPTION(0x500, HardwareInterrupt, do_IRQ, EXC_XFER_LITE) @@ -382,7 +380,7 @@ InstructionTLBMiss: #endif mfsprr10, SPRN_SPRG_SCRATCH2 EXCEPTION_EPILOG_0 -bInstructionAccess +bInstructionTLBError . = 0x1200 DataStoreTLBMiss: @@ -477,7 +475,11 @@ DataStoreTLBMiss: */ . = 0x1300 InstructionTLBError: -bInstructionAccess +EXCEPTION_PROLOG +mrr4,r12 +mrr5,r9 +/* 0x400 is InstructionAccess exception, needed by bad_page_fault() */ +EXC_XFER_LITE(0x400, handle_page_fault) /* This is the data TLB error on the MPC8xx. This could be due to * many reasons, including a dirty update to a pte. We can catch that If not I think you should keep the old/current way? Same for DataTLB Miss/Error /* This is the data TLB error on the MPC8xx. This could be due to * many reasons, including a dirty update to a pte. We can catch that diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 51ab9e7..4d63c96 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -526,6 +526,7 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) break; case 0x400: case 0x480: + case 0x1300: printk(KERN_ALERT "Unable to handle kernel paging request for " "instruction fetch\n"); break; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 03/21] powerpc/8xx: exception InstructionAccess does not exist on MPC8xx
Le 18/09/2014 22:02, Joakim Tjernlund a écrit : christophe leroy wrote on 2014/09/18 21:11:01: Le 18/09/2014 20:12, Joakim Tjernlund a écrit : leroy christophe wrote on 2014/09/18 18:42:14: Le 18/09/2014 17:15, Joakim Tjernlund a écrit : Christophe Leroy wrote on 2014/09/17 18:36:57: Exception InstructionAccess does not exist on MPC8xx. No need to branch there from somewhere else. Handling can be done directly in InstructionTLBError Exception. Signed-off-by: Christophe Leroy . = 0x1200 DataStoreTLBMiss: @@ -477,7 +475,11 @@ DataStoreTLBMiss: */ . = 0x1300 InstructionTLBError: -bInstructionAccess +EXCEPTION_PROLOG +mrr4,r12 +mrr5,r9 +/* 0x400 is InstructionAccess exception, needed by bad_page_fault() */ +EXC_XFER_LITE(0x400, handle_page_fault) You should have the code in TLBMiss and have the TLBError branch there as that is the common case. As far as I remember, I tried it already but we don't have enough space in TLBmiss for that. We can only have 40 instructions. Do your other optimizations first, then you might have space :) Even with the optimisation first, there is no chance to fit ITLBError instructions after ITLBMiss and before DTLBMiss. After optimisation, TLBMiss goes from 0x1100 to 0x1174. TLBError goes from 0x1300 to 0x13b4. DTLBMiss is at 0x1200 And this is without CPU6 ERRATA. So this is hopeless I believe. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB
Le 10/12/2013 23:24, Scott Wood a écrit : On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote: Today, the only way to load kernels whose size is greater than 8Mbytes is to activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is limited to 8Mbytes. This patch adds the capability to select the size of initial memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB is active or not. It allows to load "big" kernels (for instance when activating CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB. Signed-off-by: Christophe Leroy diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -980,6 +980,29 @@ config PIN_TLB bool "Pinned Kernel TLBs (860 ONLY)" depends on ADVANCED_OPTIONS && 8xx + +choice + prompt "Initial Data Memory Mapped on 8xx" + default 8xx_MAP_8M + depends on ADVANCED_OPTIONS && 8xx + +config 8xx_INIT_MAP_8M + bool "8 Mbytes" + +config 8xx_INIT_MAP_16M + bool "16 Mbytes" + +config 8xx_INIT_MAP_24M + bool "24 Mbytes" Are you working with a loader that passes initial-mapped-area size in r7 as per ePAPR? If so, we could rely on that at runtime. If you're using a non-ancient U-Boot, it should qualify here even if it's not fully ePAPR compliant (it passes the value of the bootm_mapsize variable in r7). Ok, let me check that. But it means that the size of the kernel I can boot will depend on the initial memory mapped by uboot ? Isn't it limitating ? Even if uboot only maps 8Mbytes, why couldn't I be allowed to boot a kernel having 10 Mbytes data if I have 32 Mbytes mem on the board ? I don't like the idea of having to change the bootloader just because I want to activate CONFIG_LOCKDEP to debug my kernel. -#ifdef CONFIG_PIN_TLB +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M) /* Map two more 8M kernel data pages. */ +#ifdef CONFIG_PIN_TLB addir10, r10, 0x0100 mtspr SPRN_MD_CTR, r10 +#endif lis r8, KERNELBASE@h /* Create vaddr for TLB */ addis r8, r8, 0x0080 /* Add 8M */ @@ -858,15 +860,19 @@ addis r11, r11, 0x0080/* Add 8M */ mtspr SPRN_MD_RPN, r11 +#ifdef CONFIG_8xx_INIT_MAP_24M +#ifdef CONFIG_PIN_TLB addir10, r10, 0x0100 mtspr SPRN_MD_CTR, r10 +#endif Are these ifdefs for CONFIG_PIN_TLB really needed? It shouldn't harm anything to use those entries even if they're not being pinned. I'm not sure I understand your comment. ifdef for CONFIG_PIN_TLB was already there before, but was enclosing the whole block, so 24 Mbytes were automatically mapped when you selected CONFIG_PIN_TLB and only 8 Mbytes were mapped when you didn't select CONFIG_PIN_TLB. I reduced the scope of those ifdefs so that they now apply on the pinning only. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc 8xx: Loading kernels over 8Mbytes without CONFIG_PIN_TLB
Le 11/12/2013 00:18, Scott Wood a écrit : On Wed, 2013-12-11 at 00:05 +0100, leroy christophe wrote: Le 10/12/2013 23:24, Scott Wood a écrit : On Tue, 2013-12-10 at 12:29 +0100, Christophe Leroy wrote: Today, the only way to load kernels whose size is greater than 8Mbytes is to activate CONFIG_PIN_TLB. Otherwise, the physical memory initially mapped is limited to 8Mbytes. This patch adds the capability to select the size of initial memory between 8/16/24 Mbytes and this is regardless of whether CONFIG_PIN_TLB is active or not. It allows to load "big" kernels (for instance when activating CONFIG_LOCKDEP_SUPPORT) without having to activate CONFIG_PIN_TLB. Signed-off-by: Christophe Leroy diff -ur a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -980,6 +980,29 @@ config PIN_TLB bool "Pinned Kernel TLBs (860 ONLY)" depends on ADVANCED_OPTIONS && 8xx + +choice + prompt "Initial Data Memory Mapped on 8xx" + default 8xx_MAP_8M + depends on ADVANCED_OPTIONS && 8xx + +config 8xx_INIT_MAP_8M + bool "8 Mbytes" + +config 8xx_INIT_MAP_16M + bool "16 Mbytes" + +config 8xx_INIT_MAP_24M + bool "24 Mbytes" Are you working with a loader that passes initial-mapped-area size in r7 as per ePAPR? If so, we could rely on that at runtime. If you're using a non-ancient U-Boot, it should qualify here even if it's not fully ePAPR compliant (it passes the value of the bootm_mapsize variable in r7). Ok, let me check that. But it means that the size of the kernel I can boot will depend on the initial memory mapped by uboot ? Isn't it limitating ? The ePAPR IMA is supposed to be large enough to include the OS image, device tree, etc. Even if uboot only maps 8Mbytes, why couldn't I be allowed to boot a kernel having 10 Mbytes data if I have 32 Mbytes mem on the board ? I don't like the idea of having to change the bootloader just because I want to activate CONFIG_LOCKDEP to debug my kernel. Well, as noted, if you're using a non-ancient U-Boot you shouldn't have to change anything because it already implements r7. Now, the value of r7 it passes might be a lie as far as ePAPR is concerned, since it's supposed to represent what's actually mapped, but that's another matter. Even fixing that wouldn't mean you have to change U-Boot every time the kernel size changes; you'd just set it to something reasonable and be done with it. I'm not fond of adding kconfigs to hack around a problem that has already been addressed in the standard that governs the PPC boot process that U-Boot claims to implement. Well, ok, that makes sense. I'll investigate around that solution. -#ifdef CONFIG_PIN_TLB +#if defined (CONFIG_8xx_INIT_MAP_16M) || defined (CONFIG_8xx_INIT_MAP_24M) /* Map two more 8M kernel data pages. */ +#ifdef CONFIG_PIN_TLB addir10, r10, 0x0100 mtspr SPRN_MD_CTR, r10 +#endif lis r8, KERNELBASE@h /* Create vaddr for TLB */ addis r8, r8, 0x0080 /* Add 8M */ @@ -858,15 +860,19 @@ addis r11, r11, 0x0080/* Add 8M */ mtspr SPRN_MD_RPN, r11 +#ifdef CONFIG_8xx_INIT_MAP_24M +#ifdef CONFIG_PIN_TLB addir10, r10, 0x0100 mtspr SPRN_MD_CTR, r10 +#endif Are these ifdefs for CONFIG_PIN_TLB really needed? It shouldn't harm anything to use those entries even if they're not being pinned. I'm not sure I understand your comment. ifdef for CONFIG_PIN_TLB was already there before, but was enclosing the whole block, so 24 Mbytes were automatically mapped when you selected CONFIG_PIN_TLB and only 8 Mbytes were mapped when you didn't select CONFIG_PIN_TLB. I reduced the scope of those ifdefs so that they now apply on the pinning only. There wasn't previously an ifdef specifically around the setting of SPRN_MD_CTR. That's new. There was an ifdef around the entire block, which has gone away because you are now trying to map more than 8M regardless of CONFIG_PIN_TLB, but that has nothing to do with whether there should be an ifdef around SPRN_MD_CTR. Euh, ok, but then we have to fix it in the whole function, not only in this block. Do you think it is worth doing it ? Then we are back to the problem we discussed some months ago which is that the 8xx is decrementing the MD_CTR after writting a TLB entry, and if pinning is activated it decrements it out of the pinnable area. So it would still be needed to: * Reposition it for each entry for when the pinning is activated * Make sure we set it out of the area at the end when the pinning is not active hence the area not protected. * Then we should probably reverse the entries, start at 31 and go down to 28 instead of going from 28 to 31 as do today. But is it worth doing such a big change
Re: Linux 3.16: all my drivers on SPI bus report WARNING: at drivers/base/dd.c:286
I've been able to identify the origin of the issue. It happens since the below
commit.
Do you know what should be done to fix that ?
Christophe
From 7a40054361162d2f78f332aa868fed137beb7901 Mon Sep 17 00:00:00 2001
From: Axel Lin
Date: Sun, 30 Mar 2014 16:42:57 +0800
Subject: spi: fsl-spi: Fix memory leak
mpc8xxx_spi_probe() has set master->cleanup = mpc8xxx_spi_cleanup,
however current code overrides the setting in fsl_spi_probe() and set
master->cleanup = fsl_spi_cleanup.
Thus the memory allocated for cs is not freed anywhere.
Convert to use devm_kzalloc to fix the memory leak.
Signed-off-by: Axel Lin
Signed-off-by: Mark Brown
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index b3e7775..98ccd23 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -431,7 +431,7 @@ static int fsl_spi_setup(struct spi_device *spi)
return -EINVAL;
if (!cs) {
- cs = kzalloc(sizeof *cs, GFP_KERNEL);
+ cs = devm_kzalloc(&spi->dev, sizeof(*cs), GFP_KERNEL);
if (!cs)
return -ENOMEM;
spi->controller_state = cs;
--
cgit v0.10.1
Le 19/08/2014 11:21, leroy christophe a écrit :
Since Linux 3.16, for all drivers tied to SPI bus, I get the following
warning on a PowerPC 8xx.
It doesn't happen with Linux 3.15
What can be the reason / what should I look at ?
[3.086957] device: 'spi32766.1': device_add
[3.087179] bus: 'spi': add device spi32766.1
[3.087653] bus: 'spi': driver_probe_device: matched device
spi32766.1 with driver lm70
[3.087743] bus: 'spi': really_probe: probing driver lm70 with
device spi32766.1
[3.088014] [ cut here ]
[3.092348] WARNING: at drivers/base/dd.c:286
[3.096637] Modules linked in:
[3.099697] CPU: 0 PID: 25 Comm: kworker/u2:1 Tainted: G W
3.16.1-s3k-drv-999-svn5771_knld-999 #158
[3.109610] Workqueue: deferwq deferred_probe_work_func
[3.114736] task: c787f020 ti: c790c000 task.ti: c790c000
[3.120062] NIP: c01df158 LR: c01df144 CTR:
[3.124983] REGS: c790db30 TRAP: 0700 Tainted: GW
(3.16.1-s3k-drv-999-svn5771_knld-999)
[3.134162] MSR: 00029032 CR: 22002082 XER: 2000
[3.140703]
[3.140703] GPR00: 0001 c790dbe0 c787f020 0044 0054
0308 c056da0e 20737069
[3.140703] GPR08: 33323736 000ebfe0 0308 000ebfdf 22002082
c046c5a0 c046c608
[3.140703] GPR16: c046c614 c046c620 c046c62c c046c638 c046c648
c046c654 c046c68c c046c6c4
[3.140703] GPR24: 0003 c0401aa0 c0596638
c059662c c054e7a8 c7996800
[3.170102] NIP [c01df158] driver_probe_device+0xf8/0x334
[3.175431] LR [c01df144] driver_probe_device+0xe4/0x334
[3.180633] Call Trace:
[3.183093] [c790dbe0] [c01df144] driver_probe_device+0xe4/0x334
(unreliable)
[3.190147] [c790dc10] [c01dd15c] bus_for_each_drv+0x7c/0xc0
[3.195741] [c790dc40] [c01df5fc] device_attach+0xcc/0xf8
[3.201076] [c790dc60] [c01dd6d4] bus_probe_device+0xb4/0xc4
[3.20] [c790dc80] [c01db9f8] device_add+0x270/0x564
[3.211923] [c790dcc0] [c0219e84] spi_add_device+0xc0/0x190
[3.217427] [c790dce0] [c021a79c] spi_register_master+0x720/0x834
[3.223455] [c790dd40] [c021cb48] of_fsl_spi_probe+0x55c/0x614
[3.229234] [c790dda0] [c01e0d2c] platform_drv_probe+0x30/0x74
[3.234987] [c790ddb0] [c01df18c] driver_probe_device+0x12c/0x334
[3.241008] [c790dde0] [c01dd15c] bus_for_each_drv+0x7c/0xc0
[3.246602] [c790de10] [c01df5fc] device_attach+0xcc/0xf8
[3.251937] [c790de30] [c01dd6d4] bus_probe_device+0xb4/0xc4
[3.257536] [c790de50] [c01de9d8] deferred_probe_work_func+0x98/0xe0
[3.263816] [c790de70] [c00305b8] process_one_work+0x18c/0x440
[3.269577] [c790dea0] [c0030a00] worker_thread+0x194/0x67c
[3.275105] [c790def0] [c0039198] kthread+0xd0/0xe4
[3.279911] [c790df40] [c000c6d0] ret_from_kernel_thread+0x5c/0x64
[3.285970] Instruction dump:
[3.288900] 80de 419e01d0 3b7b0038 3c60c046 7f65db78 38635264
48211b99 813f00a0
[3.296559] 381f00a0 7d290278 3169 7c0b4910 <0f00> 93df0044
7fe3fb78 4bfffd4d
[3.304401] ---[ end trace c75d3461bf9e2961 ]---
[3.309598] device: 'hwmon1': device_add
[3.310246] driver: 'lm70': driver_bound: bound to device 'spi32766.1'
[3.310342] bus: 'spi': really_probe: bound device spi32766.1 to
driver lm70
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Linux 3.16: all my drivers on SPI bus report WARNING: at drivers/base/dd.c:286
Since Linux 3.16, for all drivers tied to SPI bus, I get the following warning on a PowerPC 8xx. It doesn't happen with Linux 3.15 What can be the reason / what should I look at ? [3.086957] device: 'spi32766.1': device_add [3.087179] bus: 'spi': add device spi32766.1 [3.087653] bus: 'spi': driver_probe_device: matched device spi32766.1 with driver lm70 [3.087743] bus: 'spi': really_probe: probing driver lm70 with device spi32766.1 [3.088014] [ cut here ] [3.092348] WARNING: at drivers/base/dd.c:286 [3.096637] Modules linked in: [3.099697] CPU: 0 PID: 25 Comm: kworker/u2:1 Tainted: G W 3.16.1-s3k-drv-999-svn5771_knld-999 #158 [3.109610] Workqueue: deferwq deferred_probe_work_func [3.114736] task: c787f020 ti: c790c000 task.ti: c790c000 [3.120062] NIP: c01df158 LR: c01df144 CTR: [3.124983] REGS: c790db30 TRAP: 0700 Tainted: GW (3.16.1-s3k-drv-999-svn5771_knld-999) [3.134162] MSR: 00029032 CR: 22002082 XER: 2000 [3.140703] [3.140703] GPR00: 0001 c790dbe0 c787f020 0044 0054 0308 c056da0e 20737069 [3.140703] GPR08: 33323736 000ebfe0 0308 000ebfdf 22002082 c046c5a0 c046c608 [3.140703] GPR16: c046c614 c046c620 c046c62c c046c638 c046c648 c046c654 c046c68c c046c6c4 [3.140703] GPR24: 0003 c0401aa0 c0596638 c059662c c054e7a8 c7996800 [3.170102] NIP [c01df158] driver_probe_device+0xf8/0x334 [3.175431] LR [c01df144] driver_probe_device+0xe4/0x334 [3.180633] Call Trace: [3.183093] [c790dbe0] [c01df144] driver_probe_device+0xe4/0x334 (unreliable) [3.190147] [c790dc10] [c01dd15c] bus_for_each_drv+0x7c/0xc0 [3.195741] [c790dc40] [c01df5fc] device_attach+0xcc/0xf8 [3.201076] [c790dc60] [c01dd6d4] bus_probe_device+0xb4/0xc4 [3.20] [c790dc80] [c01db9f8] device_add+0x270/0x564 [3.211923] [c790dcc0] [c0219e84] spi_add_device+0xc0/0x190 [3.217427] [c790dce0] [c021a79c] spi_register_master+0x720/0x834 [3.223455] [c790dd40] [c021cb48] of_fsl_spi_probe+0x55c/0x614 [3.229234] [c790dda0] [c01e0d2c] platform_drv_probe+0x30/0x74 [3.234987] [c790ddb0] [c01df18c] driver_probe_device+0x12c/0x334 [3.241008] [c790dde0] [c01dd15c] bus_for_each_drv+0x7c/0xc0 [3.246602] [c790de10] [c01df5fc] device_attach+0xcc/0xf8 [3.251937] [c790de30] [c01dd6d4] bus_probe_device+0xb4/0xc4 [3.257536] [c790de50] [c01de9d8] deferred_probe_work_func+0x98/0xe0 [3.263816] [c790de70] [c00305b8] process_one_work+0x18c/0x440 [3.269577] [c790dea0] [c0030a00] worker_thread+0x194/0x67c [3.275105] [c790def0] [c0039198] kthread+0xd0/0xe4 [3.279911] [c790df40] [c000c6d0] ret_from_kernel_thread+0x5c/0x64 [3.285970] Instruction dump: [3.288900] 80de 419e01d0 3b7b0038 3c60c046 7f65db78 38635264 48211b99 813f00a0 [3.296559] 381f00a0 7d290278 3169 7c0b4910 <0f00> 93df0044 7fe3fb78 4bfffd4d [3.304401] ---[ end trace c75d3461bf9e2961 ]--- [3.309598] device: 'hwmon1': device_add [3.310246] driver: 'lm70': driver_bound: bound to device 'spi32766.1' [3.310342] bus: 'spi': really_probe: bound device spi32766.1 to driver lm70 Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
3.16/3.16.1: Kernel Oops in nft_do_chain
Calling 'iptables-compat -L', first time nothing is listed on the screen. Second try, it generates following Oops. See below the console dump and the disassembled code around the failing address root@vgoip:~# /usr/local/sbin/iptables-compat -L root@vgoip:~# /usr/local/sbin/iptables-compat -L Chain INPUT (policy ACCEPT) target prot opt source destination Chain FORWARD (policy ACCEPT) target prot opt source destination Chain OUTPUT (policy ACCEPT) target prot opt source destination root@vgoip:~# [ 191.400860] Unable to handle kernel paging request for data at address 0x0008 [ 191.408022] Faulting instruction address: 0xc02f9924 [ 191.413126] Oops: Kernel access of bad area, sig: 11 [#1] [ 191.418245] PREEMPT CMPC885 [ 191.421002] Modules linked in: [ 191.424060] CPU: 0 PID: 69 Comm: irq/38-fs_enet- Not tainted 3.16.1 #236 [ 191.433166] task: c793ab50 ti: c7ff2000 task.ti: c79e4000 [ 191.438487] NIP: c02f9924 LR: c0365b54 CTR: c0365ae8 [ 191.443407] REGS: c7ff3b70 TRAP: 0300 Not tainted (3.16.1) [ 191.451554] MSR: 9032 CR: 28002082 XER: [ 191.458102] DAR: 0008 DSISR: c000 GPR00: 0300 c7ff3c20 c793ab50 c7ff3d98 c7a9d540 c791a000 c030db0c GPR08: c7ff3e28 0011 004e 005c 0044 GPR16: c7a9d590 c7ff3c80 fffc 0001 c7ff3c28 c7ff3c74 GPR24: c7a9d590 c7a9d590 c041252c c041242c c7ff3c30 c7ff3d98 [ 191.490095] NIP [c02f9924] nft_do_chain+0x438/0x4f4 [ 191.494890] LR [c0365b54] nft_do_chain_ipv4+0x6c/0x7c [ 191.499833] Call Trace: [ 191.502295] [c7ff3c20] [c02f9970] nft_do_chain+0x484/0x4f4 (unreliable) [ 191.508830] [c7ff3d90] [c0365b54] nft_do_chain_ipv4+0x6c/0x7c [ 191.514514] [c7ff3de0] [c02e134c] nf_iterate+0xe4/0x12c [ 191.519673] [c7ff3e20] [c02e15c8] nf_hook_slow+0xa0/0x1f4 [ 191.525034] [c7ff3e60] [c030dd70] ip_local_deliver+0xa0/0xac [ 191.530613] [c7ff3e70] [c030d4f4] ip_rcv_finish+0x130/0x350 [ 191.536128] [c7ff3e90] [c02b5304] __netif_receive_skb_core+0x4c4/0x600 [ 191.542595] [c7ff3ef0] [c0237824] fs_enet_rx_napi+0x30c/0x448 [ 191.548252] [c7ff3f50] [c02b5c38] net_rx_action+0x140/0x20c [ 191.553771] [c7ff3f90] [c001c918] __do_softirq+0x13c/0x2b4 [ 191.559177] [c7ff3ff0] [c000b660] call_do_softirq+0x24/0x3c [ 191.564696] [c79e5e50] [c0003e04] do_softirq_own_stack+0x3c/0x7c [ 191.570625] [c79e5e70] [c001c7d8] do_softirq+0x58/0x5c [ 191.575705] [c79e5e80] [c001cd34] __local_bh_enable_ip+0xa0/0xc4 [ 191.581649] [c79e5e90] [c00525cc] irq_forced_thread_fn+0x64/0x84 [ 191.587577] [c79e5eb0] [c00521fc] irq_thread+0x130/0x188 [ 191.592848] [c79e5ef0] [c0039190] kthread+0xd0/0xe4 [ 191.597651] [c79e5f40] [c000c6d0] ret_from_kernel_thread+0x5c/0x64 [ 191.603706] Instruction dump: [ 191.606636] 83210014 4bfffc90 813f 80090060 74090001 40820070 54290024 8009000c [ 191.614294] 3200 9009000c 8130fff8 3960 <80690008> 8089000c 80a9 31040001 [ 191.622147] ---[ end trace 86fcabb2513eb932 ]--- [ 191.626687] [ 192.599223] Kernel panic - not syncing: Fatal exception in interrupt [ 192.605305] Rebooting in 180 seconds.. if (unlikely(pkt->skb->nf_trace)) c02f98fc:81 3f 00 00 lwz r9,0(r31) c02f9900:80 09 00 60 lwz r0,96(r9) c02f9904:74 09 00 01 andis. r9,r0,1 c02f9908:40 82 00 70 bne-c02f9978 c02f990c:54 29 00 24 rlwinm r9,r1,0,0,18 c02f9910:80 09 00 0c lwz r0,12(r9) c02f9914:30 00 02 00 addic r0,r0,512 c02f9918:90 09 00 0c stw r0,12(r9) nft_trace_packet(pkt, basechain, -1, NFT_TRACE_POLICY); rcu_read_lock_bh(); stats = this_cpu_ptr(rcu_dereference(nft_base_chain(basechain)->stats)); c02f991c:81 30 ff f8 lwz r9,-8(r16) u64_stats_update_begin(&stats->syncp); stats->pkts++; stats->bytes += pkt->skb->len; c02f9920:39 60 00 00 li r11,0 nft_trace_packet(pkt, basechain, -1, NFT_TRACE_POLICY); rcu_read_lock_bh(); stats = this_cpu_ptr(rcu_dereference(nft_base_chain(basechain)->stats)); u64_stats_update_begin(&stats->syncp); stats->pkts++; ==> c02f9924:80 69 00 08 lwz r3,8(r9) c02f9928:80 89 00 0c lwz r4,12(r9) stats->bytes += pkt->skb->len; c02f992c:80 a9 00 00 lwz r5,0(r9) nft_trace_packet(pkt, basechain, -1, NFT_TRACE_POLICY); rcu_read_lock_bh(); stats = this_cpu_ptr(rcu_dereference(nft_base_chain(basechain)->stats)); u64_stats_update_begin(&stats->syncp); stats->pkts++; c02f9930:31 04 00 01 addic r8,r4,1 c02f9934:7c e3 01 94 addze r7,r3 c02f9938:90 e9 00 08 stw r7,8(r9) c02f993c:91 09 00 0c stw r8,12(r9) stats->bytes += pkt->skb->len; c02f9940:80 c9 00 04 lwz r6,4(r9) c02f9944:81 5f 00 00 lwz r10,0(r31) __local_bh_enable_ip(ip, SOFTIRQ_DISABLE_OFFSET); } Christophe -- To u
Re: 3.16/3.16.1: Kernel Oops in nft_do_chain
Le 02/09/2014 12:41, Pablo Neira Ayuso a écrit : On Tue, Sep 02, 2014 at 12:14:27PM +0200, leroy christophe wrote: Calling 'iptables-compat -L', first time nothing is listed on the screen. Second try, it generates following Oops. I'm going to pass this patch to -stable asap: commit b88825de8545ad252c31543fef13cadf4de7a2bc Author: Pablo Neira Ayuso Date: Tue Aug 5 17:25:59 2014 +0200 netfilter: nf_tables: don't update chain with unset counters Fix possible replacement of the per-cpu chain counters by null pointer when updating an existing chain in the commit path. Reported-by: Matteo Croce Signed-off-by: Pablo Neira Ayuso I think it's the root cause for this problem. Thanks, it does fix the Oops I had. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared
Hello Segei, Florian and David, I have an hardware with two ethernet interfaces, and with the two PHYs inside the same component INTEL LXT973 which has only one interrupt. I also have another hardware with two ethernet interfaces and two independant PHYs. But the two PHYs are wired to the same interrupt. This is working perfectly up to Linux 3.12. But since your commit, introduced in Linux 3.13, my interfaces don't work anymore as the second PHYs can't register IRQ. Reading the commit log, I can't really understand the reason for the change. Is it really worth it, and therefore how shall my case be handled ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
kernel 3.17 - perf build failure
LINK perf libperf.a(skip-callchain-idx.o): In function `arch_skip_callchain_idx': /root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:250: undefined reference to `pr_debug' libperf.a(skip-callchain-idx.o): In function `check_return_addr': /root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:188: undefined reference to `pr_debug' libperf.a(skip-callchain-idx.o): In function `arch_skip_callchain_idx': /root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:250: undefined reference to `pr_debug' libperf.a(skip-callchain-idx.o): In function `check_return_addr': /root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:165: undefined reference to `pr_debug' /root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:171: undefined reference to `pr_debug' libperf.a(skip-callchain-idx.o):/root/gen/trunk/knl/tools/perf/arch/powerpc/util/skip-callchain-idx.c:101: more undefined references to `pr_debug' follow collect2: ld returned 1 exit status make[1]: *** [perf] Error 1 make: *** [install] Error 2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 00/21] powerpc/8xx: Optimise MMU TLB handling and add support of 16k pages
Le 17/09/2014 22:34, Scott Wood a écrit : On Wed, 2014-09-17 at 22:33 +0200, christophe leroy wrote: Le 17/09/2014 18:40, Scott Wood a écrit : On Wed, 2014-09-17 at 18:36 +0200, Christophe Leroy wrote: This patchset: 1) provides several MMU TLB handling optimisation on MPC8xx. 2) adds support of 16k pages on MPC8xx. All changes have been successfully tested on a custom board equipped with MPC885 Signed-off-by: Christophe Leroy Tested-by: Christophe Leroy I've already applied patches 1, 2, 4, 5, 6, 9, and 10 from the previous patchset -- have they changed? -Scott No, only 3, 7, 17 are changed, and 20,21 are new. I didn't notice you already applied some. How should I then proceed now for the remaining ones ? Submit a new set ? No, I'll just skip the ones I've already applied. I think I took into account all Joakim's and your's comments in v4. Since I submitted v4, I think I didn't get any new comment. What's the way forward to get the remaining ones applied ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v4,17/21] powerpc/8xx: set PTE bit 22 off TLBmiss
Le 07/11/2014 04:37, Scott Wood a écrit :
On Fri, Sep 19, 2014 at 10:36:09AM +0200, LEROY Christophe wrote:
No need to re-set this bit at each TLB miss. Let's set it in the PTE.
Signed-off-by: Christophe Leroy
---
Changes in v2:
- None
Changes in v3:
- Removed PPC405 related macro from PPC8xx specific code
- PTE_NONE_MASK doesn't need PAGE_ACCESSED in Linux 2.6
Changes in v4:
- None
arch/powerpc/include/asm/pgtable-ppc32.h | 20
arch/powerpc/include/asm/pte-8xx.h | 7 +--
arch/powerpc/kernel/head_8xx.S | 10 ++
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h
b/arch/powerpc/include/asm/pgtable-ppc32.h
index 47edde8..35a9b44 100644
--- a/arch/powerpc/include/asm/pgtable-ppc32.h
+++ b/arch/powerpc/include/asm/pgtable-ppc32.h
@@ -172,6 +172,25 @@ static inline unsigned long pte_update(pte_t *p,
#ifdef PTE_ATOMIC_UPDATES
unsigned long old, tmp;
+#ifdef CONFIG_PPC_8xx
+ unsigned long tmp2;
+
+ __asm__ __volatile__("\
+1: lwarx %0,0,%4\n\
+ andc%1,%0,%5\n\
+ or %1,%1,%6\n\
+ /* 0x200 == Extended encoding, bit 22 */ \
+ /* Bit 22 has to be 1 if neither _PAGE_USER nor _PAGE_RW are set */ \
+ rlwimi %1,%1,32-2,0x200\n /* get _PAGE_USER */ \
+ rlwinm %3,%1,32-1,0x200\n /* get _PAGE_RW */ \
+ or %1,%3,%1\n\
+ xori%1,%1,0x200\n"
+" stwcx. %1,0,%4\n\
+ bne-1b"
Why do you need this...
diff --git a/arch/powerpc/include/asm/pte-8xx.h
b/arch/powerpc/include/asm/pte-8xx.h
index d44826e..daa4616 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -48,19 +48,22 @@
*/
#define _PAGE_RW 0x0400 /* lsb PP bits, inverted in HW */
#define _PAGE_USER0x0800 /* msb PP bits */
+/* set when neither _PAGE_USER nor _PAGE_RW are set */
+#define _PAGE_KNLRO0x0200
#define _PMD_PRESENT 0x0001
#define _PMD_BAD 0x0ff0
#define _PMD_PAGE_MASK0x000c
#define _PMD_PAGE_8M 0x000c
-#define _PTE_NONE_MASK _PAGE_ACCESSED
+#define _PTE_NONE_MASK _PAGE_KNLRO
/* Until my rework is finished, 8xx still needs atomic PTE updates */
#define PTE_ATOMIC_UPDATES1
/* We need to add _PAGE_SHARED to kernel pages */
-#define _PAGE_KERNEL_RO(_PAGE_SHARED)
+#define _PAGE_KERNEL_RO(_PAGE_SHARED | _PAGE_KNLRO)
+#define _PAGE_KERNEL_ROX (_PAGE_EXEC | _PAGE_KNLRO)
#define _PAGE_KERNEL_RW (_PAGE_DIRTY | _PAGE_RW | _PAGE_HWWRITE)
...if 0x200 is already being set in the PTE here?
If I understand well the way it works, those defines are used for
setting the PTE the first time.
Then, pte_update() is used to modify the pte settings on an existing pte
0x200 must be set when and only when the page is a RO kernel page. If
later on pte_update() is called for instance to set the page to RW, the
0x200 has to be removed. Same, if pte_update() is called to remove
_PAGE_RW (ptep_set_wrprotect() does this), 0x200 must be set back.
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Strange reports of perf events on powerpc 83xx
Hi, Has anybody already used 'perf' tool on powerpc MPC83xx ? I have been succesfully using perf on MPC8xx, but on MPC83xx I get something strange. perf record/report reports addresses on user stack, as if it was mixing up D accesses and I accesses. Any idea of what the problem can be ? # Samples: 8K of event 'cpu-clock' # Event count (approx.): 219600 # # Overhead Command Shared Object Symbol # .. # 2.62% perf_reseau4 libpthread-2.18.so [.] __libc_send 2.56% perf_reseau4 [kernel.kallsyms] [k] __ip_make_skb 1.62% perf_reseau4 [kernel.kallsyms] [k] __ip_append_data.isra.39 1.55% perf_reseau4 [kernel.kallsyms] [k] ip_finish_output 1.33% perf_reseau4 [unknown] [k] 0x7d94 1.33% perf_reseau4 [unknown] [k] 0x7d95 1.28% perf_reseau4 [unknown] [k] 0x7d97 1.26% perf_reseau4 [unknown] [k] 0x7da3 1.24% perf_reseau4 [unknown] [k] 0x7d98 1.22% perf_reseau4 [unknown] [k] 0x7d92 1.22% perf_reseau4 [unknown] [k] 0x7d9b 1.22% perf_reseau4 [unknown] [k] 0x7daa 1.21% perf_reseau4 [unknown] [k] 0x7d96 1.18% perf_reseau4 [unknown] [k] 0x7da7 1.17% perf_reseau4 [unknown] [k] 0x7d8d 1.17% perf_reseau4 [unknown] [k] 0x7d99 1.13% perf_reseau4 [unknown] [k] 0x7d90 1.13% perf_reseau4 [unknown] [k] 0x7da2 1.12% perf_reseau4 [kernel.kallsyms] [k] __local_bh_enable_ip 1.12% perf_reseau4 [unknown] [k] 0x7d9c 1.12% perf_reseau4 [unknown] [k] 0x7d9e 1.10% perf_reseau4 [unknown] [k] 0x7da0 1.08% perf_reseau4 [unknown] [k] 0x7d9f 1.08% perf_reseau4 [unknown] [k] 0x7da6 1.05% perf_reseau4 [unknown] [k] 0x7da8 1.02% perf_reseau4 [unknown] [k] 0x7d9a 1.01% perf_reseau4 [unknown] [k] 0x7db0 1.00% perf_reseau4 [unknown] [k] 0x7d89 1.00% perf_reseau4 [unknown] [k] 0x7d8b 1.00% perf_reseau4 [unknown] [k] 0x7dac Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] powerpc32: optimise csum_partial() loop
Le 07/08/2015 01:25, Segher Boessenkool a écrit : On Thu, Aug 06, 2015 at 05:45:45PM -0500, Scott Wood wrote: If this makes performance non-negligibly worse on other 32-bit chips, and is an important improvement on 8xx, then we can use an ifdef since 8xx already requires its own kernel build. I'd prefer to see a benchmark showing that it actually does make things worse on those chips, though. And I'd like to see a benchmark that shows it *does not* hurt performance on most chips, and does improve things on 8xx, and by how much. But it isn't *me* who has to show that, it is not my patch. Ok, following this discussion I made some additional measurement and it looks like: * There is almost no change on the 885 * There is a non negligeable degradation on the 8323 (19.5 tb ticks instead of 15.3) Thanks for pointing this out, I think my patch is therefore not good. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] powerpc32: optimise csum_partial() loop
Le 17/08/2015 12:56, leroy christophe a écrit : Le 07/08/2015 01:25, Segher Boessenkool a écrit : On Thu, Aug 06, 2015 at 05:45:45PM -0500, Scott Wood wrote: If this makes performance non-negligibly worse on other 32-bit chips, and is an important improvement on 8xx, then we can use an ifdef since 8xx already requires its own kernel build. I'd prefer to see a benchmark showing that it actually does make things worse on those chips, though. And I'd like to see a benchmark that shows it *does not* hurt performance on most chips, and does improve things on 8xx, and by how much. But it isn't *me* who has to show that, it is not my patch. Ok, following this discussion I made some additional measurement and it looks like: * There is almost no change on the 885 * There is a non negligeable degradation on the 8323 (19.5 tb ticks instead of 15.3) Thanks for pointing this out, I think my patch is therefore not good. Oops, I was talking about my other past, the one that was to optimise ip_csum_fast. I still have to measure csum_partial Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] powerpc32: optimise csum_partial() loop
Le 17/08/2015 13:00, leroy christophe a écrit : Le 17/08/2015 12:56, leroy christophe a écrit : Le 07/08/2015 01:25, Segher Boessenkool a écrit : On Thu, Aug 06, 2015 at 05:45:45PM -0500, Scott Wood wrote: If this makes performance non-negligibly worse on other 32-bit chips, and is an important improvement on 8xx, then we can use an ifdef since 8xx already requires its own kernel build. I'd prefer to see a benchmark showing that it actually does make things worse on those chips, though. And I'd like to see a benchmark that shows it *does not* hurt performance on most chips, and does improve things on 8xx, and by how much. But it isn't *me* who has to show that, it is not my patch. Ok, following this discussion I made some additional measurement and it looks like: * There is almost no change on the 885 * There is a non negligeable degradation on the 8323 (19.5 tb ticks instead of 15.3) Thanks for pointing this out, I think my patch is therefore not good. Oops, I was talking about my other past, the one that was to optimise ip_csum_fast. I still have to measure csum_partial Now, I have the results for csum_partial(). The measurement is done with mftbl() before and after calling the function, with IRQ off to get a stable measure. Measurement is done with a transfer of vmlinux file done 3 times via scp toward the target. We get approximatly 5 calls to csum_partial() On MPC885: 1/ Without the patchset, mean time spent in csum_partial() is 167 tb ticks. 2/ With the patchset, mean time is 150 tb ticks On MPC8323: 1/ Without the patchset, mean time is 287 tb ticks 2/ With the patchset, mean time is 256 tb ticks The improvement is approximatly 10% in both cases So, unlike my patch on ip_fast_csum(), this one is worth it. Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/2] spi: fsl-spi: Fix parameter ram offset setup for CPM1
Le 07/10/2014 02:15, Scott Wood a écrit :
On Sat, 2014-10-04 at 14:02 +0200, christophe leroy wrote:
Le 03/10/2014 22:29, Scott Wood a écrit :
On Fri, 2014-10-03 at 18:49 +0200, Christophe Leroy wrote:
On CPM1, the SPI parameter RAM has a default location. In fsl_spi_cpm_get_pram()
there was a confusion between the SPI_BASE register and the base of the SPI
parameter RAM. Fortunatly, it was working properly with MPC866 and MPC885
because they do set SPI_BASE, but on MPC860 and other old MPC8xx that doesn't
set SPI_BASE, pram_ofs was not properly set. This patch fixes this confusion.
Signed-off-by: Christophe Leroy
---
Changes from v1 to v2: none
drivers/spi/spi-fsl-cpm.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..0f3a912 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -262,15 +262,14 @@ static unsigned long fsl_spi_cpm_get_pram(struct
mpc8xxx_spi *mspi)
pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
out_be16(spi_base, pram_ofs);
} else {
- struct spi_pram __iomem *pram = spi_base;
- u16 rpbase = in_be16(&pram->rpbase);
+ u16 rpbase = in_be16(spi_base);
- /* Microcode relocation patch applied? */
+ /* Microcode relocation patch applied | rpbase set by default */
if (rpbase) {
pram_ofs = rpbase;
} else {
- pram_ofs = cpm_muram_alloc(SPI_PRAM_SIZE, 64);
- out_be16(spi_base, pram_ofs);
+ pram_ofs = offsetof(cpm8xx_t, cp_dparam[PROFF_SPI]) -
+ offsetof(cpm8xx_t, cp_dpmem[0]);
}
}
Why is PROFF_SPI not coming from the device tree?
That's where it starts to become tricky.
PROFF_SPI is defined in cpm1.h which is included by the driver already.
Yes, but those values shouldn't be used. It's a leftover from the old
way of hardcoding things and describing the hardware with kconfig rather
than the device tree.
It provides the default offset from the start of the parameter RAM.
Previously I had the following in my device tree, and the last part of
the source above (the one for rpbase == 0) could not work.
spi: spi@a80 {
cell-index = <0>;
compatible = "fsl,spi", "fsl,cpm1-spi";
reg = <0xa80 0x30 0x3d80 0x30>;
First reg area was the area for SPI registers. Second area was the
parameter RAM zone, which was just mapped to get access to the SPI_BASE
pointer (rpbase)
Now I have
compatible = "fsl,spi", "fsl,cpm1-spi-reloc";
reg = <0xa80 0x30 0x3dac 0x2>;
First reg area is the area for SPI registers. Second area is the
SPI_BASE, as for the CPM2.
On recent 8xx (885 and 866 at least) it contains the offset (=0x1D80) of
the parameter RAM. But on old ones (860, ...) it contains 0. Therefore
we have to get the default index in another way.
What I wanted was to keep something similar to what's done with CPM2.
What should it look like if that offset had to be in the device tree ?
If the offset is not relocatable or discoverable, it should stay in the
device tree. If you have an old chip you wouldn't have
fsl,cpm1-spi-reloc and thus you'd still have "0x3d80 0x30" in reg.
This index is from the start of the dual port RAM. It is 0x2000 above
the start of the CPM area.
In the DTS, we have:
soc@ff00 {
compatible = "fsl,mpc885", "fsl,pq1-soc";
#address-cells = <1>;
#size-cells = <1>;
device_type = "soc";
ranges = <0x0 0xff00 0x28000>;
bus-frequency = <0>;
clock-frequency = <0>;
cpm@9c0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,mpc885-cpm", "fsl,cpm1";
ranges;
reg = <0x9c0 0x40>;
brg-frequency = <0>;
interrupts = <0>;// cpm error interrupt
interrupt-parent = <&CPM_PIC>;
muram@2000 {
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x2000 0x2000>;
data@0 {
compatible = "fsl,cpm-muram-data";
reg = <0x0 0x1c00>;
};
};
spi: spi@a80 {
#address-cells = <1>;
#size-cells = <0>;
cell-index = <0>;
compatible = "fsl,spi", "fsl,cpm1-spi";
reg = <0xa80 0x30 0x3d80 0x30>;
interrupts = <5>;
interrupt-parent = <&CPM_PIC>;
mode = "cpu";
The binding allows me to do an of_iomap() on the parameter RAM, hence to
get access to the relocation index which is inside it.
But if the relocation index is 0, I have to calculate it by myself
because th
Re: [PATCH 2/2] spi: fsl-spi: Allow dynamic allocation of CPM1 parameter RAM
Le 07/10/2014 02:19, Scott Wood a écrit :
On Sat, 2014-10-04 at 12:15 +0200, christophe leroy wrote:
Le 03/10/2014 22:24, Scott Wood a écrit :
On Fri, 2014-10-03 at 22:15 +0200, christophe leroy wrote:
Le 03/10/2014 16:44, Mark Brown a écrit :
On Fri, Oct 03, 2014 at 02:56:09PM +0200, Christophe Leroy wrote:
+config CPM1_RELOCSPI
+ bool "Dynamic SPI relocation"
+ default n
+ help
+ On recent MPC8xx (at least MPC866 and MPC885) SPI can be relocated
+ without micropatch. This activates relocation to a dynamically
+ allocated area in the CPM Dual port RAM.
+ When combined with SPI relocation patch (for older MPC8xx) it avoids
+ the "loss" of additional Dual port RAM space just above the patch,
+ which might be needed for example when using the CPM QMC.
Something like this shouldn't be a compile time option. Either it
should be unconditional or it should be triggered in some system
specific manner (from DT, from knowing about other users or similar).
Can't be unconditional as older versions of mpc8xx (eg MPC860) don't
support relocation without a micropatch.
I have therefore submitted a v2 based on a DTS compatible property.
So the device tree change is about whether relocation is supported, not
whether it is required?
Indeed no, my intension is to say that relocation is requested. Do you
mean that it should then not use a compatible ?
The device tree describes hardware. It doesn't tell software how to use
that hardware.
Based on one of your other e-mails, I think what you want to say here is
that the old binding didn't describe the registers needed for
relocation, so the new compatible describes the new binding, rather than
requesting that software do a relocation. Software that sees the new
binding could choose to relocate, or just choose to read the current
offset from the register.
Not exactly.
The old binding does describe the entire default param RAM (0x3d80 size
0x30). The relocation index is within this param RAM at 0x3dac.
So the old binding is enough to allow relocation.
The issue today with the driver (hence my first patch) is that the
driver reads the relocation index but takes a wrong decision if the
index is 0: it assumes that an nul index means that a param RAM shall be
allocated, which is wrong. A nul index means that the component doesn't
support relocation, so the default param RAM shall be used. The function
used for that is supposed to return the index. So when the index is
null, I need to calculate it.
Now, it can't be the SPI driver by itself that decide if he has to
relocate or not. Because it depends whether I need to relocate or not.
There is no point in waisting another area of the dualport RAM if I
don't need to use SCC2 in a mode that overlaps the SPI parameter RAM.
Today on the old MPC8xx, a microcode patch is needed in order to be able
to relocate, and relocated address is directly fixed by the code
handling the patch (sysdev/micropatch.c). The patch loading function is
call very early in the boot process by cpm_reset() which is call by the
xxx_setup_arch().
I have two issues with the way it is done today:
1/ the address which in hard coded is the micropatch loading function()
is within the area for descripters for the QMC, so I would need to use
another address.
2/ for new MPC8xx which don't need microcode patch, I have no way today
to relocate.
I have the same issue with the relocation of SMC1. Today when we
activate SMC1 relocation microcode patch, the loading function has a
hard coded relocation area for SMC1 which is the area dedicated to the
MPC8xx DSP. It means that I need to change it as I want to use the DSP.
Would it be acceptable to define a fixed relocation address in the
Kconfig in which we select microcode patch (arch/powerpc/platforms/8xx),
instead of having it hardcoded in micropatch.c ?
Or maybe it would be possible to select which microcode patch we
want/need via the device tree and which address shall be used for
relocation ? What would you suggest to describe it ?
How about checking for the existing specific-SoC compatibles?
What do you mean ?
Look for "fsl,mpc885-cpm-i2c" etc. Or, if you didn't follow that
pattern (remember, I can't see your device tree!), look for
"fsl,mpc885-cpm" or "fsl,mpc866-cpm" in the parent node. It's moot
though, if the device tree also needs to be modified to describe the
register used to relocate.
-Scott
I'm not sure I understood your question.
My full device tree below
Christophe
/*
* MIA ethernet Device Tree Source
*
* Copyright 2011 CSSI, Inc
*/
/dts-v1/;
/ {
model = "MIAE";
compatible = "fsl,cmpc885", "fsl,mod885";
#address-cells = <1>;
#size-cells = <1>;
aliases {
ethernet0 = ð0;
ethernet1 = ð1;
mdio = &phy;
serial0 = &smc1;
};
cpus {
#address-cells = <1>;
#size-cells = <0>;
PowerPC,885@0 {
dev
Re: [PATCH 0/2] net: fs_enet: Remove non NAPI RX and add NAPI for TX
Le 08/10/2014 22:03, David Miller a écrit : From: Christophe Leroy Date: Tue, 7 Oct 2014 15:04:53 +0200 (CEST) When using a MPC8xx as a router, 'perf' shows a significant time spent in fs_enet_interrupt() and fs_enet_start_xmit(). 'perf annotate' shows that the time spent in fs_enet_start_xmit is indeed spent between spin_unlock_irqrestore() and the following instruction, hence in interrupt handling. This is due to the TX complete interrupt that fires after each transmitted packet. This patchset first remove all non NAPI handling as NAPI has become the only mode for RX, then adds NAPI for handling TX complete. This improves NAT TCP throughput by 21% on MPC885 with FEC. Tested on MPC885 with FEC. [PATCH 1/2] net: fs_enet: Remove non NAPI RX [PATCH 2/2] net: fs_enet: Add NAPI TX Signed-off-by: Christophe Leroy Series applied, thanks. Any particular reason you didn't just put the TX reclaim calls into the existing NAPI handler? Not really. I used the gianfar.c driver as a model. That's what other drivers do, because TX reclaim can make SKBs available for RX packet receive on the local cpu. So generally you have one NAPI context that first does any pending TX reclaim, then polls the RX ring for new packets. Is that a better approach ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] splice: sendfile() at once fails for big files
Le 06/05/2015 16:23, Jens Axboe a écrit :
On 05/05/2015 09:41 PM, Linus Torvalds wrote:
Jens, ping?
The test results should make this a no-brainer, but I hate how random
these flag ops.
Missed the original, apparently. I too am confused how this is a
correctness fix and not just an optimization.
+ if (read_len < len)
+ sd->flags |= SPLICE_F_MORE;
+ else if (!more)
+ sd->flags &= ~SPLICE_F_MORE;
Should that check be for 'more', not '!more'?
@@ -1204,6 +1204,7 @@ ssize_t splice_direct_to_actor(struct file *in,
struct splice_desc *sd,
* Don't block on output, we have to drain the direct pipe.
*/
sd->flags &= ~SPLICE_F_NONBLOCK;
+more = sd->flags & SPLICE_F_MORE;
while (len) {
size_t read_len;
@@ -1216,6 +1217,10 @@ ssize_t splice_direct_to_actor(struct file *in,
struct splice_desc *sd,
read_len = ret;
sd->total_len = read_len;
+if (read_len < len)
+sd->flags |= SPLICE_F_MORE;
+else if (!more)
+sd->flags &= ~SPLICE_F_MORE;
'more' contains whether sendfile() has been called with SPLICE_F_MORE or
not.
Until all bytes are processed, we have to force SPLICE_F_MORE regardless
of how sendfile() was called.
Once all bytes have been read, we have to reset the flags according to
how sendfile() was called, so if 'more' is NOT set, we have to clear
SPLICE_F_MORE from sd->flags (which was unconditionaly set for
processing the first bytes)
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [v2,2/2] powerpc32: add support for csum_add()
Le 05/05/2015 00:10, Segher Boessenkool a écrit : On Fri, May 01, 2015 at 08:00:14PM -0500, Scott Wood wrote: On Tue, 2015-04-28 at 21:01 +0200, christophe leroy wrote: The generated code is most likely different on ppc64. I have no ppc64 compiler For reference: yes you do. Just add -m64. [root@localhost knl]# LANG= ppc-linux-gcc -m64 test.c test.c:1:0: error: -m64 not supported in this configuration Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 1/2] powerpc32: adds handling of _PAGE_RO
Le 18/12/2014 03:14, Scott Wood a écrit :
On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote:
Some powerpc like the 8xx don't have a RW bit in PTE bits but a RO (Read Only)
bit.
This patch implements the handling of a _PAGE_RO flag to be used in place of
_PAGE_RW
Signed-off-by: Christophe Leroy
---
v2 is a complete rework compared to v1
arch/powerpc/include/asm/pgtable-ppc32.h | 11 ++-
arch/powerpc/include/asm/pgtable.h | 10 +++---
arch/powerpc/include/asm/pte-common.h| 27 ++-
arch/powerpc/mm/gup.c| 2 ++
arch/powerpc/mm/mem.c| 2 +-
arch/powerpc/mm/pgtable_32.c | 24
6 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h
b/arch/powerpc/include/asm/pgtable-ppc32.h
index 543bb8e..64ed9e1 100644
--- a/arch/powerpc/include/asm/pgtable-ppc32.h
+++ b/arch/powerpc/include/asm/pgtable-ppc32.h
@@ -125,7 +125,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
#define pte_clear(mm, addr, ptep) \
- do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
+ do { pte_update(ptep, ~_PAGE_HASHPTE, _PAGE_RO); } while (0)
Is this really necessary? It's already clearing the valid bit.
Likewise in several other places that set or check for _PAGE_RO on pages
for which no access is permitted.
You are right, this is not needed. I needed it because I had defined
pte_none() as requiring _PAGE_RO set. But we can keep value 0 as
pte_none. Taken into account in v3
Christophe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
Le 06/03/2015 12:44, Mark Brown a écrit : On Wed, Mar 04, 2015 at 09:00:39AM +0100, leroy christophe wrote: Le 03/03/2015 19:44, Mark Brown a écrit : Why are we using of_iomap() rather than a generic I/O mapping function here? because all drivers for powerpc seems to be using of_iomap(), as on powerpc the HW is described by the bootloader in a OF device tree. Today, of_iomap() is at least used in FSL SPI, FSL UART, SPI mpc52xx, UART mpc52xx, i2c-mpc, i2c-cpm, freescale ethernet drivers, etc Is it not correct ? It's legacy, all that code is really old. Modern code is written in as architecture and firmware neutral a fashion as possible to make things more consistent and maintainable. This patch is only a small bug fix. That driver already contains calls to of_iomap() and other related of_ functions. Is it worth rewriting the driver for just a small bug fix ? Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/17] crypto: talitos - Add support for SEC1
Le 06/03/2015 01:21, Kim Phillips a écrit : On Thu, 5 Mar 2015 17:46:05 +0100 Christophe Leroy wrote: [15/17] crypto: talitos - Implementation of SEC1 ... [16/17] crypto: talitos - SEC1 bugs on 0 data hash [17/17] crypto: talitos - Update DT bindings with SEC1 This patchseries doesn't apply, at least on top of Herbert's cryptodev-2.6 tree, as of today: Applying: crypto: talitos - Implementation of SEC1 error: patch failed: drivers/crypto/talitos.c:655 error: drivers/crypto/talitos.c: patch does not apply It was applying ok on linux-next as of yesterday. I will rebase the serie on cryptodev-2.6 Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/17] crypto: talitos - Add support for SEC1
Le 06/03/2015 01:28, Herbert Xu a écrit : On Thu, Mar 05, 2015 at 06:21:01PM -0600, Kim Phillips wrote: On Thu, 5 Mar 2015 17:46:05 +0100 Christophe Leroy wrote: [15/17] crypto: talitos - Implementation of SEC1 ... [16/17] crypto: talitos - SEC1 bugs on 0 data hash [17/17] crypto: talitos - Update DT bindings with SEC1 This patchseries doesn't apply, at least on top of Herbert's cryptodev-2.6 tree, as of today: Applying: crypto: talitos - Implementation of SEC1 error: patch failed: drivers/crypto/talitos.c:655 error: drivers/crypto/talitos.c: patch does not apply Also the patches are coming in a random order. Please send them one at a time to ensure proper ordering. Thanks, Kim, I have now tried on top of cryptodev-2.6 tree, and for me it works (see below). Do I clone cryptodev-2.6 from the wrong place ? On that clone, the latest commit on talitos.c is commit 5be4d4c94b1f98b839344fda7a8752a4a09d0ef5 "crypto: replace scatterwalk_sg_next with sg_next" [root@localhost ~]# git clone https://www.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git Cloning into 'cryptodev-2.6'... remote: Counting objects: 4043448, done. remote: Compressing objects: 100% (682829/682829), done. Receiving objects: 100% (4043448/4043448), 893.52 MiB | 258.00 KiB/s, done. remote: Total 4043448 (delta 3330215), reused 4043104 (delta 3329977) Resolving deltas: 100% (3330215/3330215), done. Checking connectivity... done. Checking out files: 100% (48971/48971), done. [root@localhost ~]# cd cryptodev-2.6/ [root@localhost cryptodev-2.6]# git branch test [root@localhost cryptodev-2.6]# git checkout test Switched to branch 'test' [root@localhost cryptodev-2.6]# git am /root/gen/trunk/submitted_patches/talitos/0* Applying: crypto: talitos - base address for Execution Units and macro for ISR masks Applying: crypto: talitos - Externalise specific SEC2 reset actions Applying: crypto: talitos - Use zero entry to init descriptors ptrs to zero Applying: crypto: talitos - Refactor the sg in/out chain allocation Applying: crypto: talitos - isolate scatter/gather handling for ahash Applying: crypto: talitos - Add talitos2.c to isolate SEC2 specific functions Applying: crypto: talitos - Split talitos.h into 2 parts Applying: crypto: talitos - Deport SEC2 error handling Applying: crypto: talitos - Move reset/init helpers into talitos2.h Applying: crypto: talitos - Move interrupt related macros in talitos2.h Applying: crypto: talitos - Move hash chain handling into talitos2.h Applying: crypto: talitos - Define compatible in talitos2.h instead of talitos.c Applying: crypto: talitos - move sg_count() helper into talitos.h Applying: crypto: talitos - Add a helper function to clear j_extent field Applying: crypto: talitos - Implementation of SEC1 Applying: crypto: talitos - SEC1 bugs on 0 data hash Applying: crypto: talitos - Update DT bindings with SEC1 [root@localhost cryptodev-2.6]# -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: fsl-spi: use of_iomap() to map parameter ram on CPM1
Le 03/03/2015 19:44, Mark Brown a écrit : On Thu, Feb 26, 2015 at 05:11:42PM +0100, Christophe Leroy wrote: On CPM2, the SPI parameter RAM is dynamically allocated in the dualport RAM whereas in CPM1, it is statically allocated to a default address with capability to relocate it somewhere else via the use of CPM micropatch. The address of the parameter RAM is given by the boot loader and expected to be mapped via of_iomap() Why are we using of_iomap() rather than a generic I/O mapping function here? Euh ... because all drivers for powerpc seems to be using of_iomap(), as on powerpc the HW is described by the bootloader in a OF device tree. Today, of_iomap() is at least used in FSL SPI, FSL UART, SPI mpc52xx, UART mpc52xx, i2c-mpc, i2c-cpm, freescale ethernet drivers, etc Is it not correct ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 2/2] powerpc/8xx: use _PAGE_RO instead of _PAGE_RW
Le 18/12/2014 03:22, Scott Wood a écrit : On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote: On powerpc 8xx, in TLB entries, 0x400 bit is set to 1 for read-only pages and is set to 0 for RW pages. So we should use _PAGE_RO instead of _PAGE_RW Signed-off-by: Christophe Leroy --- v2 is a complete rework compared to v1 arch/powerpc/include/asm/pte-8xx.h | 7 +++ arch/powerpc/kernel/head_8xx.S | 3 --- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h index daa4616..4f5583e 100644 --- a/arch/powerpc/include/asm/pte-8xx.h +++ b/arch/powerpc/include/asm/pte-8xx.h @@ -46,7 +46,7 @@ * require a TLB exception handler change. It is assumed unused bits * are always zero. */ -#define _PAGE_RW 0x0400 /* lsb PP bits, inverted in HW */ +#define _PAGE_RO 0x0400 /* lsb PP bits */ It looks like pte_update() needs to be updated to match. #define _PAGE_USER0x0800 /* msb PP bits */ /* set when neither _PAGE_USER nor _PAGE_RW are set */ Also update this comment. #define _PAGE_KNLRO 0x0200 @@ -62,9 +62,8 @@ #define PTE_ATOMIC_UPDATES1 /* We need to add _PAGE_SHARED to kernel pages */ -#define _PAGE_KERNEL_RO(_PAGE_SHARED | _PAGE_KNLRO) -#define _PAGE_KERNEL_ROX (_PAGE_EXEC | _PAGE_KNLRO) -#define _PAGE_KERNEL_RW(_PAGE_DIRTY | _PAGE_RW | _PAGE_HWWRITE) +#define _PAGE_KERNEL_RO(_PAGE_SHARED | _PAGE_RO | _PAGE_KNLRO) +#define _PAGE_KERNEL_ROX (_PAGE_EXEC | _PAGE_RO | _PAGE_KNLRO) #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_PTE_8xx_H */ Where did _PAGE_KERNEL_RW go? It is already defined like this in pte-common.h: #ifndef _PAGE_KERNEL_RW #define _PAGE_KERNEL_RW(_PAGE_DIRTY | _PAGE_RW | _PAGE_HWWRITE) #endif Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [v2 PATCH 1/2] powerpc32: adds handling of _PAGE_RO
Le 18/12/2014 03:14, Scott Wood a écrit :
On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote:
Some powerpc like the 8xx don't have a RW bit in PTE bits but a RO (Read Only)
bit.
This patch implements the handling of a _PAGE_RO flag to be used in place of
_PAGE_RW
Signed-off-by: Christophe Leroy
---
v2 is a complete rework compared to v1
arch/powerpc/include/asm/pgtable-ppc32.h | 11 ++-
arch/powerpc/include/asm/pgtable.h | 10 +++---
arch/powerpc/include/asm/pte-common.h| 27 ++-
arch/powerpc/mm/gup.c| 2 ++
arch/powerpc/mm/mem.c| 2 +-
arch/powerpc/mm/pgtable_32.c | 24
6 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h
b/arch/powerpc/include/asm/pgtable-ppc32.h
index 543bb8e..64ed9e1 100644
--- a/arch/powerpc/include/asm/pgtable-ppc32.h
+++ b/arch/powerpc/include/asm/pgtable-ppc32.h
@@ -125,7 +125,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
#define pte_clear(mm, addr, ptep) \
- do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
+ do { pte_update(ptep, ~_PAGE_HASHPTE, _PAGE_RO); } while (0)
Is this really necessary? It's already clearing the valid bit.
Likewise in several other places that set or check for _PAGE_RO on pages
for which no access is permitted.
@@ -287,8 +287,9 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct
*mm,
static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
{
unsigned long bits = pte_val(entry) &
- (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
- pte_update(ptep, 0, bits);
+ (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_RO |
+_PAGE_EXEC);
+ pte_update(ptep, _PAGE_RO, bits);
}
You're unconditionally clearing _PAGE_RO, and apparently relying on the
undocumented behavior of pte_update() to clear "clr" before setting
"set".
Instead I'd write this as:
unsigned long set = pte_val(entry) &
(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
unsigned long clr = pte_val(entry) & _PAGE_RO;
Don't you mean ?
unsigned long clr = ~pte_val(entry) & _PAGE_RO;
Because, we want to clear _PAGE_RO when _PAGE_RO is not set in entry.
Christophe
pte_update(ptep, clr, set);
-Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: algif_hash: splice of data > 2**16
Le 18/12/2014 13:15, Stephan Mueller a écrit : Hi Herbert, While testing the vmsplice/splice interface of algif_hash I was made aware of the problem that data blobs larger than 16 pages do not seem to be hashed properly. For testing, a file is mmap()ed and handed to vmsplice / splice. If the file is smaller than 2**16, the interface returns the proper hash. However, when the file is larger, only the first 2**16 bytes seem to be used. When adding printk's to hash_sendpage, I see that this function is invoked exactly 16 times where the first 15 invocations have the MSG_MORE flag set and the last invocation does not have MSG_MORE. Hi Stephan, I have already noticed the same issue and proposed a patch, but I never got any feedback and it has never been merged, allthought I pinged it a few times. See https://lkml.org/lkml/2014/4/18/276 Christophe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 07/11] powerpc/8xx: macro for handling CPU15 errata
Le 20/01/2015 12:09, David Laight a écrit : From Christophe Leroy Having a macro will help keep clear code. It might remove an #if but it doesn't really help. All it means is that anyone reading the code has to hunt for the definition before proceeding. Some comment about what (and why) the extra code is needed might help. The main reason is because of patch 09/11 where we have to duplicate this code. I prefer to just duplicate one line rather than duplicate the whole code (especially because in v1 of the PATCHset, it was duplicated twice): -DO_8xx_CPU15(r11, r10) [...] #ifdef CONFIG_MODULES [...] +DO_8xx_CPU15(r10, r11) [...] +#else +mfsprr10, SPRN_SRR0/* Get effective address of fault */ +DO_8xx_CPU15(r11, r10) Is this approach wrong ? ... + +#ifdef CONFIG_8xx_CPU15 +#define DO_8xx_CPU15(tmp, addr)\ + additmp, addr, PAGE_SIZE; \ + tlbie tmp;\ + additmp, addr, PAGE_SIZE; \ You've even transcribed this incorrectly. Oops Clearly not tested :-) Indeed it's been tested, but tests can only show that the code is not worth than before. This code is there to fix a chip errata which (almost?) never happens. In my production version, I have not activated this errata, and he have never seen the problem on any of the more than 200 boards that have run for at least 4 years. Christophe David + tlbie tmp +#else +#define DO_8xx_CPU15(tmp, addr) +#endif + InstructionTLBMiss: #ifdef CONFIG_8xx_CPU6 mtspr SPRN_DAR, r3 @@ -304,12 +315,7 @@ InstructionTLBMiss: EXCEPTION_PROLOG_0 mtspr SPRN_SPRG_SCRATCH2, r10 mfspr r10, SPRN_SRR0 /* Get effective address of fault */ -#ifdef CONFIG_8xx_CPU15 - addir11, r10, PAGE_SIZE - tlbie r11 - addir11, r10, -PAGE_SIZE - tlbie r11 -#endif + DO_8xx_CPU15(r11, r10) /* If we are faulting a kernel address, we have to use the * kernel page tables. -- 2.1.0 ___ Linuxppc-dev mailing list linuxppc-...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [HELP/RFC] Moving ppc8xx microcode patch from micropatch.c to firmware
Le 30/06/2015 22:38, christophe leroy a écrit :
I'm trying to move the 3 microcode patches included in
arch/powerpc/sysdev/micropatch.c into the firmware directory in order
to use request_firmware() and then be able to add additional
micropatch that I need to relocate SMC2 on my MPC885.
I've now been able to get it compiled in, was due to Makefile item
written fw_shipped- instead of fw-shipped-
I'm now facing an Oops for NULL pointer in kmem_cache_alloc() after a
call to kzalloc(sizeof(*firmware), GFP_KERNEL) in
_request_firmware_prepare() (drivers/base/firmware_class.c)
Is that due to cpm_reset() being called too early ? Shouldn't kzalloc()
allocate memory from the bootmem pool in that case ?
[0.00] Unable to handle kernel paging request for data at
address 0x
[0.00] Faulting instruction address: 0xc00a77cc
[0.00] Oops: Kernel access of bad area, sig: 11 [#1]
[0.00] PREEMPT CMPC885
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted
3.18.17-local-knld-998-g9c4c6b6-svn-dirty #1022
[0.00] task: c04dc3d0 ti: c04fc000 task.ti: c04fc000
[0.00] NIP: c00a77cc LR: c01bc3b8 CTR:
[0.00] REGS: c04fde20 TRAP: 0300 Not tainted
(3.18.17-local-knld-998-g9c4c6b6-svn-dirty)
[0.00] MSR: 1032 CR: 93d55d35 XER: a000fb40
[0.00] DAR: DSISR: c000
GPR00: c01bc3b8 c04fded0 c04dc3d0 80d0 0009
01ff
GPR08: 0022 c051 0158 53d55d39
07ff94e8
GPR16: 07bb5d70 07ff81f4 c0534468 c04fdf68
0009
GPR24: 07ffb3a0 0001 80d0 07ffb3a0 c04fc000
c0410878
[0.00] NIP [c00a77cc] kmem_cache_alloc+0x28/0x120
[0.00] LR [c01bc3b8] _request_firmware+0x58/0xa14
[0.00] Call Trace:
[0.00] [c04fded0] [c045f588] ___alloc_bootmem_nopanic+0x34/0x64
(unreliable)
[0.00] [c04fdef0] [c01bc3b8] _request_firmware+0x58/0xa14
[0.00] [c04fdf60] [c0458678] cpm_load_patch+0x34/0xac
[0.00] [c04fdf80] [c0458620] cpm_reset+0x5c/0x80
[0.00] [c04fdf90] [c0458c1c] cmpc885_setup_arch+0x10/0x30
[0.00] [c04fdfa0] [c0457cbc] setup_arch+0x130/0x168
[0.00] [c04fdfb0] [c045469c] start_kernel+0x84/0x37c
[0.00] [c04fdff0] [c0002214] start_here+0x38/0x98
[0.00] Instruction dump:
[0.00] 7c832378 4e800020 7c0802a6 9421ffe0 bf61000c 90010024
7c7d1b78 7c9b2378
[0.00] 543e0024 813e000c 39290001 913e000c <83fd> 839f0004
813e000c 3929
[0.00] ---[ end trace dc8fa200cb88537f ]---
I have written the below patch in order to test the principle, but the
firmware never gets included in my kernel, allthough I have set the
below CONFIG items:
# CONFIG_NO_UCODE_PATCH is not set
CONFIG_USB_SOF_UCODE_PATCH=y
# CONFIG_I2C_SPI_UCODE_PATCH is not set
# CONFIG_I2C_SPI_SMC1_UCODE_PATCH is not set
CONFIG_UCODE_PATCH=y
#
# Generic Driver Options
#
# CONFIG_UEVENT_HELPER is not set
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
# CONFIG_STANDALONE is not set
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
CONFIG_FW_LOADER=y
CONFIG_FIRMWARE_IN_KERNEL=y
CONFIG_EXTRA_FIRMWARE=""
CONFIG_FW_LOADER_USER_HELPER=y
CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
Can anybody help in finding what's wrong there ?
Christophe
---
arch/powerpc/sysdev/micropatch.c | 655
++-
firmware/Makefile| 3 +
firmware/freescale/i2c_spi.bin.ihex | 257
firmware/freescale/i2c_spi_smc1.bin.ihex | 257
firmware/freescale/usb_sof.bin.ihex | 513
5 files changed, 1065 insertions(+), 620 deletions(-)
create mode 100644 firmware/freescale/i2c_spi.bin.ihex
create mode 100644 firmware/freescale/i2c_spi_smc1.bin.ihex
create mode 100644 firmware/freescale/usb_sof.bin.ihex
diff --git a/arch/powerpc/sysdev/micropatch.c
b/arch/powerpc/sysdev/micropatch.c
index 6727dc5..c24780c 100644
--- a/arch/powerpc/sysdev/micropatch.c
+++ b/arch/powerpc/sysdev/micropatch.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -19,652 +21,69 @@
#include
#include
-/*
- * I2C/SPI relocation patch arrays.
- */
-
-#ifdef CONFIG_I2C_SPI_UCODE_PATCH
-
-static uint patch_2000[] __initdata = {
-0x7FFFEFD9,
-0x3FFD,
-0x7FFB49F7,
-0x7FF9,
-0x5FEFADF7,
-0x5F89ADF7,
-0x5FEFAFF7,
-0x5F89AFF7,
-0x3A9CFBC8,
-0xE7C0EDF0,
-0x77C1E1BB,
-0xF4DC7F1D,
-0xABAD932F,
-0x4E08FDCF,
-0x6E0FAFF8,
-0x7CCF76CF,
-0xFD1FF9CF,
-0xABF88DC6,
-0xAB5679F7,
-0xB0937383,
-0xDFCE79F7,
-0xB091E6BB,
-0xE5BBE74F,
-0xB3FA6F0F,
-0x6FFB76CE,
-0xEE0DF9CF,
-0x2BFBEFEF,
-0xCFEEF9CF,
-0x76CEAD24,
-0x90B2DF9A,
-0x7FDDD0BF,
-0x4BF847FD,
-0x7CCF76CE,
-0xCFEF7E1F,
-0x7F1D7DFD,
-0xF0B6EF71,
-0x7FC17
