Re: [PATCH net-next 1/2] net: phy: marvell-88x2222: check that link is operational
Hi Andrew, On Tue, Apr 13, 2021 at 03:12:05PM +0200, Andrew Lunn wrote: > Is there something like this in the marvell10 driver? No, it doesn't seem to be necessary there - I haven't seen spontaneous link-ups with the 88x3310 there. Even if we did, that would cause other issues beyond a nusience link-up event, as the PHY selects the first media that has link between copper and fiber (and both are present on Macchiatobin platforms.) If the fiber indicates link up, it would prevent a copper connection being established. > Also, do you know when there is an SFP cage? Do we need a standardised > DT property for this? phydev->sfp_bus being non-NULL? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [RFC v4 net-next 1/4] net: phy: add MediaTek PHY driver
On Tue, Apr 13, 2021 at 11:59:20AM +0800, DENG Qingfang wrote: > Within 12 hours, I got some spontaneous link down/ups when EEE is enabled: > > [16334.236233] mt7530 mdio-bus:1f wan: Link is Down > [16334.241340] br-lan: port 3(wan) entered disabled state > [16337.355988] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control > rx/tx > [16337.363468] br-lan: port 3(wan) entered blocking state > [16337.368638] br-lan: port 3(wan) entered forwarding state > > The cable is a 30m Cat.6 and never has such issue when EEE is disabled. > Perhaps WAKEUP_TIME_1000/100 or some PHY registers need to be fine-tuned, > but for now I think it should be disabled by default. Experience with Atheros AR8035 which has a very similar issue would suggest that before resorting to the blunt hammer of disabling SmartEEE, one should definitely experiment with the 1G Tw settings. Using 24us for 1G speeds on AR8035 helps a great deal, whereas the PHY defaults to 17us for 1G and 23us for 100M. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2 0/7] remove different PHY fixups
On Tue, Apr 13, 2021 at 12:00:45PM +0200, Lucas Stach wrote: > I agree with the opinion that those PHY fixups introduce more harm than > good. Essentially they are pushing board specific configuration values > into the PHY, without any checks that the fixup is even running on the > specific board it was targeted at. Yes and no. The problem is, that's an easy statement to make when one doesn't understand what they're all doing. Some are "board specific" in that the normal setup for e.g. iMX6 would be to enable clock output from the AR8035 PHY and feed that into the iMX6 - as far as I'm aware, that's the only working configuration for that SoC and PHY. However, it's also true that this fixup should not be applied unconditionally. Then there's SmartEEE - it has been found that the PHY defaults for this lead to link drops independent of the board and SoC that it is connected to. It seems that the PHY is essentially broken - it powers up with SmartEEE enabled, and when connected to another SmartEEE supporting device, it seems guaranteed that it will result in link drops in its default configuration. Freescale's approach has apparently been to unconditionally disable SmartEEE for all their platforms because of this. With a bit of research however (as has been done by Jon and myself) we've found that increasing the Tw parameter for 1G connections results in a much more stable link. So, just saying that these are bad without actually understanding what they are doing is _also_ bad. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 1/2] net: phy: marvell-88x2222: check that link is operational
On Tue, Apr 13, 2021 at 10:19:30AM +0300, Ivan Bornyakov wrote: > On Tue, Apr 13, 2021 at 01:40:32AM +0200, Andrew Lunn wrote: > > On Mon, Apr 12, 2021 at 03:16:59PM +0300, Ivan Bornyakov wrote: > > > Some SFP modules uses RX_LOS for link indication. In such cases link > > > will be always up, even without cable connected. RX_LOS changes will > > > trigger link_up()/link_down() upstream operations. Thus, check that SFP > > > link is operational before actual read link status. > > > > Sorry, but this is not making much sense to me. > > > > LOS just indicates some sort of light is coming into the device. You > > have no idea what sort of light. The transceiver might be able to > > decode that light and get sync, it might not. It is important that > > mv_read_status() returns the line side status. Has it been able to > > achieve sync? That should be independent of LOS. Or are you saying the > > transceiver is reporting sync, despite no light coming in? > > > > Andrew > > Yes, with some SFP modules transceiver is reporting sync despite no > light coming in. So, the idea is to check that link is somewhat > operational before determing line-side status. Indeed - it should be a logical and operation - there is light present _and_ the PHY recognises the signal. This is what the commit achieves, although (iirc) doesn't cater for the case where there is no SFP cage attached. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next] net: mvpp2: Add parsing support for different IPv4 IHL values
On Tue, Apr 13, 2021 at 11:45:31AM +0300, stef...@marvell.com wrote: > From: Stefan Chulski > > Add parser entries for different IPv4 IHL values. > Each entry will set the L4 header offset according to the IPv4 IHL field. > L3 header offset will set during the parsing of the IPv4 protocol. What is the impact of this commit? Is something broken at the moment, if so what? Does this need to be backported to stable kernels? These are key questions, of which the former two should be covered in every commit message so that the reason for the change can be known. It's no good just describing what is being changed in the commit without also describing why the change is being made. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] phy: nxp-c45: add driver for tja1103
On Fri, Apr 09, 2021 at 09:41:06PM +0300, Radu Pirea (NXP OSS) wrote:
> +#define B100T1_PMAPMD_CTL0x0834
> +#define B100T1_PMAPMD_CONFIG_EN BIT(15)
> +#define B100T1_PMAPMD_MASTER BIT(14)
> +#define MASTER_MODE (B100T1_PMAPMD_CONFIG_EN |
> B100T1_PMAPMD_MASTER)
> +#define SLAVE_MODE (B100T1_PMAPMD_CONFIG_EN)
> +
> +#define DEVICE_CONTROL 0x0040
> +#define DEVICE_CONTROL_RESET BIT(15)
> +#define DEVICE_CONTROL_CONFIG_GLOBAL_EN BIT(14)
> +#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13)
> +#define RESET_POLL_NS(250 * NSEC_PER_MSEC)
> +
> +#define PHY_CONTROL 0x8100
> +#define PHY_CONFIG_ENBIT(14)
> +#define PHY_START_OP BIT(0)
> +
> +#define PHY_CONFIG 0x8108
> +#define PHY_CONFIG_AUTO BIT(0)
> +
> +#define SIGNAL_QUALITY 0x8320
> +#define SQI_VALIDBIT(14)
> +#define SQI_MASK GENMASK(2, 0)
> +#define MAX_SQI SQI_MASK
> +
> +#define CABLE_TEST 0x8330
> +#define CABLE_TEST_ENABLEBIT(15)
> +#define CABLE_TEST_START BIT(14)
> +#define CABLE_TEST_VALID BIT(13)
> +#define CABLE_TEST_OK0x00
> +#define CABLE_TEST_SHORTED 0x01
> +#define CABLE_TEST_OPEN 0x02
> +#define CABLE_TEST_UNKNOWN 0x07
> +
> +#define PORT_CONTROL 0x8040
> +#define PORT_CONTROL_EN BIT(14)
> +
> +#define PORT_INFRA_CONTROL 0xAC00
> +#define PORT_INFRA_CONTROL_ENBIT(14)
> +
> +#define VND1_RXID0xAFCC
> +#define VND1_TXID0xAFCD
> +#define ID_ENABLEBIT(15)
> +
> +#define ABILITIES0xAFC4
> +#define RGMII_ID_ABILITY BIT(15)
> +#define RGMII_ABILITYBIT(14)
> +#define RMII_ABILITY BIT(10)
> +#define REVMII_ABILITY BIT(9)
> +#define MII_ABILITY BIT(8)
> +#define SGMII_ABILITYBIT(0)
> +
> +#define MII_BASIC_CONFIG 0xAFC6
> +#define MII_BASIC_CONFIG_REV BIT(8)
> +#define MII_BASIC_CONFIG_SGMII 0x9
> +#define MII_BASIC_CONFIG_RGMII 0x7
> +#define MII_BASIC_CONFIG_RMII0x5
> +#define MII_BASIC_CONFIG_MII 0x4
> +
> +#define SYMBOL_ERROR_COUNTER 0x8350
> +#define LINK_DROP_COUNTER0x8352
> +#define LINK_LOSSES_AND_FAILURES 0x8353
> +#define R_GOOD_FRAME_CNT 0xA950
> +#define R_BAD_FRAME_CNT 0xA952
> +#define R_RXER_FRAME_CNT 0xA954
> +#define RX_PREAMBLE_COUNT0xAFCE
> +#define TX_PREAMBLE_COUNT0xAFCF
> +#define RX_IPG_LENGTH0xAFD0
> +#define TX_IPG_LENGTH0xAFD1
> +#define COUNTERS_EN BIT(15)
> +
> +#define CLK_25MHZ_PS_PERIOD 4UL
> +#define PS_PER_DEGREE(CLK_25MHZ_PS_PERIOD / 360)
> +#define MIN_ID_PS8222U
> +#define MAX_ID_PS11300U
Maybe include some prefix as to which MMD each of these registers is
located?
> +static bool nxp_c45_can_sleep(struct phy_device *phydev)
> +{
> + int reg;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_STAT1);
> + if (reg < 0)
> + return false;
> +
> + return !!(reg & MDIO_STAT1_LPOWERABLE);
> +}
This looks like it could be useful as a generic helper function -
nothing in this function is specific to this PHY.
> +static int nxp_c45_resume(struct phy_device *phydev)
> +{
> + int reg;
> +
> + if (!nxp_c45_can_sleep(phydev))
> + return -EOPNOTSUPP;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> + reg &= ~MDIO_CTRL1_LPOWER;
> + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> +
> + return 0;
> +}
> +
> +static int nxp_c45_suspend(struct phy_device *phydev)
> +{
> + int reg;
> +
> + if (!nxp_c45_can_sleep(phydev))
> + return -EOPNOTSUPP;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1);
> + reg |= MDIO_CTRL1_LPOWER;
> + phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_CTRL1, reg);
> +
> + return 0;
> +}
These too look like potential generic helper functions.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: Bogus struct page layout on 32-bit
On Sat, Apr 10, 2021 at 03:06:52PM +0100, Matthew Wilcox wrote: > How about moving the flags into the union? A bit messy, but we don't > have to play games with __packed__. Yes, that is probably the better solution, avoiding the games to try and get the union appropriately placed on 32-bit systems. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 0/2] Enable 2.5Gbps speed for stmmac
On Wed, Apr 07, 2021 at 02:44:39PM +0200, Andrew Lunn wrote: > > Intel mgbe is flexible to pair with any PHY. Only Aquantia/Marvell > > multi-gige PHY can do rate adaption right? > > The Marvell/Marvell multi-gige PHY can also do rate > adaptation. Marvell buying Aquantia made naming messy :-( > I should probably use part numbers. > > > Hence, we still need to take care of others PHYs. > > Yes, it just makes working around the broken design harder if you want > to get the most out of the hardware. FYI, we really need to come up with a good solution to the rate adaption issue. What we have today really is not good. For example, take a MAC that supports only 2500base-X connected to a PHY that does rate adaption from 2500base-X to media speed. So, the PHY could be capable of 10, 100, 1G and 2.5G media speeds, and would advertise those in its supported mask. The MAC however would only report (via the validate callback) support for 2.5G speed because that's all that 2500base-X supports. What we really want when a rate adapting capable PHY is connected is to ignore what ethtool link modes the MAC supports beyond "does it support this interface type" and just use the PHY supported mask. However, that's another property of the PHY that we need to know from phylib, and it's not clear when that property should be made available. As we know from Marvell PHYs, it depends on the configurable MAC_TYPE setting, so could only be available once we've selected an interface mode for the PHY. On the other hand, we might need to know what interface mode(s) are available from the PHY and MAC to select an appropriate mode. This is not easy problems to overcome; I have had some patches for some time which allow some combination of MAC and PHY to advertise which interface mode(s) they support but I haven't been entirely happy with them to push them upstream - and it would be another phylink API change which means having to maintain the new and old code until everything has been updated (thereby making stuff a lot more complex.) After the last round of phylink API updates and the hostility from people over that, this is a big demotivating factor. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 00/20] kbuild: unify the install.sh script usage
On Wed, Apr 07, 2021 at 10:07:29AM +0200, Greg Kroah-Hartman wrote: > On Wed, Apr 07, 2021 at 09:02:29AM +0100, Russell King - ARM Linux admin > wrote: > > On Wed, Apr 07, 2021 at 09:46:18AM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Apr 07, 2021 at 09:18:11AM +0200, Geert Uytterhoeven wrote: > > > > Hi Greg, > > > > > > > > Thanks for your series! > > > > > > > > On Wed, Apr 7, 2021 at 7:34 AM Greg Kroah-Hartman > > > > wrote: > > > > > Almost every architecture has copied the "install.sh" script that > > > > > originally came with i386, and modified it in very tiny ways. This > > > > > patch series unifies all of these scripts into one single script to > > > > > allow people to understand how to correctly install a kernel, and > > > > > fixes > > > > > up some issues regarding trying to install a kernel to a path with > > > > > spaces in it. > > > > > > > > > > Note that not all architectures actually seem to have any type of way > > > > > to > > > > > install a kernel, they must rely on external scripts or tools which > > > > > feels odd as everything should be included here in the main > > > > > repository. > > > > > I'll work on trying to figure out the missing architecture issues > > > > > afterward. > > > > > > > > I'll bite ;-) > > > > > > > > Does anyone actually use these scripts (outside of x86)? > > > > Yes, every time I build a kernel. My kernel build system involves > > typing "kbuild " and the kernel gets > > built in ../build/. When the build completes, it gets > > installed into ~/systems/, tar'd up, and copied to the > > destination machines, unpacked, installed as appropriate, and > > the machine rebooted if requested. > > > > The installation step is done via the ~/bin/installkernel script. > > So you don't use install.sh at all except to invoke your local script. It depends where the kernel is being built; it has been used in the past (one will notice that the arm32 version is not a direct copy of the x86 version, and never was - it was modified from day 1.) It's placement and naming of the files in /boot is still used today, which is slightly different from the x86 version. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 00/20] kbuild: unify the install.sh script usage
On Wed, Apr 07, 2021 at 09:46:18AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 07, 2021 at 09:18:11AM +0200, Geert Uytterhoeven wrote:
> > Hi Greg,
> >
> > Thanks for your series!
> >
> > On Wed, Apr 7, 2021 at 7:34 AM Greg Kroah-Hartman
> > wrote:
> > > Almost every architecture has copied the "install.sh" script that
> > > originally came with i386, and modified it in very tiny ways. This
> > > patch series unifies all of these scripts into one single script to
> > > allow people to understand how to correctly install a kernel, and fixes
> > > up some issues regarding trying to install a kernel to a path with
> > > spaces in it.
> > >
> > > Note that not all architectures actually seem to have any type of way to
> > > install a kernel, they must rely on external scripts or tools which
> > > feels odd as everything should be included here in the main repository.
> > > I'll work on trying to figure out the missing architecture issues
> > > afterward.
> >
> > I'll bite ;-)
> >
> > Does anyone actually use these scripts (outside of x86)?
Yes, every time I build a kernel. My kernel build system involves
typing "kbuild " and the kernel gets
built in ../build/. When the build completes, it gets
installed into ~/systems/, tar'd up, and copied to the
destination machines, unpacked, installed as appropriate, and
the machine rebooted if requested.
The installation step is done via the ~/bin/installkernel script.
> > I assume the architectures that have them, only have them because they
> > were copied from x86 while doing the initial ports ("oh, a file I don't
> > have to modify at all.").
> > But installing the kernel can be very platform-specific.
> > Do you need the vmlinux, vmlinux.gz, Image, zImage, uImage, ...?
> > With separate or appended DTB?
My scripts deal with all that.
However, I haven't been able to review the changes that are being
made because I have no visibility of the common "scripts" version.
Provided it offers exactly the same functionality as the arm32
version, I'm happy. If it doesn't, it may cause a regression, and
I will be reporting that.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses
On Mon, Apr 05, 2021 at 08:58:07PM +0200, Danilo Krummrich wrote: > On Mon, Apr 05, 2021 at 03:33:55PM +0200, Andrew Lunn wrote: > However, this was about something else - Russell wrote: > > > > We have established that MDIO drivers need to reject accesses for > > > > reads/writes that they do not support [..] > The MDIO drivers do this by checking the MII_ADDR_C45 flag if it's a C45 bus > request. In case they don't support it they return -EOPNOTSUPP. So basically, > the bus drivers read/write functions (should) encode the capability of doing > C45 transfers. > > I just noted that this is redundant to the bus' capabilities field of > struct mii_bus which also encodes the bus' capabilities of doing C22 and/or > C45 > transfers. > > Now, instead of encoding this information of the bus' capabilities at both > places, I'd propose just checking the mii_bus->capabilities field in the > mdiobus_c45_*() functions. IMHO this would be a little cleaner, than having > two > places where this information is stored. What do you think about that? It would be good to clean that up, but that's a lot of work given the number of drivers - not only in terms of making the changes but also making sure that the changes are as correct as would be sensibly achievable... then there's the problem of causing regressions by doing so. The two ways were introduced at different times to do two different things: the checking in the read/write methods in each driver was the first method, which was being added to newer drivers. Then more recently came the ->capabilities field. So now we have some drivers that: - do no checks and don't fill in ->capabilities either (some of which are likely C22-only.) - check in their read/write methods for access types they don't support (e.g. drivers/net/ethernet/marvell/mvmdio.c) and don't fill in ->capabilities. Note, mvmdio supports both C22-only and C45-only interfaces with no C22-and-C45 interfaces. - do fill in ->capabilities with MDIOBUS_C22_C45 (and hence have no checks in their read/write functions). Sometimes, its best to leave stuff alone... if it ain't broke, don't make regressions. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses
On Fri, Apr 02, 2021 at 03:10:49AM +0200, Danilo Krummrich wrote: > On Thu, Apr 01, 2021 at 09:48:58AM +0100, Russell King - ARM Linux admin > wrote: > > Do you actually have a requirement for this? > > > Yes, the Marvell 88Q2112 1000Base-T1 PHY. But actually, I just recognize that > it > should be possible to just register it with the compatible string > "ethernet-phy-ieee802.3-c22" instead of "ethernet-phy-ieee802.3-c45", this > should result in probing it as c22 PHY and doing indirect accesses through > phy_*_mmd(). Unfortunately, I let my Marvell Extranet access expire last year, so I can't grab the datasheet for the 88Q2112 to see what Marvell claim it supports. > > One could also argue this is a feature, and it allows userspace to > > know whether C45 cycles are supported or not. > > > No, if the userspace requests such a transfer although the MDIO controller > does not support real c45 framing the kernel will call mdiobus_c45_addr() to > join the devaddr and and regaddr in one parameter and pass it to > mdiobus_read() or mdiobus_write(). A bus driver not supporting c45 framing > will not care and just mask/shift the joined value and write it to the > particular register. Obviously, this will result into complete garbage being > read or (even worse) written. We have established that MDIO drivers need to reject accesses for reads/writes that they do not support - this isn't something that they have historically checked for because it is only recent that phylib has really started to support clause 45 PHYs. More modern MDIO drivers check the requested access type and error out - we need the older MDIO drivers to do the same. > > In summary, I think you need to show us that you have a real world > > use case for these changes - in other words, a real world PHY setup > > that doesn't work today. > > > My concrete setup would have been the PHY I mentioned above, but if I'm right > with my assumption that I can just use "ethernet-phy-ieee802.3-c22" for it, > I don't have a concreate setup anymore. > > However, the kernel provides the option to register arbitrary MDIO drivers > with > mdio_driver_register() where a device could support c45 and live on a c22 only > bus. For such devices the fallback to indirect accesses would be useful as > well, > since they can't use the existing indirect access functions, because they're > specific to PHYs. However, currently there aren't such drivers in the kernel. > I just want to mention this for completeness. Right, and in such a scenario, I think using the PHY compatible property that allows you to specify the IDs will do exactly what you need. Yes, is_c45 will be false, which is exactly what you want in this case. The PHY specific C45 driver will still recognise the PHY ID, and bind to the PHY device. It will then issue the MMD accesses, which will go through the indirect registers. So, I don't immediately see any need to change the kernel code for this. However, you say you don't have this setup anymore, so there's no way to test this. I suggest we wait until we do have such a setup where it can be tested and proven to work, rather than changing the kernel code to try adding something that we've no way to test - but at the same time where making those changes has its own risk of causing regressions. Without it being tested, the cost/benefit ratio is weighed to the change being costly. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 1/2] net: stmmac: enable 2.5Gbps link speed
On Thu, Apr 01, 2021 at 11:01:51PM +0800, Michael Sit Wei Hong wrote:
> + /* 2.5G mode only support 2500baseT full duplex only */
> + if (priv->plat->has_gmac4 && priv->plat->speed_2500_en) {
> + phylink_set(mac_supported, 2500baseT_Full);
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> + phylink_set(mask, 1000baseKX_Full);
Why? This seems at odds to the comment above?
What about 2500baseX_Full ?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses
On Thu, Apr 01, 2021 at 03:23:05AM +0200, danilokrummr...@dk-develop.de wrote: > On 2021-03-31 20:35, Russell King - ARM Linux admin wrote: > > On Wed, Mar 31, 2021 at 07:58:33PM +0200, danilokrummr...@dk-develop.de > > wrote: > > > For this cited change the only thing happening is that if > > > get_phy_device() > > > already failed for probing with is_c45==false (C22 devices) it tries > > > to > > > probe with is_c45==true (C45 devices) which then either results into > > > actual > > > C45 frame transfers or indirect accesses by calling mdiobus_c45_*() > > > functions. > > > > Please explain why and how a PHY may not appear to be present using > > C22 frames to read the ID registers, but does appear to be present > > when using C22 frames to the C45 indirect registers - and summarise > > which PHYs have this behaviour. > > > > It seems very odd that any PHY would only implement C45 indirect > > registers in the C22 register space. > > Honestly, I can't list examples of that case (at least none that have an > upstream driver already). This part of my patch, to fall back to c45 bus > probing when c22 probing does not succeed, is also motivated by the fact > that this behaviour was already introduced with this patch: So, if I understand what you've just said, you want to make a change to the kernel to add support for something that you don't need and don't know that there's any hardware that needs it. Is that correct? > commit 0cc8fecf041d3e5285380da62cc6662bdc942d8c > Author: Jeremy Linton > Date: Mon Jun 22 20:35:32 2020 +0530 > > net: phy: Allow mdio buses to auto-probe c45 devices > > The mdiobus_scan logic is currently hardcoded to only > work with c22 devices. This works fairly well in most > cases, but its possible that a c45 device doesn't respond > despite being a standard phy. If the parent hardware > is capable, it makes sense to scan for c22 devices before > falling back to c45. > > As we want this to reflect the capabilities of the STA, > lets add a field to the mii_bus structure to represent > the capability. That way devices can opt into the extended > scanning. Existing users should continue to default to c22 > only scanning as long as they are zero'ing the structure > before use. > > Signed-off-by: Jeremy Linton > Signed-off-by: Calvin Johnson > Signed-off-by: David S. Miller > > In this patch i.a. the following lines were added. > > + case MDIOBUS_C22_C45: > + phydev = get_phy_device(bus, addr, false); > + if (IS_ERR(phydev)) > + phydev = get_phy_device(bus, addr, true); > + break; > > I'm applying the same logic for MDIOBUS_NO_CAP and MDIOBUS_C22, since > with my patch MDIO controllers with those capabilities can handle c45 bus > probing with indirect accesses. If the PHY doesn't respond to C22 accesses but is a C45 PHY, then how can this work (you seem to have essentially said it doesn't above.) > [By the way, I'm unsure if this order for MDIO bus controllers with the > capability MDIOBUS_C22_C45 makes sense, because if we assume that the > majority of c45 PHYs responds well to c22 probing (which I'm convinced of) There are some which don't - Clause 45 allows PHYs not to implement support for Clause 22 accesses. > the PHY would still be registered as is_c45==false, which results in the > fact > that even though the underlying bus is capable of real c45 framing only > indirect accessing would be performed. But this is another topic and > unrelated to the patch.] We make no distinction between IDs found via Clause 22 probing and IDs found via Clause 45 probing; the PHY driver will match either way. We don't know of any cases where the IDs are different between these two. So we still end up with the same driver being matched. Also note that there are MDIO controllers that support clause 22 and clause 45, but which require clause 22 to be probed first. Not everything is MDIO at the bus level anymore - there's PHYs that support I2C, and the I2C format of a clause 45 read is identical to a clause 22 write. > In the case a PHY is registered as a c45 compatible PHY in the device tree, > it is probed in c45 mode and therefore finally mdiobus_c45_read() is > called, > which as by now just expects the underlying MDIO bus controller to be > capable > to do c45 framing and therefore the operation would fail in case it is not. > Hence, in my opinion it is useful to fall back to indirect accesses in such > a > case to be able to support those PHYs. Do you actua
Re: [PATCH 2/2] net: mdio: support c45 peripherals on c22 busses
On Wed, Mar 31, 2021 at 07:58:33PM +0200, danilokrummr...@dk-develop.de wrote: > For this cited change the only thing happening is that if get_phy_device() > already failed for probing with is_c45==false (C22 devices) it tries to > probe with is_c45==true (C45 devices) which then either results into actual > C45 frame transfers or indirect accesses by calling mdiobus_c45_*() functions. Please explain why and how a PHY may not appear to be present using C22 frames to read the ID registers, but does appear to be present when using C22 frames to the C45 indirect registers - and summarise which PHYs have this behaviour. It seems very odd that any PHY would only implement C45 indirect registers in the C22 register space. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
On Wed, Mar 17, 2021 at 09:52:10AM +0800, Kefeng Wang wrote: > mem_init_print_info() is called in mem_init() on each architecture, > and pass NULL argument, so using void argument and move it into mm_init(). > > Acked-by: Dave Hansen > Signed-off-by: Kefeng Wang Acked-by: Russell King # for arm bits -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] arm: 9016/2: Make symbol 'tmp_pmd_table' static
Why do you have 9016/2 in the subject line? That's an identifier from
the patch system which shouldn't be in the subject line.
If you want to refer to something already committed, please do so via
the sha1 git hash and quote the first line of the commit description
within ("...") in the body of your commit description.
Thanks.
On Sat, Mar 27, 2021 at 04:30:18PM +0800, Shixin Liu wrote:
> Symbol 'tmp_pmd_table' is not used outside of kasan_init.c and only used
> when CONFIG_ARM_LPAE enabled. So marks it static and add it into
> CONFIG_ARM_LPAE.
>
> Signed-off-by: Shixin Liu
> ---
> arch/arm/mm/kasan_init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
> index 9c348042a724..3a06d3b51f97 100644
> --- a/arch/arm/mm/kasan_init.c
> +++ b/arch/arm/mm/kasan_init.c
> @@ -27,7 +27,9 @@
>
> static pgd_t tmp_pgd_table[PTRS_PER_PGD] __initdata __aligned(PGD_SIZE);
>
> -pmd_t tmp_pmd_table[PTRS_PER_PMD] __page_aligned_bss;
> +#ifdef CONFIG_ARM_LPAE
> +static pmd_t tmp_pmd_table[PTRS_PER_PMD] __page_aligned_bss;
> +#endif
>
> static __init void *kasan_alloc_block(size_t size)
> {
> --
> 2.25.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: fix smp_processor_id() in preemptible warning in harden_branch_predictor()
On Thu, Mar 25, 2021 at 09:32:35PM +0800, Liu Xiang wrote: > Russell King - ARM Linux admin 于2021年3月25日周四 下午6:06写道： > > > > On Thu, Mar 25, 2021 at 05:50:49PM +0800, Liu Xiang wrote: > > > When CONFIG_HARDEN_BRANCH_PREDICTOR is selected and user aborts occur, > > > there is a warning: > > > > > > BUG: using smp_processor_id() in preemptible [] code: > > > errnotest/577 > > > caller is __do_user_fault.constprop.4+0x24/0x88 > > > CPU: 1 PID: 577 Comm: errnotest Not tainted > > > 4.14.188-rt87-fmsh-4-g58055877a #1 > > > Hardware name: FMSH PSOC Platform > > > [<8010d6d4>] (unwind_backtrace) from [<8010a228>] (show_stack+0x10/0x14) > > > [<8010a228>] (show_stack) from [<80698f44>] (dump_stack+0x7c/0x98) > > > [<80698f44>] (dump_stack) from [<803d17d0>] > > > (check_preemption_disabled+0xc4/0xfc) > > > [<803d17d0>] (check_preemption_disabled) from [<80110eb8>] > > > (__do_user_fault.constprop.4+0x24/0x88) > > > [<80110eb8>] (__do_user_fault.constprop.4) from [<801112e4>] > > > (do_page_fault+0x2dc/0x310) > > > [<801112e4>] (do_page_fault) from [<801012a8>] (do_DataAbort+0x38/0xb8) > > > [<801012a8>] (do_DataAbort) from [<8010b03c>] (__dabt_usr+0x3c/0x40) > > > Exception stack(0xb21d1fb0 to 0xb21d1ff8) > > > 1fa0: fff4 0054 > > > fff4 > > > 1fc0: 7ed81cc8 7ed81ca0 0007a440 > > > > > > 1fe0: 7ed81ca0 00010493 0001f330 20030010 > > > > This is not the right fix - preemption is supposed to be disabled before > > this function is called. I'm not sure at the present time what the right > > fix is supposed to be because I've forgotten most of the background > > behind why this was placed where it is. > > > > -- > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! > > I have tested with the current mainline kernel, the warning still exists. Yes, it still exists, because it's never been fixed, but the way you are fixing it is not correct. We do not paper over warnings with incorrect fixes. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: syscalls: switch to generic syscalltbl.sh
On Fri, Mar 05, 2021 at 11:53:39PM +0900, Masahiro Yamada wrote: > On Fri, Mar 5, 2021 at 7:04 PM Linus Walleij wrote: > > > > On Mon, Mar 1, 2021 at 3:29 PM Masahiro Yamada wrote: > > > > > Many architectures duplicate similar shell scripts. > > > > > > This commit converts ARM to use scripts/syscalltbl.sh. > > > > > > Signed-off-by: Masahiro Yamada > > > > This looks good to me to FWIW: > > Acked-by: Linus Walleij > > Thanks for the review. > This patch is already in Russell's patch tracker. > > I manually copy/pasted your Ack: > https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9068/1 I see you added a note to 9067/1 about the order of 9067/1 and 9068/1: "Obviously, 9068 was submitted 30 seconds before 9067, but the patch tracking system unfortunately picked them in reverse order, unfortunately." The patch system doesn't "pick" the patches. It merely accepts them in the order that it receives them. I also received them in reverse order into my personal mailbox as well, and also in reverse order from infradead's mailing list. 2021-03-01 14:29:57 1lGjYa-0004Dd-Ok <= masahi...@kernel.org H=conuserg-10.nifty.com [210.131.2.77]:21701 I=[78.32.30.218]:25 P=esmtps X=TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256 S=5966 DKIM=nifty.com id=20210301142903.345278-1-masahi...@kernel.org T="[PATCH] ARM: syscalls: switch to generic syscallhdr.sh" for li...@armlinux.org.uk 2021-03-01 14:30:06 1lGjYj-0004Di-B3 <= masahi...@kernel.org H=conuserg-07.nifty.com [210.131.2.74]:55987 I=[78.32.30.218]:25 P=esmtps X=TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256 S=5811 DKIM=nifty.com id=20210301142834.345062-1-masahi...@kernel.org T="[PATCH] ARM: syscalls: switch to generic syscalltbl.sh" for li...@armlinux.org.uk 2021-03-01 14:32:46 1lGjbK-0004E2-0t <= masahi...@kernel.org H=condef-10.nifty.com [202.248.20.75]:19522 I=[78.32.30.218]:25 P=esmtps X=TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256 S=6136 DKIM=nifty.com id=20210301142903.345278-1-masahi...@kernel.org T="[PATCH] ARM: syscalls: switch to generic syscallhdr.sh" for patc...@arm.linux.org.uk 2021-03-01 14:33:47 1lGjcI-0004EF-Ny <= masahi...@kernel.org H=condef-09.nifty.com [202.248.20.74]:45947 I=[78.32.30.218]:25 P=esmtps X=TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256 S=5981 DKIM=nifty.com id=20210301142834.345062-1-masahi...@kernel.org T="[PATCH] ARM: syscalls: switch to generic syscalltbl.sh" for patc...@arm.linux.org.uk -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: fix smp_processor_id() in preemptible warning in harden_branch_predictor()
On Thu, Mar 25, 2021 at 05:50:49PM +0800, Liu Xiang wrote: > When CONFIG_HARDEN_BRANCH_PREDICTOR is selected and user aborts occur, > there is a warning: > > BUG: using smp_processor_id() in preemptible [] code: errnotest/577 > caller is __do_user_fault.constprop.4+0x24/0x88 > CPU: 1 PID: 577 Comm: errnotest Not tainted > 4.14.188-rt87-fmsh-4-g58055877a #1 > Hardware name: FMSH PSOC Platform > [<8010d6d4>] (unwind_backtrace) from [<8010a228>] (show_stack+0x10/0x14) > [<8010a228>] (show_stack) from [<80698f44>] (dump_stack+0x7c/0x98) > [<80698f44>] (dump_stack) from [<803d17d0>] > (check_preemption_disabled+0xc4/0xfc) > [<803d17d0>] (check_preemption_disabled) from [<80110eb8>] > (__do_user_fault.constprop.4+0x24/0x88) > [<80110eb8>] (__do_user_fault.constprop.4) from [<801112e4>] > (do_page_fault+0x2dc/0x310) > [<801112e4>] (do_page_fault) from [<801012a8>] (do_DataAbort+0x38/0xb8) > [<801012a8>] (do_DataAbort) from [<8010b03c>] (__dabt_usr+0x3c/0x40) > Exception stack(0xb21d1fb0 to 0xb21d1ff8) > 1fa0: fff4 0054 fff4 > 1fc0: 7ed81cc8 7ed81ca0 0007a440 > 1fe0: 7ed81ca0 00010493 0001f330 20030010 This is not the right fix - preemption is supposed to be disabled before this function is called. I'm not sure at the present time what the right fix is supposed to be because I've forgotten most of the background behind why this was placed where it is. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v3 2/2] net: pcs: configure xpcs 2.5G speed mode
On Thu, Mar 25, 2021 at 04:38:06PM +0800, Michael Sit Wei Hong wrote:
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 12a047d47dec..c95dfe4e5310 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -290,6 +290,8 @@ static int phylink_parse_mode(struct phylink *pl, struct
> fwnode_handle *fwnode)
>
> switch (pl->link_config.interface) {
> case PHY_INTERFACE_MODE_SGMII:
> + phylink_set(pl->supported, 2500baseT_Full);
> + fallthrough;
This is wrong. "SGMII" here means 1G SGMII. See the documentation in
Documentation/networking/phy.rst.
If we want to have this at 2.5G speed, then we need a separate
enumeration for that mode, just like we make a distinction between
1000BASE-X and 2500BASE-X.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Mon, Mar 22, 2021 at 06:10:01PM +0100, Cye Borg wrote: > PWS 500au: > > snow / # lspci -vvx -s 7.1 > 00:07.1 IDE interface: Contaq Microsystems 82c693 (prog-if 80 [ISA > Compatibility mode-only controller, supports bus mastering]) > Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr+ Stepping- SERR- FastB2B- DisINTx- > Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium > >TAbort- SERR- Latency: 0 > Interrupt: pin A routed to IRQ 0 > Region 0: I/O ports at 01f0 [size=8] > Region 1: I/O ports at 03f4 > Region 4: I/O ports at 9080 [size=16] > Kernel driver in use: pata_cypress > Kernel modules: pata_cypress > 00: 80 10 93 c6 45 00 80 02 00 80 01 01 00 00 80 00 > 10: f1 01 00 00 f5 03 00 00 00 00 00 00 00 00 00 00 > 20: 81 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 > > snow / # lspci -vvx -s 7.2 > 00:07.2 IDE interface: Contaq Microsystems 82c693 (prog-if 00 [ISA > Compatibility mode-only controller]) > Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr+ Stepping- SERR- FastB2B- DisINTx- > Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium > >TAbort- SERR- Latency: 0 > Interrupt: pin B routed to IRQ 0 > Region 0: I/O ports at 0170 [size=8] > Region 1: I/O ports at 0374 > Region 4: Memory at 0c24 (32-bit, non-prefetchable) > [disabled] [size=64K] > Kernel modules: pata_cypress > 00: 80 10 93 c6 45 00 80 02 00 00 01 01 00 00 80 00 > 10: 71 01 00 00 75 03 00 00 00 00 00 00 00 00 00 00 > 20: 00 00 24 0c 00 00 00 00 00 00 00 00 00 00 00 00 > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 Thanks very much. Could I also ask for the output of: # lspci -vxxx -s 7.0 as well please - this will dump all 256 bytes for the ISA bridge, which contains a bunch of configuration registers. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Mon, Mar 22, 2021 at 04:33:14PM +0100, Christoph Hellwig wrote: > On Mon, Mar 22, 2021 at 04:18:23PM +0100, Christoph Hellwig wrote: > > On Mon, Mar 22, 2021 at 03:15:03PM +, Russell King - ARM Linux admin > > wrote: > > > It gets worse than that though - due to a change to remove > > > pcibios_min_io from the generic code, moving it into the ARM > > > architecture code, this has caused a regression that prevents the > > > legacy resources being registered against the bus resource. So even > > > if they are there, they cause probe failures. I haven't found a > > > reasonable way to solve this yet, but until there is, there is no > > > way that the PATA driver can be used as the "legacy mode" support > > > is effectively done via the PCI code assigning virtual IO port > > > resources. > > > > > > I'm quite surprised that the CY82C693 even works on Alpha - I've > > > asked for a lspci for that last week but nothing has yet been > > > forthcoming from whoever responded to your patch for Alpha - so I > > > can't compare what I'm seeing with what's happening with Alpha. > > > > That sounds like something we could fix with a quirk for function 2 > > in the PCI resource assignment code. Can you show what vendor and > > device ID function 2 has so that I could try to come up with one? > > Something like this: That solves the problem for the IDE driver, which knows how to deal with legacy mode, but not the PATA driver, which doesn't. The PATA driver needs these resources. As I say, having these resources presents a problem on ARM. A previous commit (3c5d1699887b) changed the way the bus resources are setup which results in /proc/ioports containing: -000f : dma1 0020-003f : pic1 0060-006f : i8042 0070-0073 : rtc_cmos 0070-0073 : rtc0 0080-008f : dma low page 00a0-00bf : pic2 00c0-00df : dma2 0213-0213 : ISAPnP 02f8-02ff : serial8250.0 02f8-02ff : serial 03c0-03df : vga+ 03f8-03ff : serial8250.0 03f8-03ff : serial 0480-048f : dma high page 0a79-0a79 : isapnp write 1000- : PCI0 I/O 1000-107f : :00:08.0 1000-107f : 3c59x 1080-108f : :00:06.1 1090-109f : :00:07.0 1090-109f : pata_it821x 10a0-10a7 : :00:07.0 10a0-10a7 : pata_it821x 10a8-10af : :00:07.0 10a8-10af : pata_it821x 10b0-10b3 : :00:07.0 10b0-10b3 : pata_it821x 10b4-10b7 : :00:07.0 10b4-10b7 : pata_it821x The "PCI0 I/O" resource is the bus level resource, and the legacy resources can not be claimed against that. Without these resources, the PATA cypress driver doesn't work. As I said previously, the reason this regression was not picked up earlier is because I don't upgrade the kernel on this machine very often; the machine has had uptimes into thousands of days. I need to try reverting Rob's commit to find out if anything breaks on this platform - it's completely wrong from a technical point of view for any case where we have a PCI southbridge, since the southbridge provides ISA based resources. I'm not entirely sure what the point of it was, since we still have the PCIBIOS_MIN_IO macro which still uses pcibios_min_io. I'm looking at some of the other changes Rob made back at that time which also look wrong, such as 8ef6e6201b26 which has the effect of locating the 21285 IO resources to PCI address 0, over the top of the ISA southbridge resources. I've no idea what Rob was thinking when he removed the csrio allocation code in that commit, but looking at it to day, it's soo obviously wrong even to a casual glance. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: delay: avoid clang -Wtautological-constant warning
On Tue, Mar 23, 2021 at 02:20:23PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > Passing an 8-bit constant into delay() triggers a warning when building > with 'make W=1' using clang: > > drivers/clk/actions/owl-pll.c:182:2: error: result of comparison of constant > 2000 with expression of type 'u8' (aka 'unsigned char') is always false > [-Werror,-Wtautological-constant-out-of-range-compare] > udelay(pll_hw->delay); > ^ > arch/arm/include/asm/delay.h:84:9: note: expanded from macro 'udelay' > ((n) > (MAX_UDELAY_MS * 1000) ? __bad_udelay() : \ >~~~ ^ ~~ > arch/arm/mach-omap2/wd_timer.c:89:3: error: result of comparison of constant > 2000 with expression of type 'u8' (aka 'unsigned char') is always false > [-Werror,-Wtautological-constant-out-of-range-compare] > udelay(oh->class->sysc->srst_udelay); > ^~~~ > > Shut up the warning by adding a cast to a 64-bit number. A cast to 'int' > would usually be sufficient, but would fail to cause a link-time error > for large 64-bit constants. What effect (if any) does this have on code generation when the argument is not constant? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: dma-mapping: fix out of bounds access in CMA
On Tue, Mar 23, 2021 at 02:14:13PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> Dereferencing a zero-length array is always a bug, and we get a warning
> with 'make W=1' here:
>
> arch/arm/mm/dma-mapping.c: In function 'dma_contiguous_early_fixup':
> arch/arm/mm/dma-mapping.c:395:15: error: array subscript is outside
> array bounds of 'struct dma_contig_early_reserve[0]' [-Werror=array-bounds]
> 395 | dma_mmu_remap[dma_mmu_remap_num].base = base;
> | ~^~~
> arch/arm/mm/dma-mapping.c:389:40: note: while referencing 'dma_mmu_remap'
> 389 | static struct dma_contig_early_reserve dma_mmu_remap[MAX_CMA_AREAS]
> __initdata;
> |^
> arch/arm/mm/dma-mapping.c:396:15: error: array subscript is outside
> array bounds of 'struct dma_contig_early_reserve[0]' [-Werror=array-bounds]
>
> Add a runtime check to prevent this from happening, while also
> avoiding the compile-time warning.
>
> Fixes: c79095092834 ("ARM: integrate CMA with DMA-mapping subsystem")
> Signed-off-by: Arnd Bergmann
> ---
> arch/arm/mm/dma-mapping.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c4b8df2ad328..af29344fb150 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -392,6 +392,11 @@ static int dma_mmu_remap_num __initdata;
>
> void __init dma_contiguous_early_fixup(phys_addr_t base, unsigned long size)
> {
> + if (!MAX_CMA_AREAS || dma_mmu_remap_num >= MAX_CMA_AREAS) {
> + WARN_ONCE(1, "number of CMA areas\n");
> + return;
> + }
> +
What if dma_mmu_remap_num were negative - that condition is not checked
and will also result in an overflow of the array. If we're being fussy
enough to bounds check, we ought to do it properly.
So, I think a better solution would be to make dma_mmu_remap_num an
unsigned int, and then to use:
if (dma_mmu_remap_num >= ARRAY_SIZE(dma_mmu_remap)) {
...
}
which is really the condition we're after here.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Mon, Mar 22, 2021 at 05:09:13PM +0100, John Paul Adrian Glaubitz wrote: > On 3/22/21 4:15 PM, Russell King - ARM Linux admin wrote: > > I'm quite surprised that the CY82C693 even works on Alpha - I've > > asked for a lspci for that last week but nothing has yet been > > forthcoming from whoever responded to your patch for Alpha - so I > > can't compare what I'm seeing with what's happening with Alpha. > > Here is lspci on my DEC Alpha XP-1000: > > root@tsunami:~> lspci > :00:07.0 ISA bridge: Contaq Microsystems 82c693 > :00:07.1 IDE interface: Contaq Microsystems 82c693 > :00:07.2 IDE interface: Contaq Microsystems 82c693 > :00:07.3 USB controller: Contaq Microsystems 82c693 > :00:0d.0 VGA compatible controller: Texas Instruments TVP4020 [Permedia > 2] (rev 01) > 0001:01:03.0 Ethernet controller: Digital Equipment Corporation DECchip > 21142/43 (rev 41) > 0001:01:06.0 SCSI storage controller: QLogic Corp. ISP1020 Fast-wide SCSI > (rev 06) > 0001:01:08.0 PCI bridge: Digital Equipment Corporation DECchip 21152 (rev 03) > 0001:02:09.0 Ethernet controller: Intel Corporation 82541PI Gigabit Ethernet > Controller (rev 05) > root@tsunami:~> This is no good. What I asked last Thursday was: "Could you send me the output of lspci -vvx -s 7.1 and lspci -vvx -s 7.2 please?" so I can see the resources the kernel is using and a dump of the PCI config space to see what the hardware is using. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Mon, Mar 22, 2021 at 04:18:23PM +0100, Christoph Hellwig wrote: > On Mon, Mar 22, 2021 at 03:15:03PM +0000, Russell King - ARM Linux admin > wrote: > > It gets worse than that though - due to a change to remove > > pcibios_min_io from the generic code, moving it into the ARM > > architecture code, this has caused a regression that prevents the > > legacy resources being registered against the bus resource. So even > > if they are there, they cause probe failures. I haven't found a > > reasonable way to solve this yet, but until there is, there is no > > way that the PATA driver can be used as the "legacy mode" support > > is effectively done via the PCI code assigning virtual IO port > > resources. > > > > I'm quite surprised that the CY82C693 even works on Alpha - I've > > asked for a lspci for that last week but nothing has yet been > > forthcoming from whoever responded to your patch for Alpha - so I > > can't compare what I'm seeing with what's happening with Alpha. > > That sounds like something we could fix with a quirk for function 2 > in the PCI resource assignment code. Can you show what vendor and > device ID function 2 has so that I could try to come up with one? I already have a quirk in arch/arm/kernel/bios32.c for this - but it is no longer sufficient due to changes in the PCI layer, where much of this is documented. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Mon, Mar 22, 2021 at 03:54:03PM +0100, Christoph Hellwig wrote:
> On Fri, Mar 19, 2021 at 05:53:12PM +0000, Russell King - ARM Linux admin
> wrote:
> > If I extend the arch/arm/kernel/bios32.c code to kill BARs 2/3 (which
> > actually are not present on the CY82C693) then the IDE driver works
> > for me, but the PATA driver does not:
> >
> > cy82c693 :00:06.1: IDE controller (0x1080:0xc693 rev 0x00)
> > cy82c693 :00:06.1: not 100% native mode: will probe irqs later
> > legacy IDE will be removed in 2021, please switch to libata
> > Report any missing HW support to linux-...@vger.kernel.org
> > ide0: BM-DMA at 0x1080-0x1087
> > ide1: BM-DMA at 0x1088-0x108f
> > Probing IDE interface ide0...
> > hda: PIONEER DVD-RW DVR-105, ATAPI CD/DVD-ROM drive
> > hda: host max PIO4 wanted PIO255(auto-tune) selected PIO4
> > ...
> >
> > (unbind Cypress_IDE and try binding pata_cypress)
> >
> > pata_cypress :00:06.1: no available native port
>
> This comes from ata_pci_sff_init_host when it tails to initialize
> a port. There are three cases why it can't initialize the port:
>
> 1) because it is marked as dummy, which is the case for the second
> port of the cypress controller, but you're not using that even
> with the old ide driver, and we'd still not get that message just
> because of that second port.
> 2) when ata_resources_present returns false because the BAR has
> a zero start or length
> 3) because pcim_iomap_regions() fails. This prints a warning to the
> log ("failed to request/iomap BARs for port %d (errno=%d)") that you
> should have seen
>
> So the problem here has to be number two. The legacy ide driver OTOH
> seems to lack a lot of these checks, although I'm not sure how it
> manages to actually work without those.
>
> Can you show how the BAR assignment for the device looks using lscpi
> or a tool of your choice?
There's a big problem here. I have to explicitly zero the resources
(getting rid of the legacy ones assigned by the PCI probe code)
because they are in fact _wrong_ for the CY82C693. The PCI code
assumes that PCI function 1 (primary port) and PCI function 2
(secondary port) are two independent dual-channel IDE ports, and
as the PROG-IF of the class code indicates that all ports are in
legacy mode, the PCI code assigns the legacy ioport resources to
_both_ PCI functions. Essentially, the CY82C693 is a bit of an odd-ball
because it splits the two IDE ports across two functions rather than a
single function.
It gets worse than that though - due to a change to remove
pcibios_min_io from the generic code, moving it into the ARM
architecture code, this has caused a regression that prevents the
legacy resources being registered against the bus resource. So even
if they are there, they cause probe failures. I haven't found a
reasonable way to solve this yet, but until there is, there is no
way that the PATA driver can be used as the "legacy mode" support
is effectively done via the PCI code assigning virtual IO port
resources.
I'm quite surprised that the CY82C693 even works on Alpha - I've
asked for a lspci for that last week but nothing has yet been
forthcoming from whoever responded to your patch for Alpha - so I
can't compare what I'm seeing with what's happening with Alpha.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Fri, Mar 19, 2021 at 05:07:53PM +, Russell King - ARM Linux admin wrote: > On Thu, Mar 18, 2021 at 05:56:58AM +0100, Christoph Hellwig wrote: > > footbridge_defconfig enables CONFIG_IDE but no actual host controller > > driver, so just drop it. > > I have been using the Cypress 82C693 IDE driver on Footbridge for a > CD ROM drive, and I know it doesn't work with the PATA driver - as > I need to disable BM DMA, otherwise the 82C693/DC21285 combination > deadlocks the PCI bus. The PATA driver doesn't support disabling > BM DMA without disabling it for all PATA ports, which is really > annoying for my IT821x card in the same machine. > > So, I'm rather stuck using the PATA driver for the HDDs and the > IDE driver for the CD ROM. > > That said, a commit a while back "cleaning up" the PCI layer appears > to have totally shafted the 82C693, as the kernel tries to request > IO resources at the legacy IDE addresses against the PCI bus resource > which only covers 0x1000-0x. Hence, the 82C693 IDE ports are non- > functional at the moment. > > I'm debating about trying to find a fix to the PCI breakage that was > introduced by "ARM: move PCI i/o resource setup into common code". > > I hadn't noticed it because I don't use the CD ROM drive very often, > and I don't upgrade the kernel that often either on the machine - > but it has been running 24x7 for almost two decades. Okay, a bit more on this... If I extend the arch/arm/kernel/bios32.c code to kill BARs 2/3 (which actually are not present on the CY82C693) then the IDE driver works for me, but the PATA driver does not: cy82c693 :00:06.1: IDE controller (0x1080:0xc693 rev 0x00) cy82c693 :00:06.1: not 100% native mode: will probe irqs later legacy IDE will be removed in 2021, please switch to libata Report any missing HW support to linux-...@vger.kernel.org ide0: BM-DMA at 0x1080-0x1087 ide1: BM-DMA at 0x1088-0x108f Probing IDE interface ide0... hda: PIONEER DVD-RW DVR-105, ATAPI CD/DVD-ROM drive hda: host max PIO4 wanted PIO255(auto-tune) selected PIO4 ... (unbind Cypress_IDE and try binding pata_cypress) pata_cypress :00:06.1: no available native port -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/10] ARM: disable CONFIG_IDE in footbridge_defconfig
On Thu, Mar 18, 2021 at 05:56:58AM +0100, Christoph Hellwig wrote: > footbridge_defconfig enables CONFIG_IDE but no actual host controller > driver, so just drop it. I have been using the Cypress 82C693 IDE driver on Footbridge for a CD ROM drive, and I know it doesn't work with the PATA driver - as I need to disable BM DMA, otherwise the 82C693/DC21285 combination deadlocks the PCI bus. The PATA driver doesn't support disabling BM DMA without disabling it for all PATA ports, which is really annoying for my IT821x card in the same machine. So, I'm rather stuck using the PATA driver for the HDDs and the IDE driver for the CD ROM. That said, a commit a while back "cleaning up" the PCI layer appears to have totally shafted the 82C693, as the kernel tries to request IO resources at the legacy IDE addresses against the PCI bus resource which only covers 0x1000-0x. Hence, the 82C693 IDE ports are non- functional at the moment. I'm debating about trying to find a fix to the PCI breakage that was introduced by "ARM: move PCI i/o resource setup into common code". I hadn't noticed it because I don't use the CD ROM drive very often, and I don't upgrade the kernel that often either on the machine - but it has been running 24x7 for almost two decades. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [syzbot] upstream boot error: WARNING in __context_tracking_enter
On Fri, Mar 19, 2021 at 10:54:48AM +0100, Dmitry Vyukov wrote: > .On Fri, Mar 19, 2021 at 10:44 AM syzbot > wrote: > > syzbot found the following issue on: > > > > HEAD commit:8b12a62a Merge tag 'drm-fixes-2021-03-19' of git://anongit.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=17e815aed0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfeed364fc353c32 > > dashboard link: https://syzkaller.appspot.com/bug?extid=f09a12b2c77bfbbf51bd > > userspace arch: arm > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > Reported-by: syzbot+f09a12b2c77bfbbf5...@syzkaller.appspotmail.com > > > +Mark, arm > It did not get far with CONFIG_CONTEXT_TRACKING_FORCE (kernel doesn't boot). It seems that the path: context_tracking_user_enter() user_enter() context_tracking_enter() __context_tracking_enter() vtime_user_enter() expects preemption to be disabled. It effectively is, because local interrupts are disabled by context_tracking_enter(). However, the requirement for preemption to be disabled is not documented... so shrug. Maybe someone can say what the real requirements are here. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net V2 1/1] net: phy: fix invalid phy id when probe using C22
On Fri, Mar 19, 2021 at 08:40:45AM +0100, Heiner Kallweit wrote: > Is there a specific reason why c22 is probed first? Reversing the order > would solve the issue we speak about here. > c45-probing of c22-only PHY's shouldn't return false positives > (at least at a first glance). That would likely cause problems for the I2f MDIO driver, since a C45 read is indistinguishable from a C22 write on the I2C bus. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net V2 1/1] net: phy: fix invalid phy id when probe using C22
On Thu, Mar 18, 2021 at 09:02:22AM -0700, Florian Fainelli wrote: > On 3/18/2021 6:25 AM, Heiner Kallweit wrote: > > On 18.03.2021 10:09, Wong Vee Khee wrote: > >> When using Clause-22 to probe for PHY devices such as the Marvell > >> 88E2110, PHY ID with value 0 is read from the MII PHYID registers > >> which caused the PHY framework failed to attach the Marvell PHY > >> driver. > >> > >> Fixed this by adding a check of PHY ID equals to all zeroes. > >> > > > > I was wondering whether we have, and may break, use cases where a PHY, > > for whatever reason, reports PHY ID 0, but works with the genphy > > driver. And indeed in swphy_read_reg() we return PHY ID 0, therefore > > the patch may break the fixed phy. > > Having said that I think your patch is ok, but we need a change of > > the PHY ID reported by swphy_read_reg() first. > > At a first glance changing the PHY ID to 0x0001 in swphy_read_reg() > > should be sufficient. This value shouldn't collide with any real world > > PHY ID. > > It most likely would not, but it could be considered an ABI breakage, > unless we filter out what we report to user-space via SIOGCMIIREG and > /sys/class/mdio_bus/*/*/phy_id > > Ideally we would have assigned an unique PHY OUI to the fixed PHY but > that would have required registering Linux as a vendor, and the process > is not entirely clear to me about how to go about doing that. Doesn't that also involve yearly fees? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [syzbot] kernel panic: corrupted stack end in openat
On Tue, Mar 16, 2021 at 04:44:45PM +0100, Arnd Bergmann wrote: > On Tue, Mar 16, 2021 at 11:17 AM Dmitry Vyukov wrote: > > On Tue, Mar 16, 2021 at 11:02 AM Arnd Bergmann wrote: > > > > On Tue, Mar 16, 2021 at 8:18 AM syzbot > > > > > > > [<8073772c>] (integrity_kernel_read) from [<8073a904>] > > > > > (ima_calc_file_hash_tfm+0x178/0x228 > > > > > security/integrity/ima/ima_crypto.c:484) > > > > > [<8073a78c>] (ima_calc_file_hash_tfm) from [<8073ae2c>] > > > > > (ima_calc_file_shash security/integrity/ima/ima_crypto.c:515 [inline]) > > > > > [<8073a78c>] (ima_calc_file_hash_tfm) from [<8073ae2c>] > > > > > (ima_calc_file_hash+0x124/0x8b8 > > > > > security/integrity/ima/ima_crypto.c:572) > > > > > > ima_calc_file_hash_tfm() has a SHASH_DESC_ON_STACK(), which by itself can > > > use up 512 bytes, but KASAN sometimes triples this number. However, I see > > > you do not actually have KASAN enabled, so there is probably more to it. > > > > The compiler is gcc version 10.2.1 20210110 (Debian 10.2.1-6) > > Ok, building with Ubuntu 10.2.1-1ubuntu1 20201207 locally, that's > the closest I have installed, and I think the Debian and Ubuntu versions > are generally quite close in case of gcc since they are maintained by > the same packagers. > > I see ima_calc_field_array_hash_tfm() shows up as one of the larger > stack users, but not alarmingly high: > ../security/integrity/ima/ima_crypto.c: In function > ‘ima_calc_field_array_hash_tfm’: > ../security/integrity/ima/ima_crypto.c:624:1: warning: the frame size > of 664 bytes is larger than 600 bytes [-Wframe-larger-than=] > none of the other functions from the call chain have more than 600 bytes > in this combination of config/compiler/sourcetree. > > In combination, I don't get to more than ~2300 bytes: ... which shouldn't be a problem - that's just over 1/4 of the stack space. Could it be the syzbot's gcc is doing something weird and inflating the stack frames? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [V2 net-next] net: mvpp2: Add reserved port private flag configuration
On Tue, Mar 16, 2021 at 03:28:51PM +, Stefan Chulski wrote: > No XDP doesn't require this. One of the use cases of the port reservation > feature is the Marvell User Space SDK (MUSDK) which its latest code is > publicly available here: > https://github.com/MarvellEmbeddedProcessors/musdk-marvell > You can find example use case for this application here: > http://wiki.macchiatobin.net/tiki-index.php?page=MUSDK+Introduction I really, really hope that someone has thought this through: Packet Processor I/O Interface (PPIO) The MUSDK PPIO driver provides low-level network interface API for User-Space network drivers/applications. The PPIO infrastrcuture maps Marvell's Packet Processor (PPv2) configuration space and I/O descriptors space directly to user-space memory. This allows user-space driver/application to directly process the packet processor I/O rings from user space, without any overhead of a copy operation. I realy, really hope that you are not exposing the I/O descriptors to userspace, allowing userspace to manipulate the physical addresses in those descriptors, and that userspace is not dealing with physical addresses. If userspace has access to the I/O descriptors with physical addresses, or userspace is dealing with physical addresses, then you can say good bye to any kind of security on the platform. Essentially, in such a scenario, the entire system memory becomes accessible to userspace, which includes the kernel. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [syzbot] kernel panic: corrupted stack end in openat
On Tue, Mar 16, 2021 at 08:59:17AM +0100, Dmitry Vyukov wrote: > On Tue, Mar 16, 2021 at 8:18 AM syzbot > wrote: > > > > Hello, > > > > syzbot found the following issue on: > > > > HEAD commit:1e28eed1 Linux 5.12-rc3 > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=167535e6d0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=e0cee1f53de33ca3 > > dashboard link: https://syzkaller.appspot.com/bug?extid=0b06ef9b44d00d600183 > > userspace arch: arm > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > Reported-by: syzbot+0b06ef9b44d00d600...@syzkaller.appspotmail.com > > +arm32 maintainer > I think this is a real stack overflow on arm32, the stack is indeed deep. There's no way to know for sure because there's no indication of the stack pointer in this, so we don't know how much space remains. Therefore we don't know whether this is something in the dumped path, or an interrupt causing it. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next] net: mvpp2: Add reserved port private flag configuration
On Wed, Mar 10, 2021 at 11:42:09AM +0200, stef...@marvell.com wrote: > From: Stefan Chulski > > According to Armada SoC architecture and design, all the PPv2 ports > which are populated on the same communication processor silicon die > (CP11x) share the same Classifier and Parser engines. > > Armada is an embedded platform and therefore there is a need to reserve > some of the PPv2 ports for different use cases. > > For example, a port can be reserved for a CM3 CPU running FreeRTOS for > management purposes or by user-space data plane application. > > During port reservation all common configurations are preserved and > only RXQ, TXQ, and interrupt vectors are disabled. If a port is reserved for use by the CM3, what are the implications for Linux running on the AP? Should Linux have knowledge of the port? What configurations of the port should be permitted? I think describing how a port reserved for use by the CM3 CPU should appear to Linux is particularly important for the commit commentry to cover. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: arm: lockdep complaining about locks allocations in static memory
On Wed, Mar 10, 2021 at 02:54:30PM +0100, Jan Kardell wrote: > Hi, > > During work lift the software and kernel versions on our custom TI am3352 > board I started to see lockdep warnings after enabling CONFIG_PREEMT. > Lockdep seems to think the memory that previously was initmem is static > memory. I'm using linux 5.4, as that is what is used in the next OpenWrt > version. > > [ 92.198989] WARNING: CPU: 0 PID: 2015 at kernel/locking/lockdep.c:1119 > alloc_netdev_mqs+0xb4/0x3b0 > > I guess CONFIG_PREEMT just changes the timing of allocations, and is > otherwise irrelevant. > > This was fixed for s390 in linux 5.2 commit > 7a5da02de8d6eafba99556f8c98e5313edebb449 by adding the function > arch_is_kernel_initmem_freed(). Later a very similar change was made for > powerpc, and a different solution for x86. I now believe that is needed for > arm as well. Though I don't know the inner workings of arm memory management > so I don't know if an identical solution to s390 will do for arm, but my > experiments suggests it works for am335x. The commit message for s390 says > "virt == phys", but that seems not to be the case for my arm system. I don't see any reason this couldn't be added to arm, but it needs someone to create and test a patch - which implies that they need to have a problem that needs to be solved. As you seem to be experiencing the problem, it seems you are well suited to this. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] clk: Fix doc of clk_get_parent
On Sun, Mar 07, 2021 at 02:29:07PM +, Paul Cercueil wrote: > Hi, > > Le dim. 7 mars 2021 à 14:27, Russell King - ARM Linux admin > a écrit : > > On Sun, Mar 07, 2021 at 02:06:26PM +, Paul Cercueil wrote: > > > On error, or when the passed parameter is NULL, the return value is > > > NULL > > > and not a PTR_ERR()-encoded value. > > > > No, the documentation is accurate. The CCF is just an implementation > > of the API, the file you are modifying is the definitive API > > documentation. > > Well, then the code has to be fixed, because right now it returns NULL on > error, since the 2.6 days. And you consider NULL to be an error? A NULL clock isn't defined to be an error by the API. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] clk: Fix doc of clk_get_parent
On Sun, Mar 07, 2021 at 02:06:26PM +, Paul Cercueil wrote: > On error, or when the passed parameter is NULL, the return value is NULL > and not a PTR_ERR()-encoded value. No, the documentation is accurate. The CCF is just an implementation of the API, the file you are modifying is the definitive API documentation. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2] amba: Remove deferred device addition
On Wed, Mar 03, 2021 at 08:08:44PM -0800, Saravana Kannan wrote:
> Marek,
>
> I tested it and saw the device get added before the resources were
> available and the uevent file looked okay. Would you mind testing it
> further?
To put it bluntly, if you have tested this, the testing was not very
effective. Deleting the lines that are removed by the patch so we can
see what the new code looks like below:
> > +int amba_device_add(struct amba_device *dev, struct resource *parent)
> > {
> > + int ret;
> >
> > WARN_ON(dev->irq[0] == (unsigned int)-1);
> > WARN_ON(dev->irq[1] == (unsigned int)-1);
> >
> > ret = request_resource(parent, &dev->res);
> > if (ret)
> > + return ret;
> >
> > + /* If primecell ID isn't hard-coded, figure it out */
> > + if (dev->periphid) {
> > + ret = amba_read_periphid(dev);
So, if the peripheral ID has _already_ been set, we attempt to read the
peripheral ID from the device. Isn't that just wrong?
> > + if (ret && ret != -EPROBE_DEFER)
> > + goto err_release;
> > /*
> > +* AMBA device uevents require reading its pid and cid
> > +* registers. To do this, the device must be on, clocked
> > and
> > +* out of reset. However in some cases those resources
> > might
> > +* not yet be available. If that's the case, we suppress
> > the
> > +* generation of uevents until we can read the pid and cid
> > +* registers. See also amba_match().
> > */
> > + if (ret)
> > + dev_set_uevent_suppress(&dev->dev, true);
> > }
If the peripheral ID has not been set, we don't attempt to read it, and
we generate an add event when the amba device is added with a zero
peripheral ID.
I guess that if() statement should be negated - and with such an error,
I fail to see how this code could have been properly tested.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH drivers/net] #ifdef mdio_bus_phy_suspend() and mdio_bus_phy_suspend()
On Wed, Mar 03, 2021 at 09:53:38AM -0800, Paul E. McKenney wrote: > drivers/net: #ifdef mdio_bus_phy_suspend() and mdio_bus_phy_suspend() > > The following build error is emitted by rcutorture builds of v5.12-rc1: > > drivers/net/phy/phy_device.c:293:12: warning: ‘mdio_bus_phy_resume’ defined > but not used [-Wunused-function] > drivers/net/phy/phy_device.c:273:12: warning: ‘mdio_bus_phy_suspend’ defined > but not used [-Wunused-function] > > The problem is that these functions are only used by SIMPLE_DEV_PM_OPS(), > which creates a dev_pm_ops structure only in CONFIG_PM_SLEEP=y kernels. > Therefore, the mdio_bus_phy_suspend() and mdio_bus_phy_suspend() functions > will be used only in CONFIG_PM_SLEEP=y kernels. This commit therefore > wraps them in #ifdef CONFIG_PM_SLEEP. Arnd submitted a patch that Jakub has applied which fix these warnings in a slightly different way. Please see 20210225145748.404410-1-a...@kernel.org -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v3] net: phy: add Marvell 88X2222 transceiver support
Hi, Mostly great, but just a couple more points. On Wed, Mar 03, 2021 at 01:57:57PM +0300, Ivan Bornyakov wrote: > + adv = 0; > + > + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, > + priv->supported)) > + adv |= ADVERTISE_1000XFULL; > + > + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, > + priv->supported)) > + adv |= ADVERTISE_1000XPAUSE; > + > + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, > + priv->supported)) > + adv |= ADVERTISE_1000XPSE_ASYM; You could use the helper: adv = linkmode_adv_to_mii_adv_x(priv->supported, ETHTOOL_LINK_MODE_1000baseX_Full_BIT); instead here. It's also a bit weird to set the advertisement based off a "supported" mask. > + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT, supported); Does the PHY support backplane links? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] module: remove duplicate include in arch/arm/mach-sa1100/hackkit.c
On Tue, Mar 02, 2021 at 11:17:21PM -0800, menglong8.d...@gmail.com wrote: > From: Zhang Yunkai > > 'linux/tty.h' included in 'arch/arm/mach-sa1100/hackkit.c' is duplicated. > > Signed-off-by: Zhang Yunkai I don't see this change has anything to do with module code, so what is the reason for "module:" in the subject line? What am I missing here? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] module: remove duplicate include in ./arch/arm/include/asm/pgtable.h
On Tue, Mar 02, 2021 at 06:04:22PM -0800, menglong8.d...@gmail.com wrote: > From: Zhang Yunkai > > 'asm-generic/pgtable-nopud.h' included in 'pgtable.h' is duplicated. > > Signed-off-by: Zhang Yunkai I don't see this change has anything to do with module code, so what is the reason for "module:" in the subject line? What am I missing here? I also think that this patch needs a better explanation, since it's not a removal of a redundant include, whereas your other similarly described changes are. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 02/11] pragma once: convert arch/arm/tools/gen-mach-types
On Sun, Feb 28, 2021 at 07:59:16PM +0300, Alexey Dobriyan wrote: > From 72842f89ae91a4d02ea29604f87c373052bd3f64 Mon Sep 17 00:00:00 2001 > From: Alexey Dobriyan > Date: Tue, 9 Feb 2021 14:37:40 +0300 > Subject: [PATCH 02/11] pragma once: convert arch/arm/tools/gen-mach-types > > Generate arch/arm/include/generated/asm/mach-types.h without include > guard. The fundamental question of "why" is missing from this commit message. Are we making this change to all kernel headers? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 1/1] arm: print alloc free paths for address in registers
On Wed, Feb 24, 2021 at 06:07:34PM +0530, Maninder Singh wrote:
> +bool slab_page_object(unsigned long address, void **object, struct
> kmem_cache **cache)
> +{
> + void *addr = (void *)address;
> + struct page *page;
> +
> + if ((addr >= (void *)PAGE_OFFSET) &&
> + (addr < high_memory)) {
> + page = virt_to_head_page(addr);
This check is not sufficient. There can be holes in the page array.
You need to use virt_addr_valid() to validate "addr".
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
On Wed, Feb 24, 2021 at 07:14:54PM +0800, liang wang wrote: > Hi Russell, > > On Wed, 24 Feb 2021 at 18:39, Russell King - ARM Linux admin > wrote: > > > > On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote: > > > Hi,all > > > > > > ftrace function_graph tracer always cause kernel panic on my ARM device > > > with > > > multiple CPUs, I found a solution for the problem on ARM64, refers to > > > the patch above, > > > I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM, > > > > Patches get applied to the ARM tree after they've been submitted to > > the patch system. If they don't get submitted to the patch system, > > then they get buried and forgotten. > > > > -- > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! > > Thanks for replying, > So I refers to the solution on ARM64 and sent my patch on January as below. > > http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/628460.html > > But I haven't got any replies on my patch. How can I get my patch reviewed > and submitted to the patch system? I don't use ftrace, so I'm not the right person to review it - but it seems that lots of patches for ARM are left to me to try and review, despite me not knowing sufficient to know whether it's correct or not. This isn't sustainable. > Should I resend a new patch to this maillist? You can try but I suspect the same thing will happen. The only thing I can think is if you put it in the patch system and I apply it, if someone has a problem with it, they'll shout about it soon enough. Not the best way, but I don't see much other option. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote: > Hi,all > > ftrace function_graph tracer always cause kernel panic on my ARM device with > multiple CPUs, I found a solution for the problem on ARM64, refers to > the patch above, > I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM, Patches get applied to the ARM tree after they've been submitted to the patch system. If they don't get submitted to the patch system, then they get buried and forgotten. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v4] ARM: Implement SLS mitigation
On Fri, Feb 19, 2021 at 03:08:13PM -0800, Jian Cai wrote: > diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening > index 269967c4fc1b..146b75a79d9e 100644 > --- a/security/Kconfig.hardening > +++ b/security/Kconfig.hardening > @@ -121,6 +121,16 @@ choice > > endchoice > > +config HARDEN_SLS_ALL > + bool "enable SLS vulnerability hardening" > + default n Please get rid of this useless "default n" > + depends on $(cc-option,-mharden-sls=all) > + help > + Enables straight-line speculation vulnerability hardening on ARM and > ARM64 > + architectures. It inserts speculation barrier sequences (SB or DSB+ISB > + depending on the target architecture) after RET and BR, and replacing > + BLR with BL+BR sequence. Given that this is in an architecture independent Kconfig file, and it detects support in CC for this feature, why should this help text be written to be specific to a couple of architectures? Will this feature only ever be available on these two architectures? What if someone adds support for another architecture? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v4] arm: OABI compat: fix build when EPOLL is not enabled
On Sat, Feb 20, 2021 at 10:47:48AM -0800, Randy Dunlap wrote: > --- > KernelVersion: v5.11 > I don't know what format is used for KernelVersion. > This patch applies to any Linux kernel v5.x and probably even older. I normally ask for it to be the kernel version (without git) that the patch was generated against, so if there's problems applying it, I can try a bit harder to apply it (such as checking out that version and applying it there, and then merging the result.) It's original purpose was when I was maintaining the 2.4 and 2.5/2.6 kernel trees at the same time - it was critical to know which tree people wanted their patch applied to. Note that the patch system email parser can cope with the tag appearing almost anywhere in the email - either the message header or the body of the email, except intermingling with the patch itself. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2] net: phy: add Marvell 88X2222 transceiver support
On Sat, Feb 20, 2021 at 12:46:23PM +0300, Ivan Bornyakov wrote:
> Add basic support for the Marvell 88X multi-speed ethernet
> transceiver.
>
> This PHY provides data transmission over fiber-optic as well as Twinax
> copper links. The 88X supports 2 ports of 10GBase-R and 1000Base-X
> on the line-side interface. The host-side interface supports 4 ports of
> 10GBase-R, RXAUI, 1000Base-X and 2 ports of XAUI.
>
> This driver, however, supports only XAUI on the host-side and
> 1000Base-X/10GBase-R on the line-side, for now. The SGMII is also
> supported over 1000Base-X. Interrupts are not supported.
>
> Internal registers access compliant with the Clause 45 specification.
>
> Signed-off-by: Ivan Bornyakov
> ---
> drivers/net/phy/Kconfig | 6 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/marvell-88x.c | 510 ++
> include/linux/marvell_phy.h | 1 +
> 4 files changed, 518 insertions(+)
> create mode 100644 drivers/net/phy/marvell-88x.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 698bea312adc..a615b3660b05 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -201,6 +201,12 @@ config MARVELL_10G_PHY
> help
> Support for the Marvell Alaska MV88X3310 and compatible PHYs.
>
> +config MARVELL_88X_PHY
> + tristate "Marvell 88X PHY"
> + help
> + Support for the Marvell 88X Dual-port Multi-speed Ethernet
> + Transceiver.
> +
> config MICREL_PHY
> tristate "Micrel PHYs"
> help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index a13e402074cf..de683e3abe63 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -63,6 +63,7 @@ obj-$(CONFIG_LSI_ET1011C_PHY) += et1011c.o
> obj-$(CONFIG_LXT_PHY)+= lxt.o
> obj-$(CONFIG_MARVELL_10G_PHY)+= marvell10g.o
> obj-$(CONFIG_MARVELL_PHY)+= marvell.o
> +obj-$(CONFIG_MARVELL_88X_PHY)+= marvell-88x.o
> obj-$(CONFIG_MESON_GXL_PHY) += meson-gxl.o
> obj-$(CONFIG_MICREL_KS8995MA)+= spi_ks8995.o
> obj-$(CONFIG_MICREL_PHY) += micrel.o
> diff --git a/drivers/net/phy/marvell-88x.c
> b/drivers/net/phy/marvell-88x.c
> new file mode 100644
> index ..5f1b6185e272
> --- /dev/null
> +++ b/drivers/net/phy/marvell-88x.c
> @@ -0,0 +1,510 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Marvell 88x dual-port multi-speed ethernet transceiver.
> + *
> + * Supports:
> + * XAUI on the host side.
> + * 1000Base-X or 10GBase-R on the line side.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* Port PCS Configuration */
> +#define MV_PCS_CONFIG 0xF002
> +#define MV_PCS_HOST_XAUI0x73
> +#define MV_PCS_LINE_10GBR (0x71 << 8)
> +#define MV_PCS_LINE_1GBX_AN (0x7B << 8)
> +#define MV_PCS_LINE_SGMII_AN(0x7F << 8)
> +
> +/* Port Reset and Power Down */
> +#define MV_PORT_RST 0xF003
> +#define MV_LINE_RST_SW BIT(15)
> +#define MV_HOST_RST_SW BIT(7)
> +#define MV_PORT_RST_SW (MV_LINE_RST_SW | MV_HOST_RST_SW)
> +
> +/* 10GBASE-R PCS Real Time Status Register */
> +#define MV_10GBR_STAT_RT0x8002
> +
> +/* 1000Base-X/SGMII Control Register */
> +#define MV_1GBX_CTRL(0x2000 + MII_BMCR)
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define MV_1GBX_STAT(0x2000 + MII_BMSR)
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define MV_1GBX_ADVERTISE (0x2000 + MII_ADVERTISE)
> +
> +/* 1000Base-X PHY Specific Status Register */
> +#define MV_1GBX_PHY_STAT0xA003
> +#define MV_1GBX_PHY_STAT_AN_RESOLVEDBIT(11)
> +#define MV_1GBX_PHY_STAT_DUPLEX BIT(13)
> +#define MV_1GBX_PHY_STAT_SPEED100 BIT(14)
> +#define MV_1GBX_PHY_STAT_SPEED1000 BIT(15)
> +
> +struct mv_data {
> + phy_interface_t line_interface;
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
> +};
> +
> +/* SFI PMA transmit enable */
> +static int mv_tx_enable(struct phy_device *phydev)
> +{
> + return phy_clear_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> + MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +/* SFI PMA transmit disable */
> +static int mv_tx_disable(struct phy_device *phydev)
> +{
> + return phy_set_bits_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_TXDIS,
> + MDIO_PMD_TXDIS_GLOBAL);
> +}
> +
> +static int mv_soft_reset(struct phy_device *phydev)
> +{
> + int val, ret;
> +
> + ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
> + MV_PORT_RST_SW);
> + if (ret < 0)
> + return ret;
> +
> + return phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND2, MV_PORT_RST,
>
Re: 5.10 LTS Kernel: 2 or 6 years?
On Thu, Feb 18, 2021 at 05:19:54PM +, Jari Ruusu wrote: > In-tree iwlwifi worked half-ok on early 4.9.y stable. If > connection somehow de-autheticated (out of radio range or > whatever) it crashed the kernel spectacularly. Eventually that was > fixed and in-tree iwlwifi worked fine on 4.9.y and 4.14.y stable > kernels. On second half of year 2020 (don't remember exactly when) > iwlwifi started causing erratic behavior when some random process > terminated, as if some exit processing left some resources > un-freed or something weird like that. So you bisected the stable kernel, and reported the problem so the problem commit could be dealt with? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: arch/arm/kernel/sys_oabi-compat.c:257:6: error: implicit declaration of function 'ep_op_has_event'
On Wed, Feb 17, 2021 at 09:56:08PM -0800, Randy Dunlap wrote: > On 2/17/21 9:26 PM, kernel test robot wrote: > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > master > > head: f40ddce88593482919761f74910f42f4b84c004b > > commit: c281634c865202e2776b0250678ff93c771947ff ARM: compat: remove > > KERNEL_DS usage in sys_oabi_epoll_ctl() > > date: 10 months ago > > config: arm-randconfig-m031-20210218 (attached as .config) > > compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 > > reproduce (this is a W=1 build): > > wget > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c281634c865202e2776b0250678ff93c771947ff > > git remote add linus > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > git fetch --no-tags linus master > > git checkout c281634c865202e2776b0250678ff93c771947ff > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > > ARCH=arm > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot > > > > All errors (new ones prefixed by >>): > > > >arch/arm/kernel/sys_oabi-compat.c:142:17: warning: no previous prototype > > for 'sys_oabi_stat64' [-Wmissing-prototypes] > > 142 | asmlinkage long sys_oabi_stat64(const char __user * filename, > > | ^~~ > >arch/arm/kernel/sys_oabi-compat.c:152:17: warning: no previous prototype > > for 'sys_oabi_lstat64' [-Wmissing-prototypes] > > 152 | asmlinkage long sys_oabi_lstat64(const char __user * filename, > > | ^~~~ > >arch/arm/kernel/sys_oabi-compat.c:162:17: warning: no previous prototype > > for 'sys_oabi_fstat64' [-Wmissing-prototypes] > > 162 | asmlinkage long sys_oabi_fstat64(unsigned long fd, > > | ^~~~ > >arch/arm/kernel/sys_oabi-compat.c:172:17: warning: no previous prototype > > for 'sys_oabi_fstatat64' [-Wmissing-prototypes] > > 172 | asmlinkage long sys_oabi_fstatat64(int dfd, > > | ^~ > >arch/arm/kernel/sys_oabi-compat.c:229:17: warning: no previous prototype > > for 'sys_oabi_fcntl64' [-Wmissing-prototypes] > > 229 | asmlinkage long sys_oabi_fcntl64(unsigned int fd, unsigned int > > cmd, > > | ^~~~ > >arch/arm/kernel/sys_oabi-compat.c:251:17: warning: no previous prototype > > for 'sys_oabi_epoll_ctl' [-Wmissing-prototypes] > > 251 | asmlinkage long sys_oabi_epoll_ctl(int epfd, int op, int fd, > > | ^~ > >In file included from include/linux/kernel.h:11, > > from include/linux/list.h:9, > > from include/linux/wait.h:7, > > from include/linux/wait_bit.h:8, > > from include/linux/fs.h:6, > > from include/uapi/linux/aio_abi.h:31, > > from include/linux/syscalls.h:74, > > from arch/arm/kernel/sys_oabi-compat.c:73: > >arch/arm/kernel/sys_oabi-compat.c: In function 'sys_oabi_epoll_ctl': > >>> arch/arm/kernel/sys_oabi-compat.c:257:6: error: implicit declaration of > >>> function 'ep_op_has_event' [-Werror=implicit-function-declaration] > > 257 | if (ep_op_has_event(op) && > > | ^~~ > >include/linux/compiler.h:58:52: note: in definition of macro > > '__trace_if_var' > > 58 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? > > (cond) : __trace_if_value(cond)) > > |^~~~ > >arch/arm/kernel/sys_oabi-compat.c:257:2: note: in expansion of macro 'if' > > 257 | if (ep_op_has_event(op) && > > | ^~ > >>> arch/arm/kernel/sys_oabi-compat.c:264:9: error: implicit declaration of > >>> function 'do_epoll_ctl'; did you mean 'sys_epoll_ctl'? > >>> [-Werror=implicit-function-declaration] > > 264 | return do_epoll_ctl(epfd, op, fd, &kernel, false); > > | ^~~~ > > | sys_epoll_ctl > >arch/arm/kernel/sys_oabi-compat.c: At top level: > >arch/arm/kernel/sys_oabi-compat.c:267:17: warning: no previous prototype > > for 'sys_oabi_epoll_wait' [-Wmissing-prototypes] > > 267 | asmlinkage long sys_oabi_epoll_wait(int epfd, > > | ^~~ > >arch/arm/kernel/sys_oabi-compat.c:309:17: warning: no previous prototype > > for 'sys_oabi_semtimedop' [-Wmissing-prototypes] > > 309 | asmlinkage long sys_oabi_semtimedop(int semid, > > | ^~~ > >arch/arm/kernel/sys_oabi-compat.c:352:17: warn
Re: [net-next] net: mvpp2: fix interrupt mask/unmask skip condition
On Thu, Feb 11, 2021 at 05:13:19PM +0200, stef...@marvell.com wrote:
> From: Stefan Chulski
>
> The condition should be skipped if CPU ID equal to nthreads.
> The patch doesn't fix any actual issue since
> nthreads = min_t(unsigned int, num_present_cpus(), MVPP2_MAX_THREADS).
> On all current Armada platforms, the number of CPU's is
> less than MVPP2_MAX_THREADS.
>
> Fixes: e531f76757eb ("net: mvpp2: handle cases where more CPUs are available
> than s/w threads")
> Reported-by: Russell King
> Signed-off-by: Stefan Chulski
Reviewed-by: Russell King
Thanks.
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index a07cf60..74613d3 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1135,7 +1135,7 @@ static void mvpp2_interrupts_mask(void *arg)
> struct mvpp2_port *port = arg;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (smp_processor_id() >= port->priv->nthreads)
> return;
>
> mvpp2_thread_write(port->priv,
> @@ -1153,7 +1153,7 @@ static void mvpp2_interrupts_unmask(void *arg)
> u32 val;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (smp_processor_id() >= port->priv->nthreads)
> return;
>
> val = MVPP2_CAUSE_MISC_SUM_MASK |
> --
> 1.9.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH v2 01/12] fix: arm64: dts: replace wrong regulator on ap emmc
On Thu, Feb 11, 2021 at 01:57:25PM +, Kostya Porotchkin wrote: > > > -- > > On Wed, Feb 10, 2021 at 04:09:38PM +0200, kos...@marvell.com wrote: > > > From: Konstantin Porotchkin > > > > > > Replace wrong regulator in AP0 eMMC definition on MacchiatoBIN board > > > with 3.3V regulator. > > > The MacchiatoBIN board has no 1.8V regulator connected to AP0 eMMC > > > (ap0_sdhci0) interface. > > > > There seems to be some variability between Macchiatobin versions according > > to the schematics. > > > > The VDDO_H supply is connected to the eMMC VCCQ pins, and is also > > connected to the AP_VDDO_H pins. It is wired to the 1.8V regulator on rev > > 1.1 > > schematics, but hard-wired to the 3.3V regulator on rev 1.3 schematics. > > > > This needs clarification from SolidRun before the patch can be accepted - > > was > > VDDO_H ever wired to the 1.8V regulator on production hardware? > > > [KP] I will try to find a relevant contact in SolidRun for get this issue > clarified. I've already added Jon Nettleton. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:22:35PM +, Stefan Chulski wrote: > > Ditto. > > > > I don't think these need to be fixed in the net tree, but it would still be > > nice > > to fix the problem. Please do so, as an initial patch in your series - so > > we can > > then backport if it turns out to eventually be necessary. > > > > Thanks. > > My series already has 15 patches and patchwork not happy about series with > over 15 patches. > Maybe I can send this as separate patch to net-next(or net) first and base > this series on this net-next tree with this patch? In that case, send the fixes as a separate series and get that merged first. It shouldn't take very long to get the fixes merged. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:02:14PM +, Stefan Chulski wrote:
> > On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 761f745..8b4073c 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -1133,14 +1133,19 @@ static inline void
> > > mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) static
> > > void mvpp2_interrupts_mask(void *arg) {
> > > struct mvpp2_port *port = arg;
> > > + int cpu = smp_processor_id();
> > > + u32 thread;
> > >
> > > /* If the thread isn't used, don't do anything */
> > > - if (smp_processor_id() > port->priv->nthreads)
> > > + if (cpu > port->priv->nthreads)
> > > return;
> >
> > What happened to a patch fixing this? Did I miss it? Was it submitted
> > independently to the net tree?
>
> Some reviewers asked to remove this from the series. I would send it as
> separate patch to net.
It is not a regression, and although it is a fix, as you explained when
I first raised it, it isn't a condition that can be reached due to:
priv->nthreads = min_t(unsigned int, num_present_cpus(),
MVPP2_MAX_THREADS);
and I don't think we support a dynamic present CPU mask on any platform
that is currently supported by this driver.
If we did, then it would be possible for the off-by-one issue to be
triggered.
No matter what, it should happen _before_ this patch set is merged.
Trying to do it afterwards guarantees more pain if stable trees decide
they want to backport the fix.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 14/15] net: mvpp2: set 802.3x GoP Flow Control mode
On Thu, Feb 11, 2021 at 12:49:01PM +0200, stef...@marvell.com wrote: > From: Stefan Chulski > > This patch fix GMAC TX flow control autoneg. > Flow control autoneg wrongly were disabled with enabled TX > flow control. > > Signed-off-by: Stefan Chulski > Acked-by: Marcin Wojtas Should this patch be placed towards the start of this series (along with the other fix for the thread number limit I mentioned previously?) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 13/15] net: mvpp2: add PPv23 RX FIFO flow control
On Thu, Feb 11, 2021 at 12:49:00PM +0200, stef...@marvell.com wrote:
> +/* Configure Rx FIFO Flow control thresholds */
> +void mvpp23_rx_fifo_fc_en(struct mvpp2 *priv, int port, bool en)
> +{
> + int val;
u32 ?
> +
> + val = mvpp2_read(priv, MVPP2_RX_FC_REG(port));
> +
> + if (en)
> + val |= MVPP2_RX_FC_EN;
> + else
> + val &= ~MVPP2_RX_FC_EN;
> +
> + mvpp2_write(priv, MVPP2_RX_FC_REG(port), val);
if (en)
val = MVPP2_RX_FC_EN;
else
val = 0;
mvpp2_modify(priv + MVPP2_RX_FC_REG(port), MVPP2_RX_FC_EN, val);
?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 09/15] net: mvpp2: enable global flow control
On Thu, Feb 11, 2021 at 12:48:56PM +0200, stef...@marvell.com wrote:
> +static void mvpp2_cm3_write(struct mvpp2 *priv, u32 offset, u32 data)
> +{
> + writel(data, priv->cm3_base + offset);
> +}
> +
> +static u32 mvpp2_cm3_read(struct mvpp2 *priv, u32 offset)
> +{
> + return readl(priv->cm3_base + offset);
> +}
> +
Would it also make sense to have mvpp2_cm3_modify() ? You seem to be
adding several instances of read-modify-write sequences to CM3 RAM in
your series.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 07/15] net: mvpp2: add FCA periodic timer configurations
On Thu, Feb 11, 2021 at 12:48:54PM +0200, stef...@marvell.com wrote:
> @@ -751,6 +760,10 @@
> #define MVPP2_TX_FIFO_THRESHOLD(kb) \
> ((kb) * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
>
> +/* MSS Flow control */
> +#define FC_QUANTA0x
> +#define FC_CLK_DIVIDER 100
You later change the number of tabs for these definitions in a later
patch. Would it be better to start having the correct number of tabs?
> +
> /* RX buffer constants */
> #define MVPP2_SKB_SHINFO_SIZE \
> SKB_DATA_ALIGN(sizeof(struct skb_shared_info))
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 5730900..761f745 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1280,6 +1280,49 @@ static void mvpp22_gop_init_10gkr(struct mvpp2_port
> *port)
> writel(val, mpcs + MVPP22_MPCS_CLK_RESET);
> }
>
> +static void mvpp22_gop_fca_enable_periodic(struct mvpp2_port *port, bool en)
> +{
> + struct mvpp2 *priv = port->priv;
> + void __iomem *fca = priv->iface_base + MVPP22_FCA_BASE(port->gop_id);
> + u32 val;
net likes to have reverse christmas tree variables. I think you should
clean this up. However...
> +
> + val = readl(fca + MVPP22_FCA_CONTROL_REG);
> + val &= ~MVPP22_FCA_ENABLE_PERIODIC;
> + if (en)
> + val |= MVPP22_FCA_ENABLE_PERIODIC;
> + writel(val, fca + MVPP22_FCA_CONTROL_REG);
if (en)
val = MVPP22_FCA_ENABLE_PERIODIC;
else
val = 0;
mvpp2_modify(priv->iface_base + MVPP22_FCA_BASE(port->gop_id) +
MVPP22_FCA_CONTROL_REG, MVPP22_FCA_ENABLE_PERIODIC, val);
avoids the need for "fca".
> +}
> +
> +static void mvpp22_gop_fca_set_timer(struct mvpp2_port *port, u32 timer)
> +{
> + struct mvpp2 *priv = port->priv;
> + void __iomem *fca = priv->iface_base + MVPP22_FCA_BASE(port->gop_id);
> + u32 lsb, msb;
Same reverse christmas tree issue here.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 761f745..8b4073c 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1133,14 +1133,19 @@ static inline void
> mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec)
> static void mvpp2_interrupts_mask(void *arg)
> {
> struct mvpp2_port *port = arg;
> + int cpu = smp_processor_id();
> + u32 thread;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (cpu > port->priv->nthreads)
> return;
What happened to a patch fixing this? Did I miss it? Was it submitted
independently to the net tree?
> @@ -1150,20 +1155,25 @@ static void mvpp2_interrupts_mask(void *arg)
> static void mvpp2_interrupts_unmask(void *arg)
> {
> struct mvpp2_port *port = arg;
> - u32 val;
> + int cpu = smp_processor_id();
> + u32 val, thread;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (cpu > port->priv->nthreads)
> return;
Ditto.
I don't think these need to be fixed in the net tree, but it would still
be nice to fix the problem. Please do so, as an initial patch in your
series - so we can then backport if it turns out to eventually be
necessary.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 05/15] net: mvpp2: add PPv23 version definition
On Thu, Feb 11, 2021 at 12:48:52PM +0200, stef...@marvell.com wrote:
> From: Stefan Chulski
>
> This patch add PPv23 version definition.
> PPv23 is new packet processor in CP115.
> Everything that supported by PPv22, also supported by PPv23.
> No functional changes in this stage.
>
> Signed-off-by: Stefan Chulski
> Acked-by: Marcin Wojtas
Reviewed-by: Russell King
> @@ -7049,6 +7049,11 @@ static int mvpp2_probe(struct platform_device *pdev)
> priv->port_map |= BIT(i);
> }
>
> + if (priv->hw_version != MVPP21) {
> + if (mvpp2_read(priv, MVPP2_VER_ID_REG) == MVPP2_VER_PP23)
> + priv->hw_version = MVPP23;
> + }
> +
The only minor comment I have on this is... the formatting of the
above. Wouldn't:
if (priv->hw_version >= MVPP22 &&
mvpp2_read(priv, MVPP2_VER_ID_REG) == MVPP2_VER_PP23)
priv->hw_version = MVPP23;
read better?
Do we need to even check priv->hw_version here? Isn't this register
implemented in PPv2.1 where it contains the value zero?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 06/15] net: mvpp2: increase BM pool and RXQ size
On Thu, Feb 11, 2021 at 12:48:53PM +0200, stef...@marvell.com wrote: > From: Stefan Chulski > > BM pool and RXQ size increased to support Firmware Flow Control. > Minimum depletion thresholds to support FC are 1024 buffers. > BM pool size increased to 2048 to have some 1024 buffers > space between depletion thresholds and BM pool size. > > Jumbo frames require a 9888B buffer, so memory requirements > for data buffers increased from 7MB to 24MB. > > Signed-off-by: Stefan Chulski > Acked-by: Marcin Wojtas Reviewed-by: Russell King -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 04/15] net: mvpp2: always compare hw-version vs MVPP21
On Thu, Feb 11, 2021 at 12:48:51PM +0200, stef...@marvell.com wrote:
> @@ -1199,7 +1199,7 @@ static bool mvpp2_port_supports_xlg(struct mvpp2_port
> *port)
>
> static bool mvpp2_port_supports_rgmii(struct mvpp2_port *port)
> {
> - return !(port->priv->hw_version == MVPP22 && port->gop_id == 0);
> + return !(port->priv->hw_version != MVPP21 && port->gop_id == 0);
I'm still very much of the opinion (as raised several revisions back)
that using > MVPP21 or >= MVPP22 would be a lot better - especially
when we have situations like this. Having negatives within negatives
does not help readability.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 03/15] net: mvpp2: add CM3 SRAM memory map
On Thu, Feb 11, 2021 at 12:48:50PM +0200, stef...@marvell.com wrote:
> +static int mvpp2_get_sram(struct platform_device *pdev,
> + struct mvpp2 *priv)
> +{
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + if (!res) {
> + if (has_acpi_companion(&pdev->dev))
> + dev_warn(&pdev->dev, "ACPI is too old, Flow control not
> supported\n");
> + else
> + dev_warn(&pdev->dev, "DT is too old, Flow control not
> supported\n");
> + return 0;
> + }
> +
> + priv->cm3_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(priv->cm3_base))
> + return PTR_ERR(priv->cm3_base);
> +
> + return 0;
You can clean this up to use:
return PTR_ERR_OR_ZERO(priv->cm3_base);
> +
> + /* Map CM3 SRAM */
> + err = mvpp2_get_sram(pdev, priv);
> + if (err)
> + dev_warn(&pdev->dev, "Fail to alloc CM3 SRAM\n");
It looks to me like mvpp2_get_sram() only fails if we are unable to
_map_ the CM3 SRAM. We are no longer allocating anything from it, so
I think this message needs to be updated.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v13 net-next 01/15] doc: marvell: add CM3 address space and PPv2.3 description
On Thu, Feb 11, 2021 at 12:48:48PM +0200, stef...@marvell.com wrote: > From: Stefan Chulski > > Patch adds CM3 address space and PPv2.3 description. > > Signed-off-by: Stefan Chulski > Acked-by: Marcin Wojtas It seems this is missing the ack that you got from Rob in your previous posting. Your changelog says that only the module parameter was removed, so I guess nothing changed in this patch. Please wait to see if there are further comments before posting another revision of this series. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2 01/12] fix: arm64: dts: replace wrong regulator on ap emmc
On Wed, Feb 10, 2021 at 04:09:38PM +0200, kos...@marvell.com wrote: > From: Konstantin Porotchkin > > Replace wrong regulator in AP0 eMMC definition on MacchiatoBIN > board with 3.3V regulator. > The MacchiatoBIN board has no 1.8V regulator connected to AP0 > eMMC (ap0_sdhci0) interface. There seems to be some variability between Macchiatobin versions according to the schematics. The VDDO_H supply is connected to the eMMC VCCQ pins, and is also connected to the AP_VDDO_H pins. It is wired to the 1.8V regulator on rev 1.1 schematics, but hard-wired to the 3.3V regulator on rev 1.3 schematics. This needs clarification from SolidRun before the patch can be accepted - was VDDO_H ever wired to the 1.8V regulator on production hardware? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v2 02/12] dts: mvebu: Update A8K AP806/AP807 SDHCI settings
On Wed, Feb 10, 2021 at 04:09:39PM +0200, kos...@marvell.com wrote:
> From: Konstantin Porotchkin
>
> Select the AP SDHCI PHY slow mode for AP806 die only (move it
> from armada-ap80x.dtsi to armada-ap806.dtsi). This will allow
> running AP807 based devices at HS400 speed.
> Remove Ap SDHCI slow mode property from MacchiatoBin board DTS
> since it is already selected on the SoC level.
>
> Signed-off-by: Konstantin Porotchkin
Acked-by: Russell King
> ---
> arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi | 5 -
> arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 12
> arch/arm64/boot/dts/marvell/armada-ap80x.dtsi | 1 -
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> index 73733b4126e2..69653de998e2 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> @@ -109,11 +109,6 @@
>
> &ap_sdhci0 {
> bus-width = <8>;
> - /*
> - * Not stable in HS modes - phy needs "more calibration", so add
> - * the "slow-mode" and disable SDR104, SDR50 and DDR50 modes.
> - */
> - marvell,xenon-phy-slow-mode;
> no-1-8-v;
> no-sd;
> no-sdio;
> diff --git a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
> b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
> index 866628679ac7..828cd539173b 100644
> --- a/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-ap806.dtsi
> @@ -28,3 +28,15 @@
> reg = <0x278 0xa30>;
> };
> };
> +
> +&ap_sdhci0 {
> + /*
> + * SoC based on AP806 revision A0, A1 and A2 should use slow mode
> + * settings for Ap SDHCI due to HW Erratum HWE-7296210
> + * AP806 revesion B0 and later has this erratum fixed and the slow
> + * mode could be removed in board DTS:
> + * /delete-property/marvell,xenon-phy-slow-mode;
> + * Starting from B0 revision, the AP SDHCI can run with HS400 timing.
> + */
> + marvell,xenon-phy-slow-mode;
> +};
> diff --git a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> index 12e477f1aeb9..edd6131a0587 100644
> --- a/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi
> @@ -257,7 +257,6 @@
> clock-names = "core";
> clocks = <&ap_clk 4>;
> dma-coherent;
> - marvell,xenon-phy-slow-mode;
> status = "disabled";
> };
>
> --
> 2.17.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 01/20] net: phy: realtek: Fix events detection failure in LPI mode
On Wed, Feb 10, 2021 at 07:47:20PM +0300, Serge Semin wrote:
> On Tue, Feb 09, 2021 at 10:56:46AM +0000, Russell King - ARM Linux admin
> wrote:
> > On Tue, Feb 09, 2021 at 11:37:29AM +0100, Heiner Kallweit wrote:
> > > Right, adding something like a genphy_{read,write}_mmd() doesn't make
> > > too much sense for now. What I meant is just exporting mmd_phy_indirect().
> > > Then you don't have to open-code the first three steps of a mmd
> > > read/write.
> > > And it requires no additional code in phylib.
> >
> > ... but at the cost that the compiler can no longer inline that code,
> > as I mentioned in my previous reply. (However, the cost of the accesses
> > will be higher.) On the plus side, less I-cache footprint, and smaller
> > kernel code.
>
> Just to note mmd_phy_indirect() isn't defined with inline specifier,
> but just as static and it's used twice in the
> drivers/net/phy/phy-core.c unit. So most likely the compiler won't
> inline the function code in there.
You can't always tell whether the compiler will inline a static function
or not.
> Anyway it's up to the PHY
> library maintainers to decide. Please settle the issue with Heiner and
> Andrew then. I am ok with both solutions and will do as you decide.
FYI, *I* am one of the phylib maintainers.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next] net: phy: introduce phydev->port
On Wed, Feb 10, 2021 at 12:20:02PM +0100, Michael Walle wrote: > > Am 2021-02-09 17:38, schrieb Michael Walle: > > --- a/drivers/net/phy/phy.c > > +++ b/drivers/net/phy/phy.c > > @@ -308,7 +308,7 @@ void phy_ethtool_ksettings_get(struct phy_device > > *phydev, > > if (phydev->interface == PHY_INTERFACE_MODE_MOCA) > > cmd->base.port = PORT_BNC; > > else > > - cmd->base.port = PORT_MII; > > + cmd->base.port = phydev->port; > > cmd->base.transceiver = phy_is_internal(phydev) ? > > XCVR_INTERNAL : XCVR_EXTERNAL; > > cmd->base.phy_address = phydev->mdio.addr; > > Russell, the phylink has a similiar place where PORT_MII is set. I don't > know if we'd have to change that, too. What would we change it to? If there's no PHY attached and no SFP, what kind of interface do we have? As we've no idea what's on the media side, assuming that we are presenting a MII-like interface to stuff outside of what we control is entirely reasonable. Claiming the world is TP would be entirely wrong, there may not be a RJ45 jack. Consider the case where the MAC is connected to a switch. It's a MII-like link. It's certianly not TP, BNC, fiber, AUI, or direct attach. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 7/9] net: phy: icplus: select page before writing control register
On Wed, Feb 10, 2021 at 12:14:35PM +0100, Michael Walle wrote: > Am 2021-02-10 11:49, schrieb Russell King - ARM Linux admin: > The PHY doesn't support fiber and register 0..15 are always available > regardless of the selected page for the IP101G. > > genphy_() stuff will work, but the IP101G PHY driver specific functions, > like interrupt and mdix will break if someone is messing with the page > register from userspace. > > So Heiner's point was, that there are other PHY drivers which > also break when a user changes registers from userspace and no one > seemed to cared about that for now. > > I guess it boils down to: how hard should we try to get the driver > behave correctly if the user is changing registers. Or can we > just make the assumption that if the PHY driver sets the page > selection to its default, all the other callbacks will work > on this page. Provided the PHY driver uses the paged accessors for all paged registers, userspace can't break the PHY driver because we have proper locking in the paged accessors (I wrote them.) Userspace can access the paged registers too, but there will be no locking other than on each individual access. This can't be fixed without augmenting the kernel/ user API, and in any case is not a matter for the PHY driver. So, let's stop worrying about the userspace paged access problem for driver reviews; that's a core phylib and userspace API issue. The paged accessor API is designed to allow the driver author to access registers in the most efficient manner. There are two parts to it. 1) the phy_*_paged() accessors switch the page before accessing the register, and restore it afterwards. If you need to access a lot of paged registers, this can be inefficient. 2) phy_save_page()..phy_restore_page() allows wrapping of __phy_* accessors to access paged registers. 3) phy_select_page()..phy_restore_page() also allows wrapping of __phy_* accessors to access paged registers. phy_save_page() and phy_select_page() must /always/ be paired with a call to phy_restore_page(), since the former pair takes the bus lock and the latter releases it. (2) and (3) allow multiple accesses to either a single page without constantly saving and restoring the page, and can also be used to select other pages without the save/restore and locking steps. We could export __phy_read_page() and __phy_write_page() if required. While the bus lock is taken, userspace can't access any PHY on the bus. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next] net: phy: introduce phydev->port
On Wed, Feb 10, 2021 at 02:51:34AM +0100, Andrew Lunn wrote: > This is a general comment, not a problem specific to this patch. > > There is some interesting race conditions here. The marvell driver > first checks the fibre page and gets the status of the fiber port. As > you can see from the hunk above, it clears out pause, duplex, speed, > sets port to PORT_FIBRE, and then reads the PHY registers to set these > values. If link is not detected on the fibre, it swaps page and does > it all again, but for the copper port. So once per second, > phydev->port is going to flip flop PORT_FIBER->PORT_TP, if copper has > link. > > Now, the read_status() call into the driver should be performed while > holding the phydev->lock. So to the PHY state machine, this flip/flop > does not matter, it is atomic with respect to the lock. But > phy_ethtool_ksettings_get() is not talking the lock. It could see > speed, duplex, and port while they have _UNKNOWN values, or port is > part way through a flip flop. I think we need to take the lock here. > phy_ethtool_ksettings_set() should also probably take the lock. phy_ethtool_ksettings_get() needs to take the lock, otherwise it could read the phy_device members in the middle of an update. This is likely a long-standing phylib bug. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 7/9] net: phy: icplus: select page before writing control register
On Wed, Feb 10, 2021 at 11:38:18AM +0100, Michael Walle wrote: > Am 2021-02-10 11:30, schrieb Russell King - ARM Linux admin: > > On Wed, Feb 10, 2021 at 08:03:07AM +0100, Heiner Kallweit wrote: > > > On 09.02.2021 17:40, Michael Walle wrote: > > > > +out: > > > > + return phy_restore_page(phydev, oldpage, err); > > > > > > If a random page was set before entering config_init, do we actually > > > want > > > to restore it? Or wouldn't it be better to set the default page as > > > part > > > of initialization? > > > > I think you've missed asking one key question: does the paging on this > > PHY affect the standardised registers at 0..15 inclusive, or does it > > only affect registers 16..31? > > For this PHY it affects only registers >=16. But that doesn't invaldiate > the point that for other PHYs this might affect all regsisters. Eg. ones > where you could select between fiber and copper pages, right? You are modifying the code using ip101a_g_* functions, which is only used for the IP101A and IP101G PHYs. Do these devices support fiber in a way that change the first 16 registers? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 7/9] net: phy: icplus: select page before writing control register
On Wed, Feb 10, 2021 at 08:03:07AM +0100, Heiner Kallweit wrote: > On 09.02.2021 17:40, Michael Walle wrote: > > +out: > > + return phy_restore_page(phydev, oldpage, err); > > If a random page was set before entering config_init, do we actually want > to restore it? Or wouldn't it be better to set the default page as part > of initialization? I think you've missed asking one key question: does the paging on this PHY affect the standardised registers at 0..15 inclusive, or does it only affect registers 16..31? If it doesn't affect the standardised registers, then the genphy_* functions don't care which page is selected. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v11 net-next 15/15] net: mvpp2: add TX FC firmware check
On Tue, Feb 09, 2021 at 10:42:31AM +0200, stef...@marvell.com wrote:
> if (priv->global_tx_fc && priv->hw_version != MVPP21) {
> - val = mvpp2_cm3_read(priv, MSS_FC_COM_REG);
> - val |= FLOW_CONTROL_ENABLE_BIT;
> - mvpp2_cm3_write(priv, MSS_FC_COM_REG, val);
> + err = mvpp2_enable_global_fc(priv);
> + if (err) {
> + dev_warn(&pdev->dev, "CM3 firmware not running, version
> should be higher than 18.09 ");
> + dev_warn(&pdev->dev, "and chip revision B0\n");
> + dev_warn(&pdev->dev, "Flow control not supported\n");
I would much rather this was:
dev_warn(&pdev->dev, "Minimum of CM3 firmware 18.09 and
chip revision B0 required for flow control\n");
rather than trying to split it across several kernel messages.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 01/20] net: phy: realtek: Fix events detection failure in LPI mode
On Tue, Feb 09, 2021 at 11:37:29AM +0100, Heiner Kallweit wrote:
> Right, adding something like a genphy_{read,write}_mmd() doesn't make
> too much sense for now. What I meant is just exporting mmd_phy_indirect().
> Then you don't have to open-code the first three steps of a mmd read/write.
> And it requires no additional code in phylib.
... but at the cost that the compiler can no longer inline that code,
as I mentioned in my previous reply. (However, the cost of the accesses
will be higher.) On the plus side, less I-cache footprint, and smaller
kernel code.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 01/20] net: phy: realtek: Fix events detection failure in LPI mode
On Tue, Feb 09, 2021 at 01:15:28PM +0300, Serge Semin wrote:
> On Mon, Feb 08, 2021 at 09:14:02PM +0100, Heiner Kallweit wrote:
> > Nice analysis. Alternatively to duplicating this code piece we could
> > export mmd_phy_indirect(). But up to you.
>
> I also considered creating a generic method to access the MMD
> registers of a generic PHY, something like phy_read()/phy_write(), but
> for MMD (alas just exporting mmd_phy_indirect() would not be enough).
> But as I see it such methods need to be created only after we get to
> have at least several places with duplicating direct MMD-read/write
> patterns. Doing that just for a single place seems redundant. Anyway it's
> up to maintainers to decide whether they want to see a generic part
> of the phy_read_mmd()/phy_write_mmd() methods being detached and
> exported as something like genphy_{read,write}_mmd() methods. I can do
> that in v2 if you ask me to.
Please not genphy_* - that namespace is used for up-to-1G PHYs.
I thought about suggesting what you are proposing, but the problem is
this is just making things less and less efficient. Every time we
break a function up and export it, we increase the execution overhead
of the code. That said, the PHY accesses are relatively slow.
My opinion is that as this is just a single location at the moment,
it is not worth the effort - but if we get more of examples of this,
then it makes sense to provide the common accessor.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH v5 07/15] net: mdiobus: Introduce fwnode_mdiobus_register_phy()
On Mon, Feb 08, 2021 at 08:42:36PM +0530, Calvin Johnson wrote:
> +int fwnode_mdiobus_register_phy(struct mii_bus *bus,
> + struct fwnode_handle *child, u32 addr)
> +{
> + struct mii_timestamper *mii_ts;
If you initialise this to NULL...
> + struct phy_device *phy;
> + bool is_c45 = false;
> + u32 phy_id;
> + int rc;
> +
> + if (is_of_node(child)) {
> + mii_ts = of_find_mii_timestamper(to_of_node(child));
> + if (IS_ERR(mii_ts))
> + return PTR_ERR(mii_ts);
> + }
> +
> + rc = fwnode_property_match_string(child, "compatible",
> "ethernet-phy-ieee802.3-c45");
> + if (rc >= 0)
> + is_c45 = true;
> +
> + if (is_c45 || fwnode_get_phy_id(child, &phy_id))
> + phy = get_phy_device(bus, addr, is_c45);
> + else
> + phy = phy_device_create(bus, addr, phy_id, 0, NULL);
> + if (IS_ERR(phy)) {
> + if (mii_ts && is_of_node(child))
Then you don't need is_of_node() here.
> + /* phy->mii_ts may already be defined by the PHY driver. A
> + * mii_timestamper probed via the device tree will still have
> + * precedence.
> + */
> + if (mii_ts)
> + phy->mii_ts = mii_ts;
Should this be moved out of the if() case?
I'm thinking of the future where we may end up adding mii timestamper
support for ACPI.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH v5 15/15] net: dpaa2-mac: Add ACPI support for DPAA2 MAC driver
On Mon, Feb 08, 2021 at 08:42:44PM +0530, Calvin Johnson wrote:
> Modify dpaa2_mac_connect() to support ACPI along with DT.
> Modify dpaa2_mac_get_node() to get the dpmac fwnode from either
> DT or ACPI.
>
> Replace of_get_phy_mode with fwnode_get_phy_mode to get
> phy-mode for a dpmac_node.
>
> Use helper function phylink_fwnode_phy_connect() to find phy_dev and
> connect to mac->phylink.
>
> Signed-off-by: Calvin Johnson
I don't think this does the full job.
> static int dpaa2_pcs_create(struct dpaa2_mac *mac,
> - struct device_node *dpmac_node, int id)
> + struct fwnode_handle *dpmac_node,
> + int id)
> {
> struct mdio_device *mdiodev;
> - struct device_node *node;
> + struct fwnode_handle *node;
>
> - node = of_parse_phandle(dpmac_node, "pcs-handle", 0);
> - if (!node) {
> + node = fwnode_find_reference(dpmac_node, "pcs-handle", 0);
> + if (IS_ERR(node)) {
> /* do not error out on old DTS files */
> netdev_warn(mac->net_dev, "pcs-handle node not found\n");
> return 0;
> }
>
> - if (!of_device_is_available(node)) {
> + if (!of_device_is_available(to_of_node(node))) {
If "node" is an ACPI node, then to_of_node() returns NULL, and
of_device_is_available(NULL) is false. So, if we're using ACPI
and we enter this path, we will always hit the error below:
> netdev_err(mac->net_dev, "pcs-handle node not available\n");
> - of_node_put(node);
> + of_node_put(to_of_node(node));
> return -ENODEV;
> }
> @@ -306,7 +321,7 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
>* error out if the interface mode requests them and there is no PHY
>* to act upon them
>*/
> - if (of_phy_is_fixed_link(dpmac_node) &&
> + if (of_phy_is_fixed_link(to_of_node(dpmac_node)) &&
If "dpmac_node" is an ACPI node, to_of_node() will return NULL, and
of_phy_is_fixed_link() will oops.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH v5 13/15] phylink: introduce phylink_fwnode_phy_connect()
On Mon, Feb 08, 2021 at 08:42:42PM +0530, Calvin Johnson wrote: > Define phylink_fwnode_phy_connect() to connect phy specified by > a fwnode to a phylink instance. > > Signed-off-by: Calvin Johnson Also, the subject line should be "net: phylink: ..." Consistency is really appreciated. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH v5 13/15] phylink: introduce phylink_fwnode_phy_connect()
On Mon, Feb 08, 2021 at 08:42:42PM +0530, Calvin Johnson wrote:
> +int phylink_fwnode_phy_connect(struct phylink *pl,
> +struct fwnode_handle *fwnode,
> +u32 flags)
> +{
> + struct fwnode_handle *phy_fwnode;
> + struct phy_device *phy_dev;
> + int ret;
> +
> + if (is_of_node(fwnode)) {
> + /* Fixed links and 802.3z are handled without needing a PHY */
> + if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> + (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> + phy_interface_mode_is_8023z(pl->link_interface)))
> + return 0;
This difference between ACPI and DT really needs to be described in the
commit description.
For example, why is it acceptable to have a PHY in fixed-link mode if
we're using ACPI, and not DT?
If we look at the phylink code, accepting a PHY when in fixed-link mode
is basically not supported... so why should ACPI allow this?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v1 5/7] ARM i.MX6q: remove Atheros AR8035 SmartEEE fixup
On Mon, Feb 08, 2021 at 10:20:38AM +0100, Oleksij Rempel wrote: > On Wed, Feb 03, 2021 at 09:56:28AM +0000, Russell King - ARM Linux admin > wrote: > > That is the historical fix for this problem, but there is a better > > solution now in net-next - configuring the Tw parameter for gigabit > > connections. That solves the random link drop issue when EEE is > > enabled. > > Do you mean this properties? > qca,smarteee-tw-us-1g > qca,smarteee-tw-us-100m > > Do you have some recommendations, which values can be here used? Are > they same for all MACs? Or, can we calculate this values automatically? I don't think there's a way to "calculate" them. The AR8035 default is 17us for 1G and 23us for 100M. Increasing the 1G Tw to 26us or 27us fixes it on several different Solidrun platforms (iMX6 Hummingboards and Cubox-i, and LX2160A based). The boards all have differing layouts, so I don't think it's layout or SoC specific (which is good news.) These figures have been arrived at by repetitive long-term testing and observing whether there are sporadic link drops over these platforms. > Beside, I have seen this patch: "ARM: dts: imx6qdl-sr-som: fix some > cubox-i platforms" That's for a different problem: moving these settings to DT broke some Cubox-i platforms because of the weird ways that the AR8035 configures the address bits, using the LED pin. Tying a LED to the LED pin is not sufficient to guarantee that a board always configures the PHY to a particular address, so it can appear on address 0 or 4 depending on noise, temperature, supply voltage, and PHY chip thresholds. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: kexec: Fix panic after TLB are invalidated
On Fri, Feb 05, 2021 at 12:40:54AM +, Giancarlo Ferrari wrote: > Russell, > > On Fri, Feb 05, 2021 at 12:18:33AM +, Russell King - ARM Linux admin > wrote: > > On Thu, Feb 04, 2021 at 11:48:42PM +, Giancarlo Ferrari wrote: > > > Can I ask about having it integrated ? > > > > Thanks for testing. Are you willing for me to add: > > > > Tested-by: Giancarlo Ferrari > > > > to the commit log? > > > > Sure. > > I have a question regarding the patch. I saw that the structure start at > the end of the relocation code: > > data = reboot_code_buffer + relocate_new_kernel_size; > > which means it overlap with the global symbol relocate_new_kernel_size. > I think is minor comment as the variable is only used in the fncpy() > then thrown away. The same is true of the rest of the kernel image if that's how you'd like to look at it. relocate_new_kernel_size is just there to tell the C code the size of the function, nothing more nothing less. It isn't there to be copied. The copied code doesn't use it. > Something like: > > data = reboot_code_buffer + relocate_new_kernel_size + sizeof(long); No. That will place the structure after the size variable, which we don't want, and... > and accordingly in the instruction (arch/arm/kernel/relocate_kernel.S): > > adr r7, relocate_new_kernel_end ... we will then need to follow this with: add r7, r7, #4 or replace it with: adr r7, relocate_new_kernel_end + 4 so that r7 points at "data". -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] ARM: kexec: Fix panic after TLB are invalidated
On Thu, Feb 04, 2021 at 11:48:42PM +, Giancarlo Ferrari wrote: > Can I ask about having it integrated ? Thanks for testing. Are you willing for me to add: Tested-by: Giancarlo Ferrari to the commit log? I can move it into the fixes branch which I want to send to Linus by Saturday at the very latest. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 09:31:06PM +, Guillaume Tucker wrote:
> On 04/02/2021 18:23, Nick Desaulniers wrote:
> > You're right, I missed `LLVM=1`. Adding `LD=ld.bfd` I think should
> > permit fallback to BFD.
>
> That was close, except we're cross-compiling with GCC for arm.
> So I've now built a plain next-20210203 (without Ard's fix) using
> this command line:
>
> make LD=arm-linux-gnueabihf-ld.bfd -j18 ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf- LLVM=1 CC="ccache clang" zImage
>
> I'm using a modified Docker image gtucker/kernelci-build-clang-11
> with the very latest LLVM 11 and gcc-8-arm-linux-gnueabihf
> packages added to be able to use the GNU linker. BTW I guess we
> should enable this kind of hybrid build setup on kernelci.org as
> well.
...
> And this booted fine, which confirms it's really down to how
> ld.lld puts together the kernel image. Does it actually solve
> the debate whether this is an issue to fix in the assembly code
> or at link time?
Well... as I mentioned previously, we don't really understand what
is going on between the decompressor running with the caches on,
turning the caches off, jumping into the decompressed kernel, and
then getting to the v7 setup code.
The results from various attempts at solving the problem which lead
to Ard's original patch that caused your breakage were not making a
whole lot of sense (I think I wrote that all up in a previous email
thread, so won't repeat it here.)
So, I was slightly nervous about merging Ard's fix - and your report
suggested that there is indeed more going on here that we don't
understand.
When I was tracking down what was going on, I had this patch applied
(I've had to recreate it, so may not be exactly what I had), with the
DEBUG_LL stuff appropriately enabled. It may be worth applying this
patch, enabling the DEBUG_LL stuff appropriately for one of your
failing boards, and try booting it.
You should get two strings of identical hex numbers that look
something like:
480009000401400030004820071d40008090
If they're looking like instructions, for example:
ee060f37e3a00080ee020f10ee020f30ee030f10e3a00903ee050f30
Then it's likely that you are seeing a very similar problem as I was
without Ard's patch. If you do get instruction-like content, then
you will likely find the sequence of instructions in the decompressor
code.
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 28c9d32fa99a..19fa93ae282c 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -475,7 +475,39 @@ ENDPROC(cpu_pj4b_do_resume)
ldr r12, [r0]
add r12, r12, r0@ the local stack
stmia r12, {r1-r6, lr}@ v7_invalidate_l1 touches r0-r6
+ ldr r0, [r12, #0]
+ bl printhex8
+ ldr r0, [r12, #4]
+ bl printhex8
+ ldr r0, [r12, #8]
+ bl printhex8
+ ldr r0, [r12, #12]
+ bl printhex8
+ ldr r0, [r12, #16]
+ bl printhex8
+ ldr r0, [r12, #20]
+ bl printhex8
+ ldr r0, [r12, #24]
+ bl printhex8
+ mov r0, #'\n'
+ bl printch
bl v7_invalidate_l1
+ ldr r0, [r12, #0]
+ bl printhex8
+ ldr r0, [r12, #4]
+ bl printhex8
+ ldr r0, [r12, #8]
+ bl printhex8
+ ldr r0, [r12, #12]
+ bl printhex8
+ ldr r0, [r12, #16]
+ bl printhex8
+ ldr r0, [r12, #20]
+ bl printhex8
+ ldr r0, [r12, #24]
+ bl printhex8
+ mov r0, #'\n'
+ bl printch
ldmia r12, {r1-r6, lr}
__v7_setup_cont:
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
On Thu, Feb 04, 2021 at 05:56:50PM +0100, Greg Kroah-Hartman wrote: > On Thu, Feb 04, 2021 at 04:52:24PM +0000, Russell King - ARM Linux admin > wrote: > > On Tue, Feb 02, 2021 at 03:06:05PM +0100, Greg Kroah-Hartman wrote: > > > I'm glad to take this through my char/misc tree, as that's where the > > > other coresight changes flow through. So if no one else objects, I will > > > do so... > > > > Greg, did you end up pulling this after all? If not, Uwe produced a v2. > > I haven't merged v2 yet as I don't know what you've done. > > I thought you merged this? I took v1, and put it in a branch I've promised in the past not to rebase/rewind. Uwe is now asking for me to take a v2 or apply a patch on top. The only reason to produce an "immutable" branch is if it's the basis for some dependent work and you need that branch merged into other people's trees... so the whole "lets produce a v2" is really odd workflow... I'm confused about what I should do, and who has to be informed which option I take. I'm rather lost here too. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [GIT PULL] immutable branch for amba changes targeting v5.12-rc1
On Tue, Feb 02, 2021 at 03:06:05PM +0100, Greg Kroah-Hartman wrote: > I'm glad to take this through my char/misc tree, as that's where the > other coresight changes flow through. So if no one else objects, I will > do so... Greg, did you end up pulling this after all? If not, Uwe produced a v2. I haven't merged v2 yet as I don't know what you've done. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 03:25:20PM +0100, Ard Biesheuvel wrote: > Pushing contents of the cache hierarchy to main memory is *not* a > valid use of set/way ops, and so there is no point in pretending that > set/way ops will produce the same results as by-VA ops. Only the by-VA > ops give the architectural guarantees that we rely on for correctness. ... yet we /were/ doing that, and it worked fine for 13 years - from 1st June 2007 until the by-VA merge into mainline on the 3rd April 2020. You may be right that it wasn't the most correct way, but it worked for those 13 years without issue, and it's only recently that it's become a problem, and trying to "fix" that introduced a regression, and fixing that regression has caused another regression... and I what I'm wondering is how many more regression fixing cycles it's going to take - how many regression fixes on top of other regression fixes are we going to end up seeing here. The fact is, we never properly understood why your patch caused the regression I was seeing. If we don't understand it, then we can never say that we've fixed the problem properly. That is highlighted by the fact that fixing the regression I was seeing has caused another regression. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 12:26:44PM +, Marc Zyngier wrote: > I agree. With set/way CMOs, there is no way to reach the PoC if > it beyond the system cache, leading to an unbootable kernel. > This is actually pretty well documented in the architecture, > and it did bite us for the first time on XGene-1, 7 years ago. That may be, however we still do set/way maintenance to invalidate the L1 cache as that is required for ARMv7 to place the cache into a known state, as stated by the architecture reference manual. Arguably, that should be done by firmware, but when starting secondary CPUs, there are platforms out there which do not bring the L1 cache to a defined state. So we are pretty much stuck with doing set/way operations during CPU initialisation in the main kernel. If ARMv8 decides that this is not supportable, then that's a matter for ARMv8 to address without impacting the requirements of ARMv7. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 11:32:05AM +, Guillaume Tucker wrote: > Yes it does fix the issue: > > https://lava.collabora.co.uk/scheduler/job/3173819 > > with Ard's fix applied to this test branch: > > https://gitlab.collabora.com/gtucker/linux/-/commits/next-20210203-ard-fix/ > > > +clang +Nick > > It's worth mentioning that the issue only happens with kernels > built with Clang. As you can see there are several other arm > platforms failing with clang-11 builds but booting fine with > gcc-8: My gut feeling is that it isn't Clang specific - it's likely down to the exact code/data placement, how things end up during decompression, and exactly what state the cache ends up in. That certainly was the case with the original regression. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 11:27:16AM +0100, Ard Biesheuvel wrote: > Hi Russell, > > If Guillaume is willing to do the experiment, and it fixes the issue, > it proves that rk3288 is relying on the flush before the MMU is > disabled, and so in that case, the fix is trivial, and we can just > apply it. > > If the experiment fails (which would mean rk3288 does not tolerate the > cache maintenance being performed after cache off), it is going to be > hairy, and so it will definitely take more time. > > So in the latter case (or if Guillaume does not get back to us), I > think reverting my queued fix is the only sane option. But in that > case, may I suggest that we queue the revert of the original by-VA > change for v5.12 so it gets lots of coverage in -next, and allows us > an opportunity to come up with a proper fix in the same timeframe, and > backport the revert and the subsequent fix as a pair? Otherwise, we'll > end up in the situation where v5.10.x until today has by-va, v5.10.x-y > has set/way, and v5.10y+ has by-va again. (I don't think we care about > anything before that, given that v5.4 predates any of this) I'm suggesting dropping your fix (9052/1) and reverting "ARM: decompressor: switch to by-VA cache maintenance for v7 cores" which gets us to a point where _both_ regressions are fixed. I'm of the opinion that the by-VA patch was incorrect when it was merged (it caused a regression), and it's only a performance improvement. Our attempts so far to fix it are just causing other regressions. So, I think it is reasonable to revert both back to a known good point which has worked over a decade. If doing so causes regressions (which I think is unlikely), then that would be unfortunate but alas is a price that's worth paying to get back to a known good point - since then we're not stacking regression fixes on top of other regression fixes. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: next/master bisection: baseline.login on rk3288-rock2-square
On Thu, Feb 04, 2021 at 10:07:58AM +0100, Ard Biesheuvel wrote: > On Thu, 4 Feb 2021 at 09:43, Guillaume Tucker > wrote: > > > > Hi Ard, > > > > Please see the bisection report below about a boot failure on > > rk3288 with next-20210203. It was also bisected on > > imx6q-var-dt6customboard with next-20210202. > > > > Reports aren't automatically sent to the public while we're > > trialing new bisection features on kernelci.org but this one > > looks valid. > > > > The kernel is most likely crashing very early on, so there's > > nothing in the logs. Please let us know if you need some help > > with debugging or trying a fix on these platforms. > > > > Thanks for the report. Ard, I want to send my fixes branch today which includes your regression fix that caused this regression. As this is proving difficult to fix, I can only drop your fix from my fixes branch - and given that this seems to be problematical, I'm tempted to revert the original change at this point which should fix both of these regressions - and then we have another go at getting rid of the set/way instructions during the next cycle. Thoughts? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH 02/11] dts: mvebu: Update A8K AP806 SDHCI settings
On Wed, Feb 03, 2021 at 02:50:45PM +, Kostya Porotchkin wrote: > [KP] So for older systems this "slow mode" parameter could be set on the > board level. > When it is set in ap80x,dtsi file it downgrades all systems to HS-SDR52, even > if they support HS400 on AP side. > MacchiatoBIN AP eMMC is connected to 3.3v regulator and has "no-1-8-v" flag > set, so it should remain in low speed anyway. Your reasoning does not make sense. The ap80x.dtsi file does not specify "marvell,xenon-phy-slow-mode". It is not specified at this level. It is already specified at board level. Given that Macchiatobin will still use slow mode, why remove the marvell,xenon-phy-slow-mode property from this file? Also, if you're upgrading ap80x.dtsi to use a bus-width of 8, why keep the bus-width specifier of 8 in the board files? This patch just doesn't make sense, and your responses to our points seem to add to the confusion. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH 02/11] dts: mvebu: Update A8K AP806 SDHCI settings
On Wed, Feb 03, 2021 at 02:37:22PM +, Kostya Porotchkin wrote: > Hi, Baruch, > > > -Original Message- > > From: Baruch Siach > > Sent: Wednesday, February 3, 2021 15:59 > > To: Kostya Porotchkin > > Cc: linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; > > and...@lunn.ch; j...@semihalf.com; gregory.clem...@bootlin.com; > > li...@armlinux.org.uk; Nadav Haklai ; > > robh...@kernel.org; Stefan Chulski ; > > m...@semihalf.com; Ben Peled ; > > sebastian.hesselba...@gmail.com; linux-arm-ker...@lists.infradead.org > > Subject: [EXT] Re: [PATCH 02/11] dts: mvebu: Update A8K AP806 SDHCI > > settings > > > > External Email > > > > -- > > Hi Konstantin, > > > > On Wed, Feb 03 2021, kos...@marvell.com wrote: > > > From: Konstantin Porotchkin > > > > > > Update the settings for AP806 SDHCI interface according to latest > > > Xenon drivers changes. > > > - no need to select the PHY slow mode anymore > > > > Why? Has anything changed since the introduction of marvell,xenon-phy-slow- > > mode? > [KP] AP806 B0, AP807 and later do not need the "slow mode" set by the default. > The HWE-7296210 errata is not applicable to these components and they are > able > to run AP SDHCI in HS400 8-bit mode. So what about all those people, such as me, who have A0 silicon on their Macchiatobin boards? You can't just go around removing DT properties like this. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 03/11] dts: mvebu: Add pin control definitions for SDIO interafce
On Wed, Feb 03, 2021 at 03:31:30PM +0200, kos...@marvell.com wrote:
> From: Konstantin Porotchkin
>
> Add SDIO mode pin control configration for CP0 on A8K DB.
>
> Signed-off-by: Konstantin Porotchkin
> ---
> arch/arm64/boot/dts/marvell/armada-70x0.dtsi | 6 ++
> arch/arm64/boot/dts/marvell/armada-80x0.dtsi | 6 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-70x0.dtsi
> b/arch/arm64/boot/dts/marvell/armada-70x0.dtsi
> index 293403a1a333..179218774ba9 100644
> --- a/arch/arm64/boot/dts/marvell/armada-70x0.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-70x0.dtsi
> @@ -47,6 +47,12 @@
> cp0_pinctrl: pinctrl {
> compatible = "marvell,armada-7k-pinctrl";
>
> + sdhci_pins: sdhi-pins {
sdhi-pins ?
> + marvell,pins = "mpp56", "mpp57", "mpp58",
> +"mpp59", "mpp60", "mpp61", "mpp62";
> + marvell,function = "sdio";
> + };
> +
> nand_pins: nand-pins {
> marvell,pins =
> "mpp15", "mpp16", "mpp17", "mpp18",
> diff --git a/arch/arm64/boot/dts/marvell/armada-80x0.dtsi
> b/arch/arm64/boot/dts/marvell/armada-80x0.dtsi
> index ee67c70bf02e..64100ae204da 100644
> --- a/arch/arm64/boot/dts/marvell/armada-80x0.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-80x0.dtsi
> @@ -70,6 +70,12 @@
> &cp0_syscon0 {
> cp0_pinctrl: pinctrl {
> compatible = "marvell,armada-8k-cpm-pinctrl";
> +
> + sdhci_pins: sdhi-pins {
sdhi-pins ?
> + marvell,pins = "mpp56", "mpp57", "mpp58",
> +"mpp59", "mpp60", "mpp61", "mpp62";
> + marvell,function = "sdio";
> + };
> };
> };
>
> --
> 2.17.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v1 5/7] ARM i.MX6q: remove Atheros AR8035 SmartEEE fixup
On Wed, Feb 03, 2021 at 10:18:55AM +0100, Oleksij Rempel wrote:
> This fixup removes the Lpi_en bit.
>
> If this patch breaks functionality of your board, use following device
> tree properties:
>
> ethernet-phy@X {
> reg = <0xX>;
> eee-broken-1000t;
> eee-broken-100tx;
>
> };
That is the historical fix for this problem, but there is a better
solution now in net-next - configuring the Tw parameter for gigabit
connections. That solves the random link drop issue when EEE is
enabled.
Support for this configuration has only recently been merged into
net-next and other trees for this merge window, so I ask that you
hold off at least this patch until the next cycle.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v1 0/7] remove different PHY fixups
On Wed, Feb 03, 2021 at 10:18:50AM +0100, Oleksij Rempel wrote: > This patch series tries to remove most of the imx6 and imx7 board > specific PHY configuration via fixup, as this breaks the PHYs when > connected to switch chips or USB Ethernet MACs. > > Each patch has the possibility to break boards, but contains a > recommendation to fix the problem in a more portable and future-proof > way. Please wait a cycle for the changes I've submitted through net-next to be merged, so we don't end up with stuff breaking during the merge window because these changes have been merged before the changes in net-next. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH] net: phy: add Marvell 88X2222 transceiver support
On Mon, Feb 01, 2021 at 10:22:51PM +0300, Ivan Bornyakov wrote:
> +/* PMD Transmit Disable */
> +#define MV_TX_DISABLE 0x0009
> +#define MV_TX_DISABLE_GLOBALBIT(0)
Please use MDIO_PMA_TXDIS and MDIO_PMD_TXDIS_GLOBAL; this is an
IEEE802.3 defined register.
> +/* 10GBASE-R PCS Status 1 */
> +#define MV_10GBR_STAT MDIO_STAT1
Nothing Marvell specific here, please use MDIO_STAT1 directly.
> +/* 1000Base-X/SGMII Control Register */
> +#define MV_1GBX_CTRL0x2000
> +
> +/* 1000BASE-X/SGMII Status Register */
> +#define MV_1GBX_STAT0x2001
> +
> +/* 1000Base-X Auto-Negotiation Advertisement Register */
> +#define MV_1GBX_ADVERTISE 0x2004
Marvell have had a habbit of placing other PHY instances within the
register space. This also looks like Clause 22 layout rather than
Clause 45 layout - please use the Clause 22 definitions for the bits
rather than Clause 45. (so BMCR_ANENABLE, BMCR_ANRESTART for
MV_1GBX_CTRL, etc).
Please define these as:
+#defineMV_1GBX_CTRL(0x2000 + MII_BMCR)
+#defineMV_1GBX_STAT(0x2000 + MII_BMSR)
+#defineMV_1GBX_ADVERTISE (0x2000 + MII_ADVERTISE)
to make it clear what is going on here.
> +static int sfp_module_insert(void *_priv, const struct sfp_eeprom_id *id)
> +{
> + struct phy_device *phydev = _priv;
> + struct device *dev = &phydev->mdio.dev;
> + struct mv_data *priv = phydev->priv;
> + phy_interface_t interface;
> +
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> + sfp_parse_support(phydev->sfp_bus, id, supported);
> + interface = sfp_select_interface(phydev->sfp_bus, supported);
> +
> + dev_info(dev, "%s SFP module inserted", phy_modes(interface));
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_10GBASER:
> + phydev->speed = SPEED_1;
> + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> + linkmode_set_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> + phydev->supported);
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> + mv_soft_reset(phydev);
> + break;
> + case PHY_INTERFACE_MODE_1000BASEX:
> + default:
> + phydev->speed = SPEED_1000;
> + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> + linkmode_clear_bit(ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> +phydev->supported);
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> + mv_soft_reset(phydev);
> + }
> +
> + priv->sfp_inserted = true;
> +
> + if (priv->net_up)
> + mv_tx_enable(phydev);
This is racy. priv->net_up is modified via the suspend/resume
callbacks, which are called with phydev->lock held. No other locks
are guaranteed to be held.
However, this function is called with the SFP sm_mutex, and rtnl
held. Consequently, the use of sfp_inserted and net_up in this
function and the suspend/resume callbacks is racy.
Why are you disabling the transmitter anyway? Is this for power
saving?
> +static void mv_update_interface(struct phy_device *phydev)
> +{
> + if ((phydev->speed == SPEED_1000 ||
> + phydev->speed == SPEED_100 ||
> + phydev->speed == SPEED_10) &&
> + phydev->interface != PHY_INTERFACE_MODE_1000BASEX) {
> + phydev->interface = PHY_INTERFACE_MODE_1000BASEX;
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> + mv_soft_reset(phydev);
> + } else if (phydev->speed == SPEED_1 &&
> +phydev->interface != PHY_INTERFACE_MODE_10GBASER) {
> + phydev->interface = PHY_INTERFACE_MODE_10GBASER;
> +
> + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> + mv_soft_reset(phydev);
> + }
This looks wrong. phydev->interface is the _host_ interface, which
you are clearly setting to XAUI here. Some network drivers depend
on this being correct (for instance, when used with the Marvell
88x3310 PHY which changes its host-side interface dynamically.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
