Re: [PATCH 06/11] bcma: convert bus code to use dev_groups
2013/10/9 John W. Linville : > On Sun, Oct 06, 2013 at 11:55:45PM -0700, Greg Kroah-Hartman wrote: >> The dev_attrs field of struct bus_type is going away soon, dev_groups >> should be used instead. This converts the bcma bus code to use the >> correct field. >> >> Cc: Rafał Miłecki >> Cc: >> Signed-off-by: Greg Kroah-Hartman >> --- >> >> Rafał, I can take this through my driver-core tree if you like, just let >> me know what would be the easiest for you. > > Makes sense to me... Oops, sorry, missed that. I'll just agree with John :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] drm: tegra: Add HDMI support
2012/11/9 Thierry Reding : > +/* all fields little endian */ > +struct hdmi_audio_infoframe { > + /* PB0 */ > + u8 csum; > + > + /* PB1 */ > + unsigned cc:3; /* channel count */ > + unsigned res1:1; > + unsigned ct:4; /* coding type */ > + > + /* PB2 */ > + unsigned ss:2; /* sample size */ > + unsigned sf:3; /* sample frequency */ > + unsigned res2:3; > + > + /* PB3 */ > + unsigned cxt:5; /* coding extention type */ > + unsigned res3:3; > + > + /* PB4 */ > + u8 ca; /* channel/speaker allocation */ > + > + /* PB5 */ > + unsigned res5:3; > + unsigned lsv:4; /* level shift value */ > + unsigned dm_inh:1; /* downmix inhibit */ > + > + /* PB6-10 reserved */ > + u8 res6; > + u8 res7; > + u8 res8; > + u8 res9; > + u8 res10; > +} __packed; I was told it won't work on different endian devices. See [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe http://lists.freedesktop.org/archives/dri-devel/2012-May/022544.html -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] drm: tegra: Add HDMI support
2012/11/9 Christian König : > On 09.11.2012 16:45, Rafał Miłecki wrote: >> I was told it won't work on different endian devices. See >> [RFC][PATCH] drm/radeon/hdmi: define struct for AVI infoframe >> http://lists.freedesktop.org/archives/dri-devel/2012-May/022544.html > > > Yeah, that's indeed true. And honestly adding just another implementation of > the HDMI info frames sounds like somebody should finally sit down and > implement it in a common drm_hdmi.c Agree. -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 1/2] mtd: bcm47: convert kzalloc to avoid invalid access
2013/5/30 Libo Chen : > mtd is just member of bcm47xxsflash, so we should free bcm47xxsflash not its > member. > So I use devm_kazlloc instead of kazlloc to avoid it. I think you should use mtd: bcm47xxsflash: prefix for both patches, but Artem or David may want to comment on that. Nice trick with that devm_kzalloc, I didn't know it. -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/mtd/devices/bcm47xxsflash.c: correct argument to kfree
2013/6/8 Julia Lawall : > From: Julia Lawall > > The argument to kfree should not be the address of a structure field. > The argument is adjusted to correspond to what is found in the subsequent > remove function. This was already addressed in [PATCH RESEND 1/2] mtd: bcm47: convert kzalloc to avoid invalid access Artem: can you pick pointed patch, please? So people don't report it over and over ;) -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pull request: wireless 2013-04-08
2013/4/8 John W. Linville : > Please consider this set of fixes for the 3.9 stream... > (...) > > Please let me know if there are problems! John, could you take a look at what has happened to the [PATCH V2] ssb: implement spurious tone avoidance ? Message-Id: <1364911046-5732-1-git-send-email-zaj...@gmail.com> This patch missed last 2 fix merges (3.9) for some reason. -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] mtd: fix kfree bcm47xxsflash
2013/5/22 Libo Chen : > mtd is just member of bcm47xxsflash, so we should free bcm47xxsflash not its > member. Thanks! -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] mtd: fix kfree bcm47xxsflash
2013/5/28 Libo Chen : > ping... It takes a lot of time for someone to pick up mtd patches, just be patient ;) -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v3] b43: N-PHY: fix gain in b43_nphy_get_gain_ctl_workaround_ent()
2013/1/20 Dan Carpenter : > There were no break statements in this switch statement so everything > used the default settings. Per Walter Harms's suggestion, I've replaced > the switch statement and done a little cleanup. > > Signed-off-by: Dan Carpenter > --- > v2: Make additional style fixes as well while we're messing with the > function. > v3: Make the array static const int. Looks fine, thanks Dan -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] mtd: add support for partition parsers
This extends MTD subsystem by adding a support for partition parsers that should be used for creating subpartitions. There are some types of partitions that require splitting, like firmware containers. It's common some home routers that a single firmware image gets flashed to predefined partition and then we need to parse it dynamically. The reason for having such parsers is to share code. Right now we have e.g. TRX parsing implemented in bcm47xxpart but this could be used in more cases (e.g. on ramips which doesn't use bcm47xxpart or with DT). This implementation requires marking partition as requiring parsing with a specific parser. This can be used right away with some parsers like bcm47xxpart, in future we will hopefully also find a solution for doing it with ofpart ("fixed-partitions"). Signed-off-by: Rafał Miłecki --- One thing I'm not proud of in this patch is struct mtd_partition casting in order to be able to modify partition offset. It's marked as const so it was the only way I could think of. Any better ideas? Currently parser gets a single MTD and it doesn't have to deal with its offset, which I kind of like as it simplifies things. --- drivers/mtd/mtdpart.c | 34 +- include/linux/mtd/partitions.h | 5 + 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index 1f13e32..0203e21 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -663,6 +663,34 @@ int mtd_del_partition(struct mtd_info *master, int partno) } EXPORT_SYMBOL_GPL(mtd_del_partition); +static int mtd_parse_part(struct mtd_part *slave, const char *parser) +{ + static const char *probes[2] = { NULL, NULL }; + struct mtd_partitions parsed = { 0 }; + int i; + int err; + + probes[0] = parser; + err = parse_mtd_partitions(&slave->mtd, probes, &parsed, NULL); + + if (err) + return err; + else if (!parsed.nr_parts) + return -ENOENT; + + for (i = 0; i < parsed.nr_parts; i++) { + struct mtd_partition *part; + + part = (struct mtd_partition *)&parsed.parts[i]; + part->offset += slave->offset; + } + err = add_mtd_partitions(slave->master, parsed.parts, parsed.nr_parts); + + if (parsed.parser) + kfree(parsed.parts); + return err; +} + /* * This function, given a master MTD object and a partition table, creates * and registers slave MTD objects which are bound to the master according to @@ -683,7 +711,9 @@ int add_mtd_partitions(struct mtd_info *master, printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name); for (i = 0; i < nbparts; i++) { - slave = allocate_partition(master, parts + i, i, cur_offset); + const struct mtd_partition *part = parts + i; + + slave = allocate_partition(master, part, i, cur_offset); if (IS_ERR(slave)) { del_mtd_partitions(master); return PTR_ERR(slave); @@ -695,6 +725,8 @@ int add_mtd_partitions(struct mtd_info *master, add_mtd_device(&slave->mtd); mtd_add_partition_attrs(slave); + if (part->parser) + mtd_parse_part(slave, part->parser); cur_offset = slave->offset + slave->mtd.size; } diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h index 70736e1..c6da77f9 100644 --- a/include/linux/mtd/partitions.h +++ b/include/linux/mtd/partitions.h @@ -20,6 +20,10 @@ * * For each partition, these fields are available: * name: string that will be used to label the partition's MTD device. + * parser: some partitions may require specific handling like splitting them + * into subpartitions (e.g. firmware partition may contain partitions + * table, kernel image and rootfs). This field may contain a name of parser + * than can handle specific type. * size: the partition size; if defined as MTDPART_SIZ_FULL, the partition * will extend to the end of the master MTD device. * offset: absolute starting position within the master MTD device; if @@ -38,6 +42,7 @@ struct mtd_partition { const char *name; /* identifier string */ + const char *parser; /* parser name */ uint64_t size; /* partition size */ uint64_t offset;/* offset within the master MTD space */ uint32_t mask_flags;/* master MTD flags to mask out for this partition */ -- 1.8.4.5
[PATCH 2/2] mtd: extract TRX parser out of bcm47xxpart into separated module
This makes TRX parsing code reusable with other platforms and parsers. There is still some place for improvement (e.g. working on some smarter method of checking rootfs format), for now we simply move the code as it is. Signed-off-by: Rafał Miłecki --- drivers/mtd/Kconfig | 4 ++ drivers/mtd/Makefile | 1 + drivers/mtd/bcm47xxpart.c| 63 + drivers/mtd/parsers/Kconfig | 8 +++ drivers/mtd/parsers/Makefile | 1 + drivers/mtd/parsers/parser_trx.c | 119 +++ 6 files changed, 136 insertions(+), 60 deletions(-) create mode 100644 drivers/mtd/parsers/Kconfig create mode 100644 drivers/mtd/parsers/Makefile create mode 100644 drivers/mtd/parsers/parser_trx.c diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index e83a279..5a2d717 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -155,6 +155,10 @@ config MTD_BCM47XX_PARTS This provides partitions parser for devices based on BCM47xx boards. +menu "Partition parsers" +source "drivers/mtd/parsers/Kconfig" +endmenu + comment "User Modules And Translation Layers" # diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index 99bb9a1..151d60df 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_MTD_AFS_PARTS) += afs.o obj-$(CONFIG_MTD_AR7_PARTS)+= ar7part.o obj-$(CONFIG_MTD_BCM63XX_PARTS)+= bcm63xxpart.o obj-$(CONFIG_MTD_BCM47XX_PARTS)+= bcm47xxpart.o +obj-y += parsers/ # 'Users' - code which presents functionality to userspace. obj-$(CONFIG_MTD_BLKDEVS) += mtd_blkdevs.o diff --git a/drivers/mtd/bcm47xxpart.c b/drivers/mtd/bcm47xxpart.c index 845dd27..61a6f45 100644 --- a/drivers/mtd/bcm47xxpart.c +++ b/drivers/mtd/bcm47xxpart.c @@ -42,7 +42,6 @@ #define ML_MAGIC2 0x26594131 #define TRX_MAGIC 0x30524448 #define SHSQ_MAGIC 0x71736873 /* shsq (weird ZTE H218N endianness) */ -#define UBI_EC_MAGIC 0x23494255 /* UBI# */ struct trx_header { uint32_t magic; @@ -61,28 +60,6 @@ static void bcm47xxpart_add_part(struct mtd_partition *part, const char *name, part->mask_flags = mask_flags; } -static const char *bcm47xxpart_trx_data_part_name(struct mtd_info *master, - size_t offset) -{ - uint32_t buf; - size_t bytes_read; - int err; - - err = mtd_read(master, offset, sizeof(buf), &bytes_read, - (uint8_t *)&buf); - if (err && !mtd_is_bitflip(err)) { - pr_err("mtd_read error while parsing (offset: 0x%X): %d\n", - offset, err); - goto out_default; - } - - if (buf == UBI_EC_MAGIC) - return "ubi"; - -out_default: - return "rootfs"; -} - static int bcm47xxpart_parse(struct mtd_info *master, const struct mtd_partition **pparts, struct mtd_part_parser_data *data) @@ -190,44 +167,10 @@ static int bcm47xxpart_parse(struct mtd_info *master, trx = (struct trx_header *)buf; trx_part = curr_part; - bcm47xxpart_add_part(&parts[curr_part++], "firmware", + bcm47xxpart_add_part(&parts[curr_part], "firmware", offset, 0); - - i = 0; - /* We have LZMA loader if offset[2] points to sth */ - if (trx->offset[2]) { - bcm47xxpart_add_part(&parts[curr_part++], -"loader", -offset + trx->offset[i], -0); - i++; - } - - if (trx->offset[i]) { - bcm47xxpart_add_part(&parts[curr_part++], -"linux", -offset + trx->offset[i], -0); - i++; - } - - /* -* Pure rootfs size is known and can be calculated as: -* trx->length - trx->offset[i]. We don't fill it as -* we want to have jffs2 (overlay) in the same mtd. -*/ - if (trx->offset[i]) { - const char *name; - -
Re: [PATCH V2] leds: trigger: Introduce an USB port trigger
On 20 July 2016 at 03:02, Rob Herring wrote: > On Fri, Jul 15, 2016 at 11:10:45PM +0200, Rafał Miłecki wrote: >> This commit adds a new trigger that can turn on LED when USB device gets >> connected to the USB port. This can be useful for various home routers >> that have USB port and a proper LED telling user a device is connected. >> >> Right now this trigger is usable with a proper DT only, there isn't a >> way to specify USB ports from user space. This may change in a future. >> >> Signed-off-by: Rafał Miłecki >> --- >> V2: The first version got support for specifying list of USB ports from >> user space only. There was a (big try &) discussion on adding DT >> support. It led to a pretty simple solution of comparing of_node of >> usb_device to of_node specified in usb-ports property. >> Since it appeared DT support may be simpler and non-DT a bit more >> complex, this version drops previous support for "ports" and >> "new_port" and focuses on DT only. The plan is to see if this >> solution with DT is OK, get it accepted and then work on non-DT. >> >> Felipe: if there won't be any objections I'd like to ask for your Ack. >> --- >> Documentation/devicetree/bindings/leds/common.txt | 11 ++ >> Documentation/leds/ledtrig-usbport.txt| 19 ++ >> drivers/leds/trigger/Kconfig | 8 + >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-usbport.c| 206 >> ++ >> 5 files changed, 245 insertions(+) >> create mode 100644 Documentation/leds/ledtrig-usbport.txt >> create mode 100644 drivers/leds/trigger/ledtrig-usbport.c >> >> diff --git a/Documentation/devicetree/bindings/leds/common.txt >> b/Documentation/devicetree/bindings/leds/common.txt >> index af10678..75536f7 100644 >> --- a/Documentation/devicetree/bindings/leds/common.txt >> +++ b/Documentation/devicetree/bindings/leds/common.txt >> @@ -50,6 +50,12 @@ property can be omitted. >> For controllers that have no configurable timeout the flash-max-timeout-us >> property can be omitted. >> >> +Trigger specific properties for child nodes: >> + >> +usbport trigger: >> +- usb-ports : List of USB ports that usbport should observed for turning on >> a >> + given LED. > > I think this should be more generic such that it could work for disk or > network LEDs too. Perhaps 'led-triggers = '? trigger is a bit of > a Linux name, but I haven't thought of anything better. Really, I'd > prefer the link in the other direction (e.g. port node have a 'leds" or > '*-leds' property), but that's maybe harder to parse. I was already thinking on this as I plan to add "netdev" trigger at some point in the future. I'd like to use "net-devices" for it (as a equivalent or "usb-ports"). However there is a problem with your idea of sharing "led-triggers" between triggers.. Consider device with generic purpose LED that should be controllable by a user. I believe we should let user switch it's state, e.g. with: echo usbport > trigger echo netdev > trigger with a shared property I don't see how we couldn't handle it properly. I don't think we can use anything like: led-triggers = <&ohci_port1>, <&ehci_port1>, <ðmac0>; I'd get even more complicated if we ever need cells for any trigger. AFAIK it's not allowed to mix handles with different cells like: led-triggers = <&ohci_port1>, <&foo 5>, <ðmac0>; These problems made me use trigger-specific properies for specifying LED triggers. Do you have any other idea for sharing a property & avoiding above problems at the same time? >> + >> Examples: >> >> system-status { >> @@ -58,6 +64,11 @@ system-status { >> ... >> }; >> >> +usb { >> + label = "USB"; > > It's not really clear in the example this is an LED node as it is > incomplete. What do you mean by incomplete? Should I include e.g. ohci_port1 in this example? >> + usb-ports = <&ohci_port1>, <&ehci_port1>; >> +}; >> + >> camera-flash { >> label = "Flash"; >> led-sources = <0>, <1>; >> diff --git a/Documentation/leds/ledtrig-usbport.txt >> b/Documentation/leds/ledtrig-usbport.txt >> new file mode 100644 >> index 000..642c4cd >> --- /dev/null >> +++ b/Documentation/leds/ledtrig-usbport.txt >> @@ -0,0
Re: [PATCH v2 4/6] net: ethernet: bgmac: convert to feature flags
On 17 August 2016 at 13:34, Rafał Miłecki wrote: > On 8 July 2016 at 01:08, Jon Mason wrote: >> mode = (bgmac_read(bgmac, BGMAC_DEV_STATUS) & BGMAC_DS_MM_MASK) >> >> BGMAC_DS_MM_SHIFT; >> - if (ci->id != BCMA_CHIP_ID_BCM47162 || mode != 0) >> + if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST || mode != 0) >> bgmac_set(bgmac, BCMA_CLKCTLST, BCMA_CLKCTLST_FORCEHT); >> - if (ci->id == BCMA_CHIP_ID_BCM47162 && mode == 2) >> + if (bgmac->feature_flags & BGMAC_FEAT_CLKCTLST && mode == 2) >> bcma_chipco_chipctl_maskset(&bgmac->core->bus->drv_cc, 1, ~0, >> BGMAC_CHIPCTL_1_RXC_DLL_BYPASS); > > Jon, it looks to me you translated two following conditions: > ci->id != BCMA_CHIP_ID_BCM47162 > and > ci->id == BCMA_CHIP_ID_BCM47162 > into the same flag check: > bgmac->feature_flags & BGMAC_FEAT_CLKCTLST > > I don't think it's intentional, is it? Do you have a moment to fix this? Ping -- Rafał
[PATCH RFC] brcmfmac: stop netif queue when waiting for packets transmission
From: Rafał Miłecki Sending a new key to the firmware should be done without any 802.1x packets pending. Currently brcmfmac has very trivial code waiting for that condition and it doesn't seem to be enough. We should stop netif from sending any extra packets in order to: 1) Make sure new 802.1x packets won't be coming over and over 2) Avoid a race with netif providing a new packet right after our waiting code Another solution would be to accept only non-802.1x packets. This would require enqueuing all packets and hacking brcmf_fws_dequeue_worker to dequeue only non-802.1x ones but that would most likely result in too hacky code. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 201a980..1791060 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -471,11 +471,14 @@ send_key_to_dongle(struct brcmf_if *ifp, struct brcmf_wsec_key *key) convert_key_from_CPU(key, &key_le); + netif_stop_queue(ifp->ndev); brcmf_netdev_wait_pend8021x(ifp); err = brcmf_fil_bsscfg_data_set(ifp, "wsec_key", &key_le, sizeof(key_le)); + netif_start_queue(ifp->ndev); + if (err) brcmf_err("wsec_key error (%d)\n", err); return err; -- 2.9.3
[PATCH] brcmfmac: fix memory leak in brcmf_fill_bss_param
From: Rafał Miłecki This function is called from get_station callback which means that every time user space was getting/dumping station(s) we were leaking 2 KiB. Signed-off-by: Rafał Miłecki Fixes: 1f0dc59a6de ("brcmfmac: rework .get_station() callback") Cc: sta...@vger.kernel.org # 4.2+ --- Kalle, ideally this should go as 4.8 fix, but I'm aware it's quite late. If you are not planning to send another pull request, just get it for the next release and let's let stable guys backport it. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index b8aec5e5..62a7675 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -2533,7 +2533,7 @@ static void brcmf_fill_bss_param(struct brcmf_if *ifp, struct station_info *si) WL_BSS_INFO_MAX); if (err) { brcmf_err("Failed to get bss info (%d)\n", err); - return; + goto out_kfree; } si->filled |= BIT(NL80211_STA_INFO_BSS_PARAM); si->bss_param.beacon_interval = le16_to_cpu(buf->bss_le.beacon_period); @@ -2545,6 +2545,9 @@ static void brcmf_fill_bss_param(struct brcmf_if *ifp, struct station_info *si) si->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_PREAMBLE; if (capability & WLAN_CAPABILITY_SHORT_SLOT_TIME) si->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_SLOT_TIME; + +out_kfree: + kfree(buf); } static s32 -- 2.9.3
[PATCH] USB: bcma: drop Northstar PHY 2.0 initialization code
From: Rafał Miłecki This driver should initialize controller only, PHY initialization should be handled by separated PHY driver. We already have phy-bcm-ns-usb2 in place so let it makes its duty. Signed-off-by: Rafał Miłecki --- drivers/usb/host/bcma-hcd.c | 80 ++--- 1 file changed, 25 insertions(+), 55 deletions(-) diff --git a/drivers/usb/host/bcma-hcd.c b/drivers/usb/host/bcma-hcd.c index e0761d9..5f425c8 100644 --- a/drivers/usb/host/bcma-hcd.c +++ b/drivers/usb/host/bcma-hcd.c @@ -239,44 +239,10 @@ static int bcma_hcd_usb20_old_arm_init(struct bcma_hcd_device *usb_dev) return 0; } -static void bcma_hcd_init_chip_arm_phy(struct bcma_device *dev) -{ - struct bcma_device *arm_core; - void __iomem *dmu; - - arm_core = bcma_find_core(dev->bus, BCMA_CORE_ARMCA9); - if (!arm_core) { - dev_err(&dev->dev, "can not find ARM Cortex A9 ihost core\n"); - return; - } - - dmu = ioremap_nocache(arm_core->addr_s[0], 0x1000); - if (!dmu) { - dev_err(&dev->dev, "can not map ARM Cortex A9 ihost core\n"); - return; - } - - /* Unlock DMU PLL settings */ - iowrite32(0xea68, dmu + 0x180); - - /* Write USB 2.0 PLL control setting */ - iowrite32(0x00dd10c3, dmu + 0x164); - - /* Lock DMU PLL settings */ - iowrite32(0x, dmu + 0x180); - - iounmap(dmu); -} - -static void bcma_hcd_init_chip_arm_hc(struct bcma_device *dev) +static void bcma_hcd_usb20_ns_init_hc(struct bcma_device *dev) { u32 val; - /* -* Delay after PHY initialized to ensure HC is ready to be configured -*/ - usleep_range(1000, 2000); - /* Set packet buffer OUT threshold */ val = bcma_read32(dev, 0x94); val &= 0x; @@ -287,20 +253,33 @@ static void bcma_hcd_init_chip_arm_hc(struct bcma_device *dev) val = bcma_read32(dev, 0x9c); val |= 1; bcma_write32(dev, 0x9c, val); + + /* +* Broadcom initializes PHY and then waits to ensure HC is ready to be +* configured. In our case the order is reversed. We just initialized +* controller and we let HCD initialize PHY, so let's wait (sleep) now. +*/ + usleep_range(1000, 2000); } -static void bcma_hcd_init_chip_arm(struct bcma_device *dev) +/** + * bcma_hcd_usb20_ns_init - Initialize Northstar USB 2.0 controller + */ +static int bcma_hcd_usb20_ns_init(struct bcma_hcd_device *bcma_hcd) { - bcma_core_enable(dev, 0); + struct bcma_device *core = bcma_hcd->core; + struct bcma_chipinfo *ci = &core->bus->chipinfo; + struct device *dev = &core->dev; - if (dev->bus->chipinfo.id == BCMA_CHIP_ID_BCM4707 || - dev->bus->chipinfo.id == BCMA_CHIP_ID_BCM53018) { - if (dev->bus->chipinfo.pkg == BCMA_PKG_ID_BCM4707 || - dev->bus->chipinfo.pkg == BCMA_PKG_ID_BCM4708) - bcma_hcd_init_chip_arm_phy(dev); + bcma_core_enable(core, 0); - bcma_hcd_init_chip_arm_hc(dev); - } + if (ci->id == BCMA_CHIP_ID_BCM4707 || + ci->id == BCMA_CHIP_ID_BCM53018) + bcma_hcd_usb20_ns_init_hc(core); + + of_platform_default_populate(dev->of_node, NULL, dev); + + return 0; } static void bcma_hci_platform_power_gpio(struct bcma_device *dev, bool val) @@ -373,16 +352,7 @@ static int bcma_hcd_usb20_init(struct bcma_hcd_device *usb_dev) if (dma_set_mask_and_coherent(dev->dma_dev, DMA_BIT_MASK(32))) return -EOPNOTSUPP; - switch (dev->id.id) { - case BCMA_CORE_NS_USB20: - bcma_hcd_init_chip_arm(dev); - break; - case BCMA_CORE_USB20_HOST: - bcma_hcd_init_chip_mips(dev); - break; - default: - return -ENODEV; - } + bcma_hcd_init_chip_mips(dev); /* In AI chips EHCI is addrspace 0, OHCI is 1 */ ohci_addr = dev->addr_s[0]; @@ -451,7 +421,7 @@ static int bcma_hcd_probe(struct bcma_device *core) err = -ENOTSUPP; break; case BCMA_CORE_NS_USB20: - err = bcma_hcd_usb20_init(usb_dev); + err = bcma_hcd_usb20_ns_init(usb_dev); break; case BCMA_CORE_NS_USB30: err = bcma_hcd_usb30_init(usb_dev); -- 2.9.3
[PATCH 3/3] ARM: BCM5301X: Specify USB 3.0 PHY in DT
From: Rafał Miłecki Driver for Northstar USB 3.0 PHY has been recently added under the name phy-bcm-ns-usb3. Add binding for it into the DT files. The only slightly tricky part is BCM47094 which uses different PHY version and requires different compatible value. Signed-off-by: Rafał Miłecki --- arch/arm/boot/dts/bcm47094.dtsi | 6 ++ arch/arm/boot/dts/bcm5301x.dtsi | 7 +++ 2 files changed, 13 insertions(+) diff --git a/arch/arm/boot/dts/bcm47094.dtsi b/arch/arm/boot/dts/bcm47094.dtsi index f039765..4f09aa0 100644 --- a/arch/arm/boot/dts/bcm47094.dtsi +++ b/arch/arm/boot/dts/bcm47094.dtsi @@ -6,6 +6,12 @@ #include "bcm4708.dtsi" +/ { + usb3_phy: usb3-phy { + compatible = "brcm,ns-bx-usb3-phy"; + }; +}; + &uart0 { clock-frequency = <12500>; }; diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi index ae4b388..f09a2bb 100644 --- a/arch/arm/boot/dts/bcm5301x.dtsi +++ b/arch/arm/boot/dts/bcm5301x.dtsi @@ -149,6 +149,13 @@ clock-names = "phy-ref-clk"; }; + usb3_phy: usb3-phy { + compatible = "brcm,ns-ax-usb3-phy"; + reg = <0x18105000 0x1000>, <0x18003000 0x1000>; + reg-names = "dmp", "ccb-mii"; + #phy-cells = <0>; + }; + axi@1800 { compatible = "brcm,bus-axi"; reg = <0x1800 0x1000>; -- 2.9.3
[PATCH 1/3] ARM: BCM5301X: Add separated DTS include file for BCM47094
From: Rafał Miłecki Use it to store BCM47094 specific properties/values and avoid repeating them in device DTS files. Signed-off-by: Rafał Miłecki --- arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts | 3 +-- arch/arm/boot/dts/bcm47094-netgear-r8500.dts | 3 +-- arch/arm/boot/dts/bcm47094.dtsi | 11 +++ 3 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 arch/arm/boot/dts/bcm47094.dtsi diff --git a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts index c8c0b36..661348d 100644 --- a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts +++ b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts @@ -9,7 +9,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm47094.dtsi" #include "bcm5301x-nand-cs0-bch1.dtsi" / { @@ -107,7 +107,6 @@ &uart0 { status = "okay"; - clock-frequency = <12500>; }; &usb3 { diff --git a/arch/arm/boot/dts/bcm47094-netgear-r8500.dts b/arch/arm/boot/dts/bcm47094-netgear-r8500.dts index 10db4f8..521b415 100644 --- a/arch/arm/boot/dts/bcm47094-netgear-r8500.dts +++ b/arch/arm/boot/dts/bcm47094-netgear-r8500.dts @@ -6,7 +6,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm47094.dtsi" #include "bcm5301x-nand-cs0-bch8.dtsi" / { @@ -100,5 +100,4 @@ &uart0 { status = "okay"; - clock-frequency = <12500>; }; diff --git a/arch/arm/boot/dts/bcm47094.dtsi b/arch/arm/boot/dts/bcm47094.dtsi new file mode 100644 index 000..f039765 --- /dev/null +++ b/arch/arm/boot/dts/bcm47094.dtsi @@ -0,0 +1,11 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * Licensed under the ISC license. + */ + +#include "bcm4708.dtsi" + +&uart0 { + clock-frequency = <12500>; +}; -- 2.9.3
[PATCH 2/3] ARM: BCM5301X: Enable UART on Netgear R8000
From: Rafał Miłecki It was tested by LEDE users, all we need is to adjust clock frequency. While we're at it create a separated DTS include file to share code with other BCM4709 devices easier. Signed-off-by: Rafał Miłecki --- arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts | 2 +- arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts | 2 +- arch/arm/boot/dts/bcm4709-netgear-r7000.dts | 2 +- arch/arm/boot/dts/bcm4709-netgear-r8000.dts | 6 +- arch/arm/boot/dts/bcm4709.dtsi| 11 +++ 5 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 arch/arm/boot/dts/bcm4709.dtsi diff --git a/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts b/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts index 8ade7de..eac0f52 100644 --- a/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts +++ b/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts @@ -9,7 +9,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm4709.dtsi" #include "bcm5301x-nand-cs0-bch8.dtsi" / { diff --git a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts index 0653e7e..aab39c9 100644 --- a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts +++ b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts @@ -9,7 +9,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm4709.dtsi" #include "bcm5301x-nand-cs0-bch8.dtsi" / { diff --git a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts index a22ed14..fd38d2a 100644 --- a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts +++ b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts @@ -9,7 +9,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm4709.dtsi" #include "bcm5301x-nand-cs0-bch8.dtsi" / { diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts index ca18151..92f8a72 100644 --- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts +++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts @@ -9,7 +9,7 @@ /dts-v1/; -#include "bcm4708.dtsi" +#include "bcm4709.dtsi" #include "bcm5301x-nand-cs0-bch8.dtsi" / { @@ -107,6 +107,10 @@ }; }; +&uart0 { + status = "okay"; +}; + &usb2 { vcc-gpio = <&chipcommon 0 GPIO_ACTIVE_HIGH>; }; diff --git a/arch/arm/boot/dts/bcm4709.dtsi b/arch/arm/boot/dts/bcm4709.dtsi new file mode 100644 index 000..f039765 --- /dev/null +++ b/arch/arm/boot/dts/bcm4709.dtsi @@ -0,0 +1,11 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * Licensed under the ISC license. + */ + +#include "bcm4708.dtsi" + +&uart0 { + clock-frequency = <12500>; +}; -- 2.9.3
[PATCH] ARM: BCM5301X: Set GPIO enabling USB power on Netgear R7000
There is one GPIO controlling power for both USB ports. Signed-off-by: Rafał Miłecki --- arch/arm/boot/dts/bcm4709-netgear-r7000.dts | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts index a22ed14..a76486b 100644 --- a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts +++ b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts @@ -104,3 +104,11 @@ &uart0 { status = "okay"; }; + +&usb2 { + vcc-gpio = <&chipcommon 0 GPIO_ACTIVE_HIGH>; +}; + +&usb3 { + vcc-gpio = <&chipcommon 0 GPIO_ACTIVE_HIGH>; +}; -- 1.8.4.5
Re: [PATCH] ARM: BCM5301X: Set GPIO enabling USB power on Netgear R7000
On 21 June 2016 at 14:22, Imre Kaloz wrote: > Is there any reason you are not handling this properly as a regulator with > usb-nop-xceiv? We can't use USB NOP PHY as we need a specific PHY driver for Broadcom's USB. I sent patch for this: https://patchwork.kernel.org/patch/9148097/
Re: [PATCH] ARM: BCM5301X: Set GPIO enabling USB power on Netgear R7000
On 21 June 2016 at 16:29, Imre Kaloz wrote: > On Tue, 21 Jun 2016 14:26:11 +0200, Rafał Miłecki wrote: > >> On 21 June 2016 at 14:22, Imre Kaloz wrote: >>> >>> Is there any reason you are not handling this properly as a regulator >>> with >>> usb-nop-xceiv? >> >> >> We can't use USB NOP PHY as we need a specific PHY driver for Broadcom's >> USB. > > > I see. That shouldn't stop you from addressing the regulator part, tho ;) How? Be more specific please. -- Rafał
[PATCH] brcmfmac: drop unused fields from struct brcmf_pub
From: Rafał Miłecki They seem to be there from the first day. We calculate these values but never use them. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h | 4 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 2 -- 3 files changed, 9 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 65e8c87..27cd50a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -519,9 +519,6 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked) ndev->needed_headroom += drvr->hdrlen; ndev->ethtool_ops = &brcmf_ethtool_ops; - drvr->rxsz = ndev->mtu + ndev->hard_header_len + - drvr->hdrlen; - /* set the mac address */ memcpy(ndev->dev_addr, ifp->mac_addr, ETH_ALEN); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 8fa34ca..f16cfc9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -112,15 +112,11 @@ struct brcmf_pub { /* Internal brcmf items */ uint hdrlen;/* Total BRCMF header length (proto + bus) */ - uint rxsz; /* Rx buffer size bus module should use */ /* Dongle media info */ char fwver[BRCMF_DRIVER_FIRMWARE_VERSION_LEN]; u8 mac[ETH_ALEN]; /* MAC address obtained from dongle */ - /* Multicast data packets sent to dongle */ - unsigned long tx_multicast; - struct mac_address addresses[BRCMF_MAX_IFS]; struct brcmf_if *iflist[BRCMF_MAX_IFS]; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c index 9f9024a..a190f53 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c @@ -2104,8 +2104,6 @@ int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb) if ((skb->priority == 0) || (skb->priority > 7)) skb->priority = cfg80211_classify8021d(skb, NULL); - drvr->tx_multicast += !!multicast; - if (fws->avoid_queueing) { rc = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb); if (rc < 0) -- 2.9.3
Re: [PATCH] firmware/broadcom: add missing header dependencies
On 24 September 2016 at 07:03, Baoyou Xie wrote: > We get 1 warning when building kernel with W=1: > drivers/firmware/broadcom/bcm47xx_sprom.c:717:5: warning: no previous > prototype for 'bcm47xx_sprom_register_fallbacks' [-Wmissing-prototypes] > > In fact, this function is declared in include/linux/bcm47xx_sprom.h, > so this patch adds missing header dependencies. > > Signed-off-by: Baoyou Xie Acked-by: Rafał Miłecki
[PATCH 1/2] brcmfmac: initialize fws(ignal) for BCDC protocol only
From: Rafał Miłecki There are two protocols used by Broadcom FullMAC devices: BCDC and msgbuf. They use different ways for (some part of) communication with the firmware. Firmware Signaling is required for the first one only (BCDC). So far we were always initializing fws and always calling it's skb processing function. It was fws that was passing skb processing to the protocol specific function. It was redundant for the msgbuf case. Simply taking few lines of code out of fws allows us to totally avoid using it. This simplifies code flow, saves some memory & will allow further optimizations like not compiling fwsignal.c. Signed-off-by: Rafał Miłecki --- .../wireless/broadcom/brcm80211/brcmfmac/core.c| 24 -- .../broadcom/brcm80211/brcmfmac/fwsignal.c | 17 ++- .../broadcom/brcm80211/brcmfmac/fwsignal.h | 1 + 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 27cd50a..bc3d8ab 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -250,7 +250,17 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb, if (eh->h_proto == htons(ETH_P_PAE)) atomic_inc(&ifp->pend_8021x_cnt); - ret = brcmf_fws_process_skb(ifp, skb); + /* determine the priority */ + if (skb->priority == 0 || skb->priority > 7) + skb->priority = cfg80211_classify8021d(skb, NULL); + + if (drvr->fws && brcmf_fws_skbs_queueing(drvr->fws)) { + ret = brcmf_fws_process_skb(ifp, skb); + } else { + ret = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb); + if (ret < 0) + brcmf_txfinalize(ifp, skb, false); + } done: if (ret) { @@ -405,7 +415,7 @@ void brcmf_txcomplete(struct device *dev, struct sk_buff *txp, bool success) struct brcmf_if *ifp; /* await txstatus signal for firmware if active */ - if (brcmf_fws_fc_active(drvr->fws)) { + if (drvr->fws && brcmf_fws_fc_active(drvr->fws)) { if (!success) brcmf_fws_bustxfail(drvr->fws, txp); } else { @@ -1006,11 +1016,13 @@ int brcmf_bus_start(struct device *dev) } brcmf_feat_attach(drvr); - ret = brcmf_fws_init(drvr); - if (ret < 0) - goto fail; + if (bus_if->proto_type == BRCMF_PROTO_BCDC) { + ret = brcmf_fws_init(drvr); + if (ret < 0) + goto fail; - brcmf_fws_add_interface(ifp); + brcmf_fws_add_interface(ifp); + } drvr->config = brcmf_cfg80211_attach(drvr, bus_if->dev, drvr->settings->p2p_enable); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c index a190f53..495eaf8 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c @@ -2100,16 +2100,6 @@ int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb) int rc = 0; brcmf_dbg(DATA, "tx proto=0x%X\n", ntohs(eh->h_proto)); - /* determine the priority */ - if ((skb->priority == 0) || (skb->priority > 7)) - skb->priority = cfg80211_classify8021d(skb, NULL); - - if (fws->avoid_queueing) { - rc = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb); - if (rc < 0) - brcmf_txfinalize(ifp, skb, false); - return rc; - } /* set control buffer information */ skcb->if_flags = 0; @@ -2155,7 +2145,7 @@ void brcmf_fws_add_interface(struct brcmf_if *ifp) struct brcmf_fws_info *fws = ifp->drvr->fws; struct brcmf_fws_mac_descriptor *entry; - if (!ifp->ndev) + if (!fws || !ifp->ndev) return; entry = &fws->desc.iface[ifp->ifidx]; @@ -2442,6 +2432,11 @@ void brcmf_fws_deinit(struct brcmf_pub *drvr) kfree(fws); } +bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws) +{ + return !fws->avoid_queueing; +} + bool brcmf_fws_fc_active(struct brcmf_fws_info *fws) { if (!fws->creditmap_received) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h index ef0ad85..8f7c1d7 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h @@ -20,6 +20,7 @@ int brcmf_fws_init(struct brcmf_pub *drvr); void brcm
[PATCH 2/2] brcmfmac: compile fws(ignal) code only with BCDC support enabled
From: Rafał Miłecki It's not needed by the other (msgbuf) protocol, so let's save some size and compile it conditionally. Signed-off-by: Rafał Miłecki --- .../wireless/broadcom/brcm80211/brcmfmac/Makefile | 4 +- .../broadcom/brcm80211/brcmfmac/fwsignal.h | 59 ++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile index 9e4b505..ad3b06e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile @@ -27,7 +27,6 @@ brcmfmac-objs += \ chip.o \ fwil.o \ fweh.o \ - fwsignal.o \ p2p.o \ proto.o \ common.o \ @@ -37,7 +36,8 @@ brcmfmac-objs += \ btcoex.o \ vendor.o brcmfmac-$(CONFIG_BRCMFMAC_PROTO_BCDC) += \ - bcdc.o + bcdc.o \ + fwsignal.o brcmfmac-$(CONFIG_BRCMFMAC_PROTO_MSGBUF) += \ commonring.o \ flowring.o \ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h index 8f7c1d7..ba0c1bc 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h @@ -18,6 +18,7 @@ #ifndef FWSIGNAL_H_ #define FWSIGNAL_H_ +#ifdef CONFIG_BRCMFMAC_PROTO_BCDC int brcmf_fws_init(struct brcmf_pub *drvr); void brcmf_fws_deinit(struct brcmf_pub *drvr); bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws); @@ -31,5 +32,63 @@ void brcmf_fws_del_interface(struct brcmf_if *ifp); void brcmf_fws_bustxfail(struct brcmf_fws_info *fws, struct sk_buff *skb); void brcmf_fws_bus_blocked(struct brcmf_pub *drvr, bool flow_blocked); void brcmf_fws_rxreorder(struct brcmf_if *ifp, struct sk_buff *skb); +#else +static inline int brcmf_fws_init(struct brcmf_pub *drvr) +{ + return -ENOTSUPP; +} + +static inline void brcmf_fws_deinit(struct brcmf_pub *drvr) +{ +} + +static inline bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws) +{ + return false; +} + +static inline bool brcmf_fws_fc_active(struct brcmf_fws_info *fws) +{ + return false; +} + +static inline void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen, +struct sk_buff *skb) +{ +} + +static inline int brcmf_fws_process_skb(struct brcmf_if *ifp, + struct sk_buff *skb) +{ + return -ENOTSUPP; +} + +static inline void brcmf_fws_reset_interface(struct brcmf_if *ifp) +{ +} + +static inline void brcmf_fws_add_interface(struct brcmf_if *ifp) +{ +} + +static inline void brcmf_fws_del_interface(struct brcmf_if *ifp) +{ +} + +static inline void brcmf_fws_bustxfail(struct brcmf_fws_info *fws, + struct sk_buff *skb) +{ +} + +static inline void brcmf_fws_bus_blocked(struct brcmf_pub *drvr, +bool flow_blocked) +{ +} + +static inline void brcmf_fws_rxreorder(struct brcmf_if *ifp, + struct sk_buff *skb) +{ +} +#endif #endif /* FWSIGNAL_H_ */ -- 2.9.3
[PATCH] brcmfmac: implement more accurate skb tracking
From: Rafał Miłecki We need to track 802.1x packets to know if there are any pending ones for transmission. This is required for performing key update in the firmware. Unfortunately our old tracking code wasn't very accurate. It was treating skb as pending as soon as it was passed by the netif. Actual handling packet to the firmware was happening later as brcmfmac internally queues them and uses its own worker(s). Other than that it was hard to handle freeing packets. Everytime we had to determine (in more generic funcions) if packet was counted as pending 802.1x one or not. It was causing some problems, e.g. it wasn't clear if brcmf_flowring_delete should free skb directly or not. This patch introduces 2 separated functions for tracking skbs. This simplifies logic, fixes brcmf_flowring_delete (maybe other hidden bugs as well) and allows further simplifications. Thanks to better accuracy is also increases time window for key update (and lowers timeout risk). Signed-off-by: Rafał Miłecki --- This was successfully tested with 4366b1. Can someone give it a try with some USB/SDIO device, please? --- .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c| 11 +++ .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 12 +++- .../wireless/broadcom/brcm80211/brcmfmac/core.c| 36 -- .../wireless/broadcom/brcm80211/brcmfmac/core.h| 2 ++ .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 14 +++-- .../wireless/broadcom/brcm80211/brcmfmac/proto.h | 11 +++ .../wireless/broadcom/brcm80211/brcmfmac/sdio.c| 8 + .../net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 10 ++ 8 files changed, 91 insertions(+), 13 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index d1bc51f..3e40244 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -326,6 +326,16 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, return 0; } +static int brcmf_proto_bcdc_hdr_get_ifidx(struct brcmf_pub *drvr, + struct sk_buff *skb) +{ + struct brcmf_proto_bcdc_header *h; + + h = (struct brcmf_proto_bcdc_header *)(skb->data); + + return BCDC_GET_IF_IDX(h); +} + static int brcmf_proto_bcdc_txdata(struct brcmf_pub *drvr, int ifidx, u8 offset, struct sk_buff *pktbuf) @@ -373,6 +383,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) } drvr->proto->hdrpull = brcmf_proto_bcdc_hdrpull; + drvr->proto->hdr_get_ifidx = brcmf_proto_bcdc_hdr_get_ifidx; drvr->proto->query_dcmd = brcmf_proto_bcdc_query_dcmd; drvr->proto->set_dcmd = brcmf_proto_bcdc_set_dcmd; drvr->proto->txdata = brcmf_proto_bcdc_txdata; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 03404cb..fef9d02 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -43,6 +43,7 @@ #include "chip.h" #include "bus.h" #include "debug.h" +#include "proto.h" #include "sdio.h" #include "core.h" #include "common.h" @@ -772,6 +773,7 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes) int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev, struct sk_buff_head *pktq) { + struct brcmf_pub *pub = sdiodev->bus_if->drvr; struct sk_buff *skb; u32 addr = sdiodev->sbwad; int err; @@ -784,10 +786,18 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev, if (pktq->qlen == 1 || !sdiodev->sg_support) skb_queue_walk(pktq, skb) { + struct brcmf_if *ifp; + int ifidx; + + ifidx = brcmf_proto_hdr_get_ifidx(pub, skb); + ifp = brcmf_get_ifp(pub, ifidx); + brcmf_tx_passing_skb(ifp, skb); err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true, addr, skb); - if (err) + if (err) { + brcmf_tx_regained_skb(ifp, skb); break; + } } else err = brcmf_sdiod_sglist_rw(sdiodev, SDIO_FUNC_2, true, addr, diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index bc3d8ab..7cdc1f6 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -247,9
Re: [PATCH] brcmfmac: implement more accurate skb tracking
On 26 September 2016 at 13:46, Arend Van Spriel wrote: > On 26-9-2016 12:23, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> We need to track 802.1x packets to know if there are any pending ones >> for transmission. This is required for performing key update in the >> firmware. > > The problem we are trying to solve is a pretty old one. The problem is > that wpa_supplicant uses two separate code paths: EAPOL messaging > through data path and key configuration though nl80211. Can I find it described/reported somewhere? >> Unfortunately our old tracking code wasn't very accurate. It was >> treating skb as pending as soon as it was passed by the netif. Actual >> handling packet to the firmware was happening later as brcmfmac >> internally queues them and uses its own worker(s). > > That does not seem right. As soon as we get a 1x packet we need to wait > with key configuration regardless whether it is still in the driver or > handed over to firmware already. OK, thanks.
Re: [PATCH] brcmfmac: implement more accurate skb tracking
On 26 September 2016 at 14:13, Rafał Miłecki wrote: > On 26 September 2016 at 13:46, Arend Van Spriel > wrote: >> On 26-9-2016 12:23, Rafał Miłecki wrote: >>> From: Rafał Miłecki >>> >>> We need to track 802.1x packets to know if there are any pending ones >>> for transmission. This is required for performing key update in the >>> firmware. >> >> The problem we are trying to solve is a pretty old one. The problem is >> that wpa_supplicant uses two separate code paths: EAPOL messaging >> through data path and key configuration though nl80211. > > Can I find it described/reported somewhere? > > >>> Unfortunately our old tracking code wasn't very accurate. It was >>> treating skb as pending as soon as it was passed by the netif. Actual >>> handling packet to the firmware was happening later as brcmfmac >>> internally queues them and uses its own worker(s). >> >> That does not seem right. As soon as we get a 1x packet we need to wait >> with key configuration regardless whether it is still in the driver or >> handed over to firmware already. > > OK, thanks. Actually, it's not OK. I was trying to report/describe/discuss this problem for over a week. I couldn't get much of answer from you. I had to come with a patch I worked on for quite some time. Only then you decided to react and reply with a reason for a nack. I see this patch may be wrong (but it's still hard to know what's going wrong without a proper hostapd bug report). I'd expect you to somehow work & communicate with open source community. -- Rafał
[PATCH] brcmfmac: proto: add callback for queuing TX data
From: Rafał Miłecki So far our core code was calling brcmf_fws_process_skb which wasn't a proper thing to do. If case of devices using msgbuf protocol fwsignal shouldn't be used. It was an unnecessary extra layer simply calling a protocol specifix txdata function. Please note we already have txdata callback, but it's used for calls between bcdc and fwsignal so it couldn't be simply used there. This makes core code more generic (instead of bcdc/fwsignal specific). Signed-off-by: Rafał Miłecki --- There is one 1 CHECK report from checkpatch.pl in this patch: CHECK: Comparison to NULL could be written "!proto->hdrpull" #157: FILE: drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c:54: + if (!proto->tx_queue_data || (proto->hdrpull == NULL) || It's caused by a code that was already there and that should be fixed in a separated patch. This shouldn't stop this patch from being applied. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 12 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 8 +++- .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 15 +-- .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h | 1 + drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 6 +++--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c | 2 +- drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h | 9 + 7 files changed, 38 insertions(+), 15 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c index 038a960..384b187 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c @@ -326,6 +326,17 @@ brcmf_proto_bcdc_hdrpull(struct brcmf_pub *drvr, bool do_fws, return 0; } +static int brcmf_proto_bcdc_tx_queue_data(struct brcmf_pub *drvr, int ifidx, + struct sk_buff *skb) +{ + struct brcmf_if *ifp = brcmf_get_ifp(drvr, ifidx); + + if (!brcmf_fws_queue_skbs(drvr->fws)) + return brcmf_proto_txdata(drvr, ifidx, 0, skb); + + return brcmf_fws_process_skb(ifp, skb); +} + static int brcmf_proto_bcdc_txdata(struct brcmf_pub *drvr, int ifidx, u8 offset, struct sk_buff *pktbuf) @@ -375,6 +386,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) drvr->proto->hdrpull = brcmf_proto_bcdc_hdrpull; drvr->proto->query_dcmd = brcmf_proto_bcdc_query_dcmd; drvr->proto->set_dcmd = brcmf_proto_bcdc_set_dcmd; + drvr->proto->tx_queue_data = brcmf_proto_bcdc_tx_queue_data; drvr->proto->txdata = brcmf_proto_bcdc_txdata; drvr->proto->configure_addr_mode = brcmf_proto_bcdc_configure_addr_mode; drvr->proto->delete_peer = brcmf_proto_bcdc_delete_peer; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 1715280..6d046ba 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -239,7 +239,13 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb, if (eh->h_proto == htons(ETH_P_PAE)) atomic_inc(&ifp->pend_8021x_cnt); - ret = brcmf_fws_process_skb(ifp, skb); + /* determine the priority */ + if ((skb->priority == 0) || (skb->priority > 7)) + skb->priority = cfg80211_classify8021d(skb, NULL); + + ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb); + if (ret < 0) + brcmf_txfinalize(ifp, skb, false); done: if (ret) { diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c index a190f53..5f1a592 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c @@ -2100,16 +2100,6 @@ int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb) int rc = 0; brcmf_dbg(DATA, "tx proto=0x%X\n", ntohs(eh->h_proto)); - /* determine the priority */ - if ((skb->priority == 0) || (skb->priority > 7)) - skb->priority = cfg80211_classify8021d(skb, NULL); - - if (fws->avoid_queueing) { - rc = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb); - if (rc < 0) - brcmf_txfinalize(ifp, skb, false); - return rc; - } /* set control buffer information */ skcb->if_flags = 0; @@ -2442,6 +2432,11 @@ void brcmf_fws_deinit(struct brcmf_pub *drvr) kfree(fws); } +bool brcmf_fws_queue_skbs(struct brcmf_fws_info *fws) +{ + ret
[PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Rafał Miłecki Flowrings contain skbs waiting for transmission that were passed to us by netif. It means we checked every one of them looking for 802.1x Ethernet type. When deleting flowring we have to use freeing function that will check for 802.1x type as well. Freeing skbs without a proper check was leading to counter not being properly decreased. This was triggering a WARNING every time brcmf_netdev_wait_pend8021x was called. Signed-off-by: Rafał Miłecki --- Kalle: this isn't important enough for 4.8 as it's too late for that. I'd like to get it for 4.9 however, as this fixes bug that could lead to WARNING on every add_key/del_key call. We was struggling with these WARNINGs for some time and this fixes one of two problems causing them. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c index b16b367..d0b738d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c @@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid, void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid) { + struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev); struct brcmf_flowring_ring *ring; + struct brcmf_if *ifp; u16 hash_idx; + u8 ifidx; struct sk_buff *skb; ring = flow->rings[flowid]; if (!ring) return; + + ifidx = brcmf_flowring_ifidx_get(flow, flowid); + ifp = brcmf_get_ifp(bus_if->drvr, ifidx); + brcmf_flowring_block(flow, flowid, false); hash_idx = ring->hash_id; flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX; @@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid) skb = skb_dequeue(&ring->skblist); while (skb) { - brcmu_pkt_buf_free_skb(skb); + brcmf_txfinalize(ifp, skb, false); skb = skb_dequeue(&ring->skblist); } -- 2.9.3
[PATCH] brcmfmac: replace WARNING on timeout with a simple error message
From: Rafał Miłecki Even with timeout increased to 950 ms we get WARNINGs from time to time. It mostly happens on A-MPDU stalls (e.g. when station goes out of range). It may take up to 5-10 secods for the firmware to recover and for that time it doesn't process packets. It's still useful to have a message on time out as it may indicate some firmware problem and incorrect key update. Raising a WARNING however wasn't really that necessary, it doesn't point to any driver bug anymore and backtrace wasn't much useful. Signed-off-by: Rafał Miłecki --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 6d046ba..9e6f60a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1161,7 +1161,8 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp) !brcmf_get_pend_8021x_cnt(ifp), MAX_WAIT_FOR_8021X_TX); - WARN_ON(!err); + if (!err) + brcmf_err("Timed out waiting for no pending 802.1x packets\n"); return !err; } -- 2.9.3
Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
On 27 September 2016 at 13:27, Kalle Valo wrote: > Arend Van Spriel writes: > >> On 27-9-2016 11:14, Rafał Miłecki wrote: >>> From: Rafał Miłecki >>> >>> Flowrings contain skbs waiting for transmission that were passed to us >>> by netif. It means we checked every one of them looking for 802.1x >>> Ethernet type. When deleting flowring we have to use freeing function >>> that will check for 802.1x type as well. >>> >>> Freeing skbs without a proper check was leading to counter not being >>> properly decreased. This was triggering a WARNING every time >>> brcmf_netdev_wait_pend8021x was called. >> >> Acked-by: Arend van Spriel >>> Signed-off-by: Rafał Miłecki >>> --- >>> Kalle: this isn't important enough for 4.8 as it's too late for that. >>> >>> I'd like to get it for 4.9 however, as this fixes bug that could lead >>> to WARNING on every add_key/del_key call. We was struggling with these >>> WARNINGs for some time and this fixes one of two problems causing them. > > Ok, I'll queue this for 4.9. > >> Please mark it for stable as well. > > I can add that. Any ideas how old releases stable releases should this > go to? I was analyzing this. 1) This patch uses brcmf_get_ifp which is available in 4.4+ only. 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac: Increase nr of supported flowrings.") 3) 4.4 would also require applying to the patch without broadcom/ subdir That said I suggest 4.5+. Any objections? -- Rafał
Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
On 27 September 2016 at 13:44, Rafał Miłecki wrote: > On 27 September 2016 at 13:27, Kalle Valo wrote: >> Arend Van Spriel writes: >> >>> On 27-9-2016 11:14, Rafał Miłecki wrote: >>>> From: Rafał Miłecki >>>> >>>> Flowrings contain skbs waiting for transmission that were passed to us >>>> by netif. It means we checked every one of them looking for 802.1x >>>> Ethernet type. When deleting flowring we have to use freeing function >>>> that will check for 802.1x type as well. >>>> >>>> Freeing skbs without a proper check was leading to counter not being >>>> properly decreased. This was triggering a WARNING every time >>>> brcmf_netdev_wait_pend8021x was called. >>> >>> Acked-by: Arend van Spriel >>>> Signed-off-by: Rafał Miłecki >>>> --- >>>> Kalle: this isn't important enough for 4.8 as it's too late for that. >>>> >>>> I'd like to get it for 4.9 however, as this fixes bug that could lead >>>> to WARNING on every add_key/del_key call. We was struggling with these >>>> WARNINGs for some time and this fixes one of two problems causing them. >> >> Ok, I'll queue this for 4.9. >> >>> Please mark it for stable as well. >> >> I can add that. Any ideas how old releases stable releases should this >> go to? > > I was analyzing this. > 1) This patch uses brcmf_get_ifp which is available in 4.4+ only. > 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac: > Increase nr of supported flowrings.") > 3) 4.4 would also require applying to the patch without broadcom/ subdir > > That said I suggest 4.5+. Any objections? Let me see if patchwork with pick Cc tag as it does for others. Cc: sta...@vger.kernel.org # 4.5+ This may be worth backporting to 4.4 as well (as it's longterm), but I'll do it separately due to patch not applying cleanly. -- Rafał
Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
On 27 September 2016 at 14:04, Arend Van Spriel wrote: > On 27-9-2016 13:58, Rafał Miłecki wrote: >> On 27 September 2016 at 13:44, Rafał Miłecki wrote: >>> On 27 September 2016 at 13:27, Kalle Valo wrote: >>>> Arend Van Spriel writes: >>>> >>>>> On 27-9-2016 11:14, Rafał Miłecki wrote: >>>>>> From: Rafał Miłecki >>>>>> >>>>>> Flowrings contain skbs waiting for transmission that were passed to us >>>>>> by netif. It means we checked every one of them looking for 802.1x >>>>>> Ethernet type. When deleting flowring we have to use freeing function >>>>>> that will check for 802.1x type as well. >>>>>> >>>>>> Freeing skbs without a proper check was leading to counter not being >>>>>> properly decreased. This was triggering a WARNING every time >>>>>> brcmf_netdev_wait_pend8021x was called. >>>>> >>>>> Acked-by: Arend van Spriel >>>>>> Signed-off-by: Rafał Miłecki >>>>>> --- >>>>>> Kalle: this isn't important enough for 4.8 as it's too late for that. >>>>>> >>>>>> I'd like to get it for 4.9 however, as this fixes bug that could lead >>>>>> to WARNING on every add_key/del_key call. We was struggling with these >>>>>> WARNINGs for some time and this fixes one of two problems causing them. >>>> >>>> Ok, I'll queue this for 4.9. >>>> >>>>> Please mark it for stable as well. >>>> >>>> I can add that. Any ideas how old releases stable releases should this >>>> go to? >>> >>> I was analyzing this. >>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only. >>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac: >>> Increase nr of supported flowrings.") >>> 3) 4.4 would also require applying to the patch without broadcom/ subdir >>> >>> That said I suggest 4.5+. Any objections? > > No objections. Just a tip. I tend to look at kernel.org main page to see > the stable and long-term kernel listed. So 4.7+ and 4.5+ have same > meaning as 4.5 and 4.6 are not stable/long-term kernels. Some projects may work on their own stable kernels, e.g. Ubuntu, see: https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable That's why I don't always look strictly at upstream stable releases only. -- Rafał
[PATCH V2 4.9] brcmfmac: use correct skb freeing helper when deleting flowring
From: Rafał Miłecki Flowrings contain skbs waiting for transmission that were passed to us by netif. It means we checked every one of them looking for 802.1x Ethernet type. When deleting flowring we have to use freeing function that will check for 802.1x type as well. Freeing skbs without a proper check was leading to counter not being properly decreased. This was triggering a WARNING every time brcmf_netdev_wait_pend8021x was called. Signed-off-by: Rafał Miłecki Acked-by: Arend van Spriel Cc: sta...@vger.kernel.org # 4.5+ --- V2: Add Cc for stable 4.5+. It doesn't apply cleanly to 4.4 and is not possible for 4.3- due to missing brcmf_get_ifp. --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c index b16b367..d0b738d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c @@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid, void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid) { + struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev); struct brcmf_flowring_ring *ring; + struct brcmf_if *ifp; u16 hash_idx; + u8 ifidx; struct sk_buff *skb; ring = flow->rings[flowid]; if (!ring) return; + + ifidx = brcmf_flowring_ifidx_get(flow, flowid); + ifp = brcmf_get_ifp(bus_if->drvr, ifidx); + brcmf_flowring_block(flow, flowid, false); hash_idx = ring->hash_id; flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX; @@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid) skb = skb_dequeue(&ring->skblist); while (skb) { - brcmu_pkt_buf_free_skb(skb); + brcmf_txfinalize(ifp, skb, false); skb = skb_dequeue(&ring->skblist); } -- 2.9.3
[PATCH] ARM: BCM5301X: Add basic dts for BCM53573 based Tenda AC9
BCM53573 seems to be low priced alternative for standard Northstar chipsets. It uses single core Cortex-A7, doesn't have SDU or local (TWD) timer. It was also stripped out of independent SPI controller and 2 GMACs. DTS for Tenda AC9 isn't completed yet. It misses e.g. switch entry (we still need some bgmac/b53 fixes) and probably some clocks. It adds support for basic features however and can be improved later. Signed-off-by: Rafał Miłecki --- MAINTAINERS | 2 + arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/bcm47189-tenda-ac9.dts | 74 arch/arm/boot/dts/bcm53573.dtsi | 147 +++ 4 files changed, 224 insertions(+) create mode 100644 arch/arm/boot/dts/bcm47189-tenda-ac9.dts create mode 100644 arch/arm/boot/dts/bcm53573.dtsi diff --git a/MAINTAINERS b/MAINTAINERS index 7304d2e..eceff03 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2516,6 +2516,8 @@ S:Maintained F: arch/arm/mach-bcm/bcm_5301x.c F: arch/arm/boot/dts/bcm5301x.dtsi F: arch/arm/boot/dts/bcm470* +F: arch/arm/boot/dts/bcm53573* +F: arch/arm/boot/dts/bcm47189* BROADCOM BCM63XX ARM ARCHITECTURE M: Florian Fainelli diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 973b0da..f7ce377 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -83,6 +83,7 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \ bcm4709-netgear-r7000.dtb \ bcm4709-netgear-r8000.dtb \ bcm47094-dlink-dir-885l.dtb \ + bcm47189-tenda-ac9.dtb \ bcm94708.dtb \ bcm94709.dtb \ bcm953012er.dtb \ diff --git a/arch/arm/boot/dts/bcm47189-tenda-ac9.dts b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts new file mode 100644 index 000..4403ae8 --- /dev/null +++ b/arch/arm/boot/dts/bcm47189-tenda-ac9.dts @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * Licensed under the ISC license. + */ + +/dts-v1/; + +#include "bcm53573.dtsi" + +/ { + compatible = "tenda,ac9", "brcm,bcm47189", "brcm,bcm53573"; + model = "Tenda AC9"; + + chosen { + bootargs = "console=ttyS0,115200 earlycon"; + }; + + memory { + reg = <0x 0x0800>; + }; + + leds { + compatible = "gpio-leds"; + + usb { + label = "bcm53xx:blue:usb"; + gpios = <&chipcommon 1 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "default-off"; + }; + + wps { + label = "bcm53xx:blue:wps"; + gpios = <&chipcommon 10 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "default-off"; + }; + + 5ghz { + label = "bcm53xx:blue:5ghz"; + gpios = <&chipcommon 11 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "default-off"; + }; + + system { + label = "bcm53xx:blue:system"; + gpios = <&chipcommon 15 GPIO_ACTIVE_HIGH>; + linux,default-trigger = "timer"; + }; + }; + + gpio-keys { + compatible = "gpio-keys"; + #address-cells = <1>; + #size-cells = <0>; + + rfkill { + label = "WiFi"; + linux,code = ; + gpios = <&chipcommon 3 GPIO_ACTIVE_LOW>; + }; + + restart { + label = "Reset"; + linux,code = ; + gpios = <&chipcommon 7 GPIO_ACTIVE_LOW>; + }; + + wps { + label = "WPS"; + linux,code = ; + gpios = <&chipcommon 9 GPIO_ACTIVE_LOW>; + }; + }; +}; diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi new file mode 100644 index 000..efa07de --- /dev/null +++ b/arch/arm/boot/dts/bcm53573.dtsi @@ -0,0 +1,147 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * Licensed under the ISC license. + */ + +#include +#include +#include +#include +#include "skeleton.dtsi" + +/ { + interrupt-parent = <&gic>; + + chosen { + stdout-path = &uart0; + }; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + device_type = "cpu"; + compati
Re: [PATCH V2] leds: trigger: Introduce an USB port trigger
HI Rob, I got problems following your objections, so it took me some time to go back to this. On 21 July 2016 at 22:42, Rob Herring wrote: > On Wed, Jul 20, 2016 at 10:06:23AM +0200, Rafał Miłecki wrote: >> On 20 July 2016 at 03:02, Rob Herring wrote: >> > On Fri, Jul 15, 2016 at 11:10:45PM +0200, Rafał Miłecki wrote: >> >> This commit adds a new trigger that can turn on LED when USB device gets >> >> connected to the USB port. This can be useful for various home routers >> >> that have USB port and a proper LED telling user a device is connected. >> >> >> >> Right now this trigger is usable with a proper DT only, there isn't a >> >> way to specify USB ports from user space. This may change in a future. >> >> >> >> Signed-off-by: Rafał Miłecki >> >> --- >> >> V2: The first version got support for specifying list of USB ports from >> >> user space only. There was a (big try &) discussion on adding DT >> >> support. It led to a pretty simple solution of comparing of_node of >> >> usb_device to of_node specified in usb-ports property. >> >> Since it appeared DT support may be simpler and non-DT a bit more >> >> complex, this version drops previous support for "ports" and >> >> "new_port" and focuses on DT only. The plan is to see if this >> >> solution with DT is OK, get it accepted and then work on non-DT. >> >> >> >> Felipe: if there won't be any objections I'd like to ask for your Ack. >> >> --- >> >> Documentation/devicetree/bindings/leds/common.txt | 11 ++ >> >> Documentation/leds/ledtrig-usbport.txt| 19 ++ >> >> drivers/leds/trigger/Kconfig | 8 + >> >> drivers/leds/trigger/Makefile | 1 + >> >> drivers/leds/trigger/ledtrig-usbport.c| 206 >> >> ++ >> >> 5 files changed, 245 insertions(+) >> >> create mode 100644 Documentation/leds/ledtrig-usbport.txt >> >> create mode 100644 drivers/leds/trigger/ledtrig-usbport.c >> >> >> >> diff --git a/Documentation/devicetree/bindings/leds/common.txt >> >> b/Documentation/devicetree/bindings/leds/common.txt >> >> index af10678..75536f7 100644 >> >> --- a/Documentation/devicetree/bindings/leds/common.txt >> >> +++ b/Documentation/devicetree/bindings/leds/common.txt >> >> @@ -50,6 +50,12 @@ property can be omitted. >> >> For controllers that have no configurable timeout the >> >> flash-max-timeout-us >> >> property can be omitted. >> >> >> >> +Trigger specific properties for child nodes: >> >> + >> >> +usbport trigger: >> >> +- usb-ports : List of USB ports that usbport should observed for turning >> >> on a >> >> + given LED. >> > >> > I think this should be more generic such that it could work for disk or >> > network LEDs too. Perhaps 'led-triggers = '? trigger is a bit of >> > a Linux name, but I haven't thought of anything better. Really, I'd >> > prefer the link in the other direction (e.g. port node have a 'leds" or >> > '*-leds' property), but that's maybe harder to parse. >> >> I was already thinking on this as I plan to add "netdev" trigger at >> some point in the future. I'd like to use "net-devices" for it (as a >> equivalent or "usb-ports"). >> >> However there is a problem with your idea of sharing "led-triggers" >> between triggers.. Consider device with generic purpose LED that >> should be controllable by a user. I believe we should let user switch >> it's state, e.g. with: >> echo usbport > trigger >> echo netdev > trigger >> with a shared property I don't see how we couldn't handle it properly. > > Well, the userspace interface and DT bindings are independent things. > You can't argue for what the DT binding should look like based on the > userspace interface. I don't understand that. It sounds like you don't want user to have control over a LED that was specified in DT. If I got it right, it sounds like a bad idea to me. It's a known case (in marketing, usage model) to allow user disable all LEDs (e.g. to don't disturb him during the night). That sounds like a valid usage for me, so I want user to be able to switch between triggers. And
[PATCH] clocksource: arm_arch_timer: Support reading clock rate from a driver
From: Rafał Miłecki On some devices using arch code for reading clock rate doesn't work. So far the only option was to specify clock-frequency in a DT. This works only if a clock frequency doesn't have to be calculated on runtime. On BCM53573 SoC (with Cortex-A7) there is ILP clock that needs its own driver. With this change we can query such clocks by using a standard: clocks = <&foo>; Signed-off-by: Rafał Miłecki --- drivers/clocksource/arm_arch_timer.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 5effd30..5ed98a2 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -405,6 +406,16 @@ arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np) arch_timer_rate = arch_timer_get_cntfrq(); } + /* Get clk rate through clk driver if present */ + if (!arch_timer_rate) { + struct clk *clk = of_clk_get(np, 0); + + if (!IS_ERR(clk)) { + if (!clk_prepare_enable(clk)) + arch_timer_rate = clk_get_rate(clk); + } + } + /* Check the timer frequency. */ if (arch_timer_rate == 0) pr_warn("Architected timer frequency not available\n"); -- 1.8.4.5
[PATCH] clk: bcm: Add driver for Northstar ILP clock
From: Rafał Miłecki This clock is present on cheaper Northstar devices like BCM53573 or BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) to calculate clock rate and allows using it in a generic (clk_*) way. Signed-off-by: Rafał Miłecki --- .../devicetree/bindings/clock/brcm,ns-ilp.txt | 28 drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-ns-ilp.c | 146 + 3 files changed, 175 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt create mode 100644 drivers/clk/bcm/clk-ns-ilp.c diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt new file mode 100644 index 000..c4df38e --- /dev/null +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt @@ -0,0 +1,28 @@ +Broadcom Northstar ILP clock + + +This binding uses the common clock binding: +Documentation/devicetree/bindings/clock/clock-bindings.txt + +This binding is used for ILP clock on Broadcom Northstar devices using +Corex-A7 CPU. ILP clock depends on ALP one and has to be calculated on +runtime. + +Required properties: +- compatible: "brcm,ns-ilp" +- reg: iomem address range of PMU (Power Management Unit) +- reg-names: "pmu", the only needed & supported reg right now +- clocks: should reference an ALP clock +- clock-names: "alp", the only needed & supported clock right now +- #clock-cells: should be <0> + +Example: + +ilp: ilp { + compatible = "brcm,ns-ilp"; + reg = <0x18012000 0x1000>; + reg-names = "pmu"; + clocks = <&alp>; + clock-names = "alp-clk"; + #clock-cells = <0>; +}; diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 1d79bd2..1389379 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC)+= clk-ns2.o obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o +obj-$(CONFIG_ARCH_BCM_5301X) += clk-ns-ilp.o diff --git a/drivers/clk/bcm/clk-ns-ilp.c b/drivers/clk/bcm/clk-ns-ilp.c new file mode 100644 index 000..230458e8 --- /dev/null +++ b/drivers/clk/bcm/clk-ns-ilp.c @@ -0,0 +1,146 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PMU_XTAL_FREQ_RATIO0x66c +#define XTAL_ALP_PER_4ILP 0x1fff +#define XTAL_CTL_EN 0x8000 +#define PMU_SLOW_CLK_PERIOD0x6dc + +struct ns_ilp { + struct clk *clk; + struct clk_hw hw; + struct clk *alp_clk; + void __iomem *pmu; +}; + +static int ns_ilp_enable(struct clk_hw *hw) +{ + struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw); + + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD); + writel(0x1, ilp->pmu + 0x674); + + return 0; +} + +static unsigned long ns_ilp_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw); + void __iomem *pmu = ilp->pmu; + u32 last_val, cur_val; + u32 sum = 0, num = 0, loop_num = 0; + u32 avg; + int err; + + err = clk_prepare_enable(ilp->alp_clk); + if (err) + return 0; + + /* Enable */ + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO); + + /* Read initial value */ + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; + + /* Try getting 20 different values for calculating average */ + while (num < 20) { + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; + + if (cur_val != last_val) { + /* Got different value, use it */ + sum += cur_val; + num++; + loop_num = 0; + last_val = cur_val; + } else if (++loop_num > 5000) { + /* Same value over and over, give up */ + sum += cur_val; + num++; + break; + } + } + + /* Disable */ + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO); + + avg = sum / num; + + return clk_get_rate(ilp->alp_clk) * 4 / avg; +} + +const struct clk_ops ns_ilp_clk_ops = { + .enable = ns_ilp_enable, + .recalc_rate = ns_ilp_recalc_rate, +}; + +static void
Re: [PATCH] clk: bcm: Add driver for Northstar ILP clock
On 29 July 2016 at 22:44, Ray Jui wrote: > On 7/29/2016 5:58 AM, Rafał Miłecki wrote: >> >> From: Rafał Miłecki >> >> This clock is present on cheaper Northstar devices like BCM53573 or >> BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) >> to calculate clock rate and allows using it in a generic (clk_*) way. >> > > I thought Northstar uses Cortex A9 instead of A7? [0.00] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine model: Tenda AC9 -- Rafał
Re: [PATCH] clk: bcm: Add driver for Northstar ILP clock
On 29 July 2016 at 22:49, Ray Jui wrote: > On 7/29/2016 1:46 PM, Rafał Miłecki wrote: >> On 29 July 2016 at 22:44, Ray Jui wrote: >>> >>> On 7/29/2016 5:58 AM, Rafał Miłecki wrote: >>>> >>>> >>>> From: Rafał Miłecki >>>> >>>> This clock is present on cheaper Northstar devices like BCM53573 or >>>> BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) >>>> to calculate clock rate and allows using it in a generic (clk_*) way. >>>> >>> >>> I thought Northstar uses Cortex A9 instead of A7? >> >> >> [0.00] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), >> cr=10c5387d >> [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing >> instruction cache >> [0.00] Machine model: Tenda AC9 >> > > Yeah ARMv7 instruction set but the core is Cortex A7. Both Cortex A7 and A9 > use ARMv7 instructions. OK, sorry for irrelevant part then :) This is from BCM4709C0: bcma: bus0: Core 10 found: ARM Cortex A9 core (ihost) (manuf 0x4BF, id 0x510, rev 0x07, class 0x0) This is from BCM47189B0:: bcma: bus0: Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class 0x0) -- Rafał
Re: [PATCH] clk: bcm: Add driver for Northstar ILP clock
On 29 July 2016 at 22:55, Florian Fainelli wrote: > On 07/29/2016 01:52 PM, Rafał Miłecki wrote: >> On 29 July 2016 at 22:49, Ray Jui wrote: >>> On 7/29/2016 1:46 PM, Rafał Miłecki wrote: >>>> On 29 July 2016 at 22:44, Ray Jui wrote: >>>>> >>>>> On 7/29/2016 5:58 AM, Rafał Miłecki wrote: >>>>>> >>>>>> >>>>>> From: Rafał Miłecki >>>>>> >>>>>> This clock is present on cheaper Northstar devices like BCM53573 or >>>>>> BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) >>>>>> to calculate clock rate and allows using it in a generic (clk_*) way. >>>>>> >>>>> >>>>> I thought Northstar uses Cortex A9 instead of A7? >>>> >>>> >>>> [0.00] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), >>>> cr=10c5387d >>>> [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing >>>> instruction cache >>>> [0.00] Machine model: Tenda AC9 >>>> >>> >>> Yeah ARMv7 instruction set but the core is Cortex A7. Both Cortex A7 and A9 >>> use ARMv7 instructions. >> >> OK, sorry for irrelevant part then :) >> >> This is from BCM4709C0: >> bcma: bus0: Core 10 found: ARM Cortex A9 core (ihost) (manuf 0x4BF, id >> 0x510, rev 0x07, class 0x0) >> >> This is from BCM47189B0:: >> bcma: bus0: Core 3 found: ARM CA7 (manuf 0x4BF, id 0x847, rev 0x00, class >> 0x0) >> > > This is indeed a Cortex A7-based chip, not clear if putting this chip in > the Northstar family is accurate here because it really seems to have a > different architecture from the NS/NSP family here... Broadcom claims it is a Northstar, at least according to their SDKs: Asus RT-AC1200G+ # cat /proc/cpuinfo Processor : ARMv7 Processor rev 5 (v7l) BogoMIPS : 1795.68 Features : swp half thumb fastmult edsp CPU implementer : 0x41 CPU architecture: 7 CPU variant : 0x0 CPU part : 0xc07 CPU revision : 5 Hardware : Northstar Prototype Revision : Serial : -- Rafał
Re: [PATCH] clk: bcm: Add driver for Northstar ILP clock
On 29 July 2016 at 15:15, Mark Rutland wrote: > On Fri, Jul 29, 2016 at 02:58:32PM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> This clock is present on cheaper Northstar devices like BCM53573 or >> BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) >> to calculate clock rate and allows using it in a generic (clk_*) way. >> >> Signed-off-by: Rafał Miłecki >> --- >> .../devicetree/bindings/clock/brcm,ns-ilp.txt | 28 >> drivers/clk/bcm/Makefile | 1 + >> drivers/clk/bcm/clk-ns-ilp.c | 146 >> + >> 3 files changed, 175 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt >> create mode 100644 drivers/clk/bcm/clk-ns-ilp.c >> >> diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt >> b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt >> new file mode 100644 >> index 000..c4df38e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt >> @@ -0,0 +1,28 @@ >> +Broadcom Northstar ILP clock >> + >> + >> +This binding uses the common clock binding: >> +Documentation/devicetree/bindings/clock/clock-bindings.txt >> + >> +This binding is used for ILP clock on Broadcom Northstar devices using >> +Corex-A7 CPU. ILP clock depends on ALP one and has to be calculated on >> +runtime. >> + >> +Required properties: >> +- compatible: "brcm,ns-ilp" >> +- reg: iomem address range of PMU (Power Management Unit) >> +- reg-names: "pmu", the only needed & supported reg right now > > From the commit message and binding description, it sounds like there > should be a binding for the PMU, and that should cover the clocks > required/exported by the PMU. This is a bit of problem, because PMU handles a lot of different stuff and is used by various drivers. Some examples of what you can do with/find on a PMU: 1) Power management 2) Watchdog 3) Timer 4) XTAL 5) PLLs 6) Control registers for some ARM debugging (whatever it is), UART, JTAG, more PMU is used by different drivers, e.g.: 1) Ethernet driver 2) Wireless driver 3) NAND controller driver I don't have access to Broadcom's datasheets so unfortunately I can't provide all details. >> +- clocks: should reference an ALP clock >> +- clock-names: "alp", the only needed & supported clock right now >> +- #clock-cells: should be <0> > > How many clocks does the PMU output, including the ILP clock? Well, ALP clock (AKA XTAL clock) is definitely part of PMU. It's a fixed rate clock with rate specific to the chip. I think ILP is also part of PMU (again: I don't have datasheets) as PMU has this ALP_PER_4ILP register. >From Broadcom's SDK I can say they also have "ARM debug unit" on some chipsets. It requires enabling "ARM debug clk" to operate which is handled by PMU as well. -- Rafał
[PATCH V2] clk: bcm: Add driver for Northstar ILP clock
From: Rafał Miłecki This clock is present on cheaper Northstar devices like BCM53573 or BCM47189 using Corex-A7. This driver uses PMU (Power Management Unit) to calculate clock rate and allows using it in a generic (clk_*) way. Signed-off-by: Rafał Miłecki --- V2: Rebase on top of clk-next Use ALP as parent clock Improve comments Switch from ioremap_nocache to ioremap Check of_clk_add_provide result for error --- .../devicetree/bindings/clock/brcm,ns-ilp.txt | 26 drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-ns-ilp.c | 147 + 3 files changed, 174 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt create mode 100644 drivers/clk/bcm/clk-ns-ilp.c diff --git a/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt new file mode 100644 index 000..2c862a0 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/brcm,ns-ilp.txt @@ -0,0 +1,26 @@ +Broadcom Northstar ILP clock + + +This binding uses the common clock binding: +Documentation/devicetree/bindings/clock/clock-bindings.txt + +This binding is used for ILP clock on Broadcom Northstar devices using +Corex-A7 CPU. ILP clock depends on ALP one and has to be calculated on +runtime. + +Required properties: +- compatible: "brcm,ns-ilp" +- reg: iomem address range of PMU (Power Management Unit) +- reg-names: "pmu", the only needed & supported reg right now +- clocks: has to reference an ALP clock +- #clock-cells: should be <0> + +Example: + +ilp: ilp { + compatible = "brcm,ns-ilp"; + reg = <0x18012000 0x1000>; + reg-names = "pmu"; + clocks = <&alp>; + #clock-cells = <0>; +}; diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 1d79bd2..1389379 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC)+= clk-ns2.o obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o +obj-$(CONFIG_ARCH_BCM_5301X) += clk-ns-ilp.o diff --git a/drivers/clk/bcm/clk-ns-ilp.c b/drivers/clk/bcm/clk-ns-ilp.c new file mode 100644 index 000..0337313 --- /dev/null +++ b/drivers/clk/bcm/clk-ns-ilp.c @@ -0,0 +1,147 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PMU_XTAL_FREQ_RATIO0x66c +#define XTAL_ALP_PER_4ILP 0x1fff +#define XTAL_CTL_EN 0x8000 +#define PMU_SLOW_CLK_PERIOD0x6dc + +struct ns_ilp { + struct clk *clk; + struct clk_hw hw; + void __iomem *pmu; +}; + +static int ns_ilp_enable(struct clk_hw *hw) +{ + struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw); + + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD); + writel(0x1, ilp->pmu + 0x674); + + return 0; +} + +static unsigned long ns_ilp_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct ns_ilp *ilp = container_of(hw, struct ns_ilp, hw); + void __iomem *pmu = ilp->pmu; + u32 last_val, cur_val; + u32 sum = 0, num = 0, loop_num = 0; + u32 avg; + + /* Enable measurement */ + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO); + + /* Read initial value */ + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; + + /* +* At minimum we should loop for a bit to let hardware do the +* measurement. This isn't very accurate however, so for a better +* precision lets try getting 20 different values for and use average. +*/ + while (num < 20) { + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; + + if (cur_val != last_val) { + /* Got different value, use it */ + sum += cur_val; + num++; + loop_num = 0; + last_val = cur_val; + } else if (++loop_num > 5000) { + /* Same value over and over, give up */ + sum += cur_val; + num++; + break; + } + } + + /* Disable measurement to save power */ + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO); + + avg = sum / num; + + return parent_rate * 4 / avg; +} + +static const struct c
[PATCH V5] leds: trigger: Introduce a USB port trigger
From: Rafał Miłecki This commit adds a new trigger responsible for turning on LED when USB device gets connected to the selected USB port. This can can useful for various home routers that have USB port(s) and a proper LED telling user a device is connected. The trigger gets its documentation file but basically it just requires enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1). There was a long discussion on design of this driver. Its current state is a result of picking them most adjustable solution as others couldn't handle all cases. 1) It wasn't possible for the driver to register separated trigger for each USB port. Some physical USB ports are handled by more than one controller and so by more than one USB port. E.g. USB 2.0 physical port may be handled by OHCI's port and EHCI's port. It's also not possible to assign more than 1 trigger to a single LED and implementing such feature would be tricky due to syncing triggers and sysfs conflicts with old triggers. 2) Another idea was to register trigger per USB hub. This wouldn't allow handling devices with multiple USB LEDs and controllers (hubs) controlling more than 1 physical port. It's common for hubs to have few ports and each may have its own LED. This final trigger is highly flexible. It allows selecting any USB ports for any LED. It was also modified (compared to the initial version) to allow choosing ports rather than having user /guess/ proper names. It was successfully tested on SmartRG SR400ac which has 3 USB LEDs, 2 physical ports and 3 controllers. Another planned feature is support for LED reacting to the USB activity. This can be implemented with another sysfs file for setting mode. The default mode wouldn't change so there won't be ABI breakage and such feature can be safely implemented later. There was also an idea of supporting other devices (PCI, SDIO, etc.) but as this driver already contains some USB specific code (and will get more) these should be probably separated drivers (triggers). Signed-off-by: Rafał Miłecki --- V2: Trying to add DT support, idea postponed as it will take more time to discuss the bindings. V3: Fix typos in commit and Documentation (thanks Jacek!) Use "ports" sysfs file for adding and removing USB ports (thx Jacek) Check if there is USB device connected after adding new USB port Fix memory leak or two V3.5: Fix e-mail address (thanks Matthias) Simplify conditions in usbport_trig_notify (thx Matthias) Make "ports" a subdirectory with file per port, to match one value per file sysfs rule (thanks Greg) As "ports" couldn't be used for adding and removing ports anymore, there are now "new_port" and "remove_port". Having them makes this API also common with e.g. pci and usb buses. V4: Add Documentation/ABI/testing/sysfs-class-led-trigger-usbport and reference it in Documentation/leds/ledtrig-usbport.txt to avoid doc duplication. V5: Update commit message to explain driver design & Documentation Avoid specifying ports manually and dummy files (chmod ) Don't use kobject_create_and_add but a struct attribute_group Fix typo, improve comments, update Date --- .../ABI/testing/sysfs-class-led-trigger-usbport| 11 + Documentation/leds/ledtrig-usbport.txt | 41 +++ drivers/leds/trigger/Kconfig | 8 + drivers/leds/trigger/Makefile | 1 + drivers/leds/trigger/ledtrig-usbport.c | 316 + 5 files changed, 377 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-usbport create mode 100644 Documentation/leds/ledtrig-usbport.txt create mode 100644 drivers/leds/trigger/ledtrig-usbport.c diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-usbport b/Documentation/ABI/testing/sysfs-class-led-trigger-usbport new file mode 100644 index 000..a9b7e92 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-usbport @@ -0,0 +1,11 @@ +What: /sys/class/leds//ports/ +Date: September 2016 +KernelVersion: 4.9 +Contact: linux-l...@vger.kernel.org +Description: + Every dir entry represents a single USB port that can be + selected for the USB port trigger. Selecting ports makes trigger + observing them for any connected devices and lighting on LED if + there are any. + Echoing "1" value selects USB port. Echoing "0" unselects it. + Current state can be also read. diff --git a/Documentation/leds/ledtrig-usbport.txt b/Documentation/leds/ledtrig-usbport.txt new file mode 100644 index 000..69f54bf --- /dev/null +++ b/Documentation/leds/ledtrig-usbport.txt @@ -0,0 +1,41 @@ +USB port LED trigger + + +This LED t
Re: [PATCH V5] leds: trigger: Introduce a USB port trigger
On 9 September 2016 at 11:34, Peter Chen wrote: > On Thu, Sep 08, 2016 at 06:08:24PM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> This commit adds a new trigger responsible for turning on LED when USB >> device gets connected to the selected USB port. This can can useful for >> various home routers that have USB port(s) and a proper LED telling user >> a device is connected. >> >> The trigger gets its documentation file but basically it just requires >> enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1). >> >> There was a long discussion on design of this driver. Its current state >> is a result of picking them most adjustable solution as others couldn't >> handle all cases. >> >> 1) It wasn't possible for the driver to register separated trigger for >>each USB port. Some physical USB ports are handled by more than one >>controller and so by more than one USB port. E.g. USB 2.0 physical >>port may be handled by OHCI's port and EHCI's port. >>It's also not possible to assign more than 1 trigger to a single LED >>and implementing such feature would be tricky due to syncing triggers >>and sysfs conflicts with old triggers. >> >> 2) Another idea was to register trigger per USB hub. This wouldn't allow >>handling devices with multiple USB LEDs and controllers (hubs) >>controlling more than 1 physical port. It's common for hubs to have >>few ports and each may have its own LED. >> >> This final trigger is highly flexible. It allows selecting any USB ports >> for any LED. It was also modified (compared to the initial version) to >> allow choosing ports rather than having user /guess/ proper names. It >> was successfully tested on SmartRG SR400ac which has 3 USB LEDs, >> 2 physical ports and 3 controllers. >> >> Another planned feature is support for LED reacting to the USB activity. >> This can be implemented with another sysfs file for setting mode. The >> default mode wouldn't change so there won't be ABI breakage and such >> feature can be safely implemented later. >> > > It has such driver at: drivers/usb/common/led.c Oh, great, I had no idea about that. So if it comes to adding activity support, we'll have to well discuss that. We may e.g. not implement that or move existing feature into more generic usbport trigger or partially duplicate this feature. Anyway, I hope this note on "usb-gadget" and "usb-host" triggers won't stop our work on "usbport". This driver implements different thing for now (discovering device in a USB port) and I don't think "usb-gadget" / "usb-host" could be extended to handle such cases. Thanks for a hint though, I'll definitely keep that in mind during further development! -- Rafał
[PATCH V8] clk: bcm: Add driver for BCM53573 ILP clock
From: Rafał Miłecki This clock is present on BCM53573 devices (including BCM47189) that use Cortex-A7. ILP is a part of PMU (Power Management Unit) multi-function device so we use syscon (and regmap) for it. Signed-off-by: Rafał Miłecki --- V2: Rebase on top of clk-next Use ALP as parent clock Improve comments Switch from ioremap_nocache to ioremap Check of_clk_add_provide result for error V3: Drop #include Make ILP DT entry part of PMU Describe ILP as subdevice of PMU in Documentation V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar" (even if used in some resources) should be used in relation to Cortex-A9 devices only. V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too early. V6: Drop #include Use "int" as type where it matches usage Add cpu_relax() in the loop Add disable callback Use _hw_ functions for registering struct clk_hw (new API) Thanks a lot Stephen! V7: Use syscon and regmap (thanks Rob!) V8: Update Documentation (drop unused "reg", unit address) --- .../bindings/clock/brcm,bcm53573-ilp.txt | 36 + drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm53573-ilp.c | 148 + 3 files changed, 185 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt new file mode 100644 index 000..2ebb107 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt @@ -0,0 +1,36 @@ +Broadcom BCM53573 ILP clock +=== + +This binding uses the common clock binding: +Documentation/devicetree/bindings/clock/clock-bindings.txt + +This binding is used for ILP clock (sometimes referred as "slow clock") +on Broadcom BCM53573 devices using Cortex-A7 CPU. + +ILP's rate has to be calculated on runtime and it depends on ALP clock +which has to be referenced. + +This clock is part of PMU (Power Management Unit), a Broadcom's device +handing power-related aspects. Its node must be sub-node of the PMU +device. + +Required properties: +- compatible: "brcm,bcm53573-ilp" +- clocks: has to reference an ALP clock +- #clock-cells: should be <0> +- clock-output-names: from common clock bindings, should contain clock + name + +Example: + +pmu@18012000 { + compatible = "simple-mfd", "syscon"; + reg = <0x18012000 0x1000>; + + ilp { + compatible = "brcm,bcm53573-ilp"; + clocks = <&alp>; + #clock-cells = <0>; + clock-output-names = "ilp"; + }; +}; diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 1d79bd2..4b8c56d 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC)+= clk-ns2.o obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o +obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c new file mode 100644 index 000..1f4e30d --- /dev/null +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c @@ -0,0 +1,148 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define PMU_XTAL_FREQ_RATIO0x66c +#define XTAL_ALP_PER_4ILP 0x1fff +#define XTAL_CTL_EN 0x8000 +#define PMU_SLOW_CLK_PERIOD0x6dc + +struct bcm53573_ilp { + struct clk_hw hw; + struct regmap *regmap; +}; + +static int bcm53573_ilp_enable(struct clk_hw *hw) +{ + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); + + regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0x10199); + regmap_write(ilp->regmap, 0x674, 0x1); + + return 0; +} + +static void bcm53573_ilp_disable(struct clk_hw *hw) +{ + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); + + regmap_write(ilp->regmap, PMU_SLOW_CLK_PERIOD, 0); + regmap_write(ilp->regmap, 0x674, 0); +} + +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct bcm53573_ilp *ilp = container_of(hw, s
get_maintainer.pl and git send-email: 5.1.2 The recipient address is not a valid
Hi Joe, Some time ago I started using git send-email with --cc-cmd and get_maintainer.pl. I liked it and it worked fine until I tried to send ubifs patch patch. I got this: Send this email? ([y]es|[n]o|[q]uit|[a]ll): y 5.1.2 The recipient address is not a valid 5.1.2 RFC-5321 address. j11sm4705544lfe.27 - gsmtp I suspect there may be some problem with MAINTAINERS entries using braces. Could you take a look at following try? https://patchwork.ozlabs.org/patch/669853/ wget -O mtd.patch https://patchwork.ozlabs.org/patch/669853/mbox/ git send-email --cc-cmd="scripts/get_maintainer.pl" mtd.patch It results in: (cc-cmd) Adding cc: linux-...@lists.infradead.org (open list:UBI FILE SYSTEM (UBIFS)) from: 'scripts/get_maintainer.pl' and Cc-ing: linux-...@lists.infradead.org) (open list:UBI FILE SYSTEM (UBIFS) Is this something to fix at get_maintainer.pl level? -- Rafał
Re: [PATCH V5] leds: trigger: Introduce a USB port trigger
On 15 September 2016 at 14:56, Pavel Machek wrote: > On Fri 2016-09-09 13:31:10, Rafał Miłecki wrote: >> On 9 September 2016 at 13:05, Greg KH wrote: >> > On Fri, Sep 09, 2016 at 05:34:40PM +0800, Peter Chen wrote: >> >> On Thu, Sep 08, 2016 at 06:08:24PM +0200, Rafał Miłecki wrote: >> >> > From: Rafał Miłecki >> >> > >> >> > This commit adds a new trigger responsible for turning on LED when USB >> >> > device gets connected to the selected USB port. This can can useful for >> >> > various home routers that have USB port(s) and a proper LED telling user >> >> > a device is connected. >> >> > >> >> > The trigger gets its documentation file but basically it just requires >> >> > enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1). >> >> > >> >> > There was a long discussion on design of this driver. Its current state >> >> > is a result of picking them most adjustable solution as others couldn't >> >> > handle all cases. >> >> > >> >> > 1) It wasn't possible for the driver to register separated trigger for >> >> >each USB port. Some physical USB ports are handled by more than one >> >> >controller and so by more than one USB port. E.g. USB 2.0 physical >> >> >port may be handled by OHCI's port and EHCI's port. >> >> >It's also not possible to assign more than 1 trigger to a single LED >> >> >and implementing such feature would be tricky due to syncing triggers >> >> >and sysfs conflicts with old triggers. >> >> > >> >> > 2) Another idea was to register trigger per USB hub. This wouldn't allow >> >> >handling devices with multiple USB LEDs and controllers (hubs) >> >> >controlling more than 1 physical port. It's common for hubs to have >> >> >few ports and each may have its own LED. >> >> > >> >> > This final trigger is highly flexible. It allows selecting any USB ports >> >> > for any LED. It was also modified (compared to the initial version) to >> >> > allow choosing ports rather than having user /guess/ proper names. It >> >> > was successfully tested on SmartRG SR400ac which has 3 USB LEDs, >> >> > 2 physical ports and 3 controllers. >> >> > >> >> > Another planned feature is support for LED reacting to the USB activity. >> >> > This can be implemented with another sysfs file for setting mode. The >> >> > default mode wouldn't change so there won't be ABI breakage and such >> >> > feature can be safely implemented later. >> >> > >> >> >> >> It has such driver at: drivers/usb/common/led.c >> > >> > Ugh, I thought I had seen something like this before... >> > >> > Rafał, can you just use this in-kernel code instead? >> >> I really don't think I can because of all the reasons I carefully >> listed in the commit message. >> >> Have you took a look at that simple driver? It does nothing I need. >> Its design doesn't allow implementing features I clearly listed in the >> commit message. > > In any case, the new driver should probably go near the old one, at > the very least. I can do that. Anyone objects? -- Rafał
Re: [PATCH V4] leds: trigger: Introduce an USB port trigger
On 31 August 2016 at 20:23, Alan Stern wrote: > On Tue, 30 Aug 2016, Rafał Miłecki wrote: > >> >> As you quite often need more complex LED management, there are >> >> triggers that were introduced in 2006 by c3bc9956ec52f ("[PATCH] LED: >> >> add LED trigger tupport"). Some triggers are trivial and could be >> >> implemented in userspace as well (e.g. "timer"). Some had to be >> >> implemented in kernelspace (CPU activity, MTD activity, etc.). Having >> >> few triggers compiled, you can assign them to LEDs at it pleases you. >> >> Your hardware may have generic LED (not labeled) and you can >> >> dynamically assign various triggers to it, depending e.g. on user >> >> actions. E.g. if user (using GUI or whatever) wants to see flash >> >> activity, your userspace script should do: >> >> echo mtd > /sys/class/leds/foo/trigger >> > >> > So for example, you might want to do: >> > >> > echo usb1-4 >/sys/class/leds/foo/trigger >> > >> > and then have the "foo" LED toggle whenever an URB was submitted or >> > completed for a device attached to the 1-4 port. Right? >> >> Not really as it won't cover some pretty common use cases. Many home >> routers have few USB ports (2-5) and only 1 USB LED. It has to be >> possible to assign few USB ports to a single LED (trigger). That way >> LED should be turned on (and kept on) if there is at least 1 USB >> device connected. You obviously can't do: >> echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger >> >> This was already brought up by Rob (who mentioned CPU trigger) and I >> replied him pretty much the same way in: >> https://lkml.org/lkml/2016/7/29/38 >> (reply starts with "Anyway, the serious limitation I see"). > > The code for a bunch of triggers must already be written. What would > the user do if he wanted to flash a single LED in response to both > CPU activity and MTD activity? If not > > echo "cpu mtd" >/sys/class/leds/foo/trigger > > then what? Well, it sounds like a new feature then. Shall we add an extra API with a request function for turning LED on? It could internally count how many requests were raised and keep LED on as long as there is at least 1 left. I guess we should implement it in trigger "subsystem" (if I can call it so). Does it sound like a good plan? -- Rafał
Re: [PATCH V4] leds: trigger: Introduce an USB port trigger
On 31 August 2016 at 21:00, Rafał Miłecki wrote: > On 31 August 2016 at 20:23, Alan Stern wrote: >> On Tue, 30 Aug 2016, Rafał Miłecki wrote: >>> Not really as it won't cover some pretty common use cases. Many home >>> routers have few USB ports (2-5) and only 1 USB LED. It has to be >>> possible to assign few USB ports to a single LED (trigger). That way >>> LED should be turned on (and kept on) if there is at least 1 USB >>> device connected. You obviously can't do: >>> echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger >>> >>> This was already brought up by Rob (who mentioned CPU trigger) and I >>> replied him pretty much the same way in: >>> https://lkml.org/lkml/2016/7/29/38 >>> (reply starts with "Anyway, the serious limitation I see"). >> >> The code for a bunch of triggers must already be written. What would >> the user do if he wanted to flash a single LED in response to both >> CPU activity and MTD activity? If not >> >> echo "cpu mtd" >/sys/class/leds/foo/trigger >> >> then what? > > Well, it sounds like a new feature then. Shall we add an extra API > with a request function for turning LED on? It could internally count > how many requests were raised and keep LED on as long as there is at > least 1 left. I guess we should implement it in trigger "subsystem" > (if I can call it so). Does it sound like a good plan? I'm pretty sure noone ever planned to have more than 1 trigger assigned to a single LED. I just realized there will be a problem with proposed solution: sysfs files conflict. Consider 2 existing triggers for a moment: 1) oneshot: it creates following sysfs files: /sys/class/leds/foo/delay_on /sys/class/leds/foo/delay_off /sys/class/leds/foo/invert /sys/class/leds/foo/shot 2) timer: it creates following sysfs files: /sys/class/leds/foo/delay_on /sys/class/leds/foo/delay_off Activating both of them will probably cause a WARNING in sysfs. They can't coexist :( We should probably have per-trigger subdirs, e.g.: /sys/class/leds/foo/trigger-oneshot/delay_on /sys/class/leds/foo/trigger-oneshot/delay_off /sys/class/leds/foo/trigger-oneshot/invert /sys/class/leds/foo/trigger-oneshot/shot /sys/class/leds/foo/trigger-timer/delay_on /sys/class/leds/foo/trigger-timer/delay_off but implementing it now would break the ABI. One workaround I can see is doing triggers V2, they: 1) Would put sysfs files in /sys/class/leds/foo/trigger-bar/ 2) Use new API for *requesting* LED to be on/off 3) There would be a counter of requests in V2 API 4) Multiple triggers V2 would be allowed to be used (assigned) at the same time -- Rafał
[PATCH V3] leds: trigger: Introduce an USB port trigger
From: Rafał Miłecki This commit adds a new trigger responsible for turning on LED when USB device gets connected to the specified USB port. This can can useful for various home routers that have USB port(s) and a proper LED telling user a device is connected. The trigger gets its documentation file but basically it just requires specifying USB port in a Linux format (e.g. echo 1-1 > new_port). During work on this trigger there was a plan to add DT bindings for it, but there wasn't an agreement on the format yet. This can be worked on later, a sysfs interface is needed anyway for platforms not using DT. Signed-off-by: Rafał Miłecki --- V2: Trying to add DT support, idea postponed as it will take more time to discuss the bindings. V3: Fix typos in commit and Documentation (thanks Jacek!) Use "ports" sysfs file for adding and removing USB ports (thx Jacek) Check if there is USB device connected after adding new USB port Fix memory leak or two Felipe: I'd like to ask for your Ack before having this patch pushed. --- Documentation/leds/ledtrig-usbport.txt | 49 +++ drivers/leds/trigger/Kconfig | 8 ++ drivers/leds/trigger/Makefile | 1 + drivers/leds/trigger/ledtrig-usbport.c | 253 + 4 files changed, 311 insertions(+) create mode 100644 Documentation/leds/ledtrig-usbport.txt create mode 100644 drivers/leds/trigger/ledtrig-usbport.c diff --git a/Documentation/leds/ledtrig-usbport.txt b/Documentation/leds/ledtrig-usbport.txt new file mode 100644 index 000..fa42227 --- /dev/null +++ b/Documentation/leds/ledtrig-usbport.txt @@ -0,0 +1,49 @@ +USB port LED trigger + + +This LED trigger can be used for signalling to the user a presence of USB device +in a given port. It simply turns on LED when device appears and turns it off +when it disappears. + +It requires specifying a list of USB ports that should be observed. Used format +matches Linux kernel format and consists of a root hub number and a hub port +separated by a dash (e.g. 3-1). + +It is also possible to handle devices with internal hubs (that are always +connected to the root hub). User can simply specify internal hub ports then +(e.g. 1-1.1, 1-1.2, etc.). + +Please note that this trigger allows assigning multiple USB ports to a single +LED. This can be useful in two cases: + +1) Device with single USB LED and few physical ports + +In such a case LED will be turned on as long as there is at least one connected +USB device. + +2) Device with a physical port handled by few controllers + +Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical +port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is +only one LED user will most likely want to assign ports from all 3 hubs. + + +This trigger can be activated from user space on led class devices as shown +below: + + echo usbport > trigger + +This adds the following sysfs attributes to the LED: + + ports - Reading it lists all USB ports assigned to the trigger. Writing USB + port number to it will make this driver start observing it. It's also + possible to remove USB port from observable list by writing it with a + "-" prefix. + +Example use-case: + + echo usbport > trigger + echo 4-1 > ports + echo 2-1 > ports + echo -4-1 > ports + cat ports diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig index 3f9ddb9..bdd6fd2 100644 --- a/drivers/leds/trigger/Kconfig +++ b/drivers/leds/trigger/Kconfig @@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC a different trigger. If unsure, say Y. +config LEDS_TRIGGER_USBPORT + tristate "USB port LED trigger" + depends on LEDS_TRIGGERS && USB + help + This allows LEDs to be controlled by USB events. Enabling this option + allows specifying list of USB ports that should turn on LED when some + USB device gets connected. + endif # LEDS_TRIGGERS diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile index a72c43c..56e1741 100644 --- a/drivers/leds/trigger/Makefile +++ b/drivers/leds/trigger/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o +obj-$(CONFIG_LEDS_TRIGGER_USBPORT) += ledtrig-usbport.o diff --git a/drivers/leds/trigger/ledtrig-usbport.c b/drivers/leds/trigger/ledtrig-usbport.c new file mode 100644 index 000..7f5237c --- /dev/null +++ b/drivers/leds/trigger/ledtrig-usbport.c @@ -0,0 +1,253 @@ +/* + * USB port LED trigger + * + * Copyright (C) 2016 Rafał Miłecki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public Licens
Re: [PATCH V3] leds: trigger: Introduce an USB port trigger
On 24 August 2016 at 11:21, Greg KH wrote: > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> This commit adds a new trigger responsible for turning on LED when USB >> device gets connected to the specified USB port. This can can useful for >> various home routers that have USB port(s) and a proper LED telling user >> a device is connected. >> >> The trigger gets its documentation file but basically it just requires >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port). >> >> During work on this trigger there was a plan to add DT bindings for it, >> but there wasn't an agreement on the format yet. This can be worked on >> later, a sysfs interface is needed anyway for platforms not using DT. >> >> Signed-off-by: Rafał Miłecki >> --- >> V2: Trying to add DT support, idea postponed as it will take more time >> to discuss the bindings. >> V3: Fix typos in commit and Documentation (thanks Jacek!) >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek) >> Check if there is USB device connected after adding new USB port >> Fix memory leak or two >> >> Felipe: I'd like to ask for your Ack before having this patch pushed. >> --- >> Documentation/leds/ledtrig-usbport.txt | 49 +++ >> drivers/leds/trigger/Kconfig | 8 ++ >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-usbport.c | 253 >> + >> 4 files changed, 311 insertions(+) >> create mode 100644 Documentation/leds/ledtrig-usbport.txt >> create mode 100644 drivers/leds/trigger/ledtrig-usbport.c > > You are adding sysfs files without adding a Documentation/ABI/ entry, > please never do that. This is the way all LED triggers are documented, I just blindly assumed it's OK. Could you be a bit more helpful and help me to understand further steps? Should I drop Documentation/leds/ledtrig-usbport.txt and add proper entry in Documentation/ABI/testing? Or should I keep both files? What about current sysfs of other triggers? Should they be moved/documented in ABI? -- Rafał
Re: [PATCH V3] leds: trigger: Introduce an USB port trigger
On 24 August 2016 at 11:22, Greg KH wrote: > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote: >> +static ssize_t ports_show(struct device *dev, struct device_attribute *attr, >> + char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data; >> + struct usbport_trig_port *port; >> + ssize_t ret = 0; >> + int len; >> + >> + list_for_each_entry(port, &usbport_data->ports, list) { >> + len = sprintf(buf + ret, "%s\n", port->name); >> + if (len >= 0) >> + ret += len; >> + } >> + >> + return ret; >> +} > > sysfs is "one value per file", here you are listing a bunch of things in > one sysfs file. Please don't do that. OK. What do you think about creating "ports" subdirectory and creating file-per-port in it? Then I'd need to bring back something like "new_port" and "remove_port". Does it sound OK? -- Rafał
Re: [PATCH V3] leds: trigger: Introduce an USB port trigger
On 24 August 2016 at 12:49, Matthias Brugger wrote: > On 24/08/16 00:03, Rafał Miłecki wrote: >> >> From: Rafał Miłecki >> >> This commit adds a new trigger responsible for turning on LED when USB >> device gets connected to the specified USB port. This can can useful for >> various home routers that have USB port(s) and a proper LED telling user >> a device is connected. >> >> The trigger gets its documentation file but basically it just requires >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port). >> >> During work on this trigger there was a plan to add DT bindings for it, >> but there wasn't an agreement on the format yet. This can be worked on >> later, a sysfs interface is needed anyway for platforms not using DT. >> >> Signed-off-by: Rafał Miłecki >> --- >> V2: Trying to add DT support, idea postponed as it will take more time >> to discuss the bindings. >> V3: Fix typos in commit and Documentation (thanks Jacek!) >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek) >> Check if there is USB device connected after adding new USB port >> Fix memory leak or two >> >> Felipe: I'd like to ask for your Ack before having this patch pushed. >> --- >> Documentation/leds/ledtrig-usbport.txt | 49 +++ >> drivers/leds/trigger/Kconfig | 8 ++ >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-usbport.c | 253 >> + >> 4 files changed, 311 insertions(+) >> create mode 100644 Documentation/leds/ledtrig-usbport.txt >> create mode 100644 drivers/leds/trigger/ledtrig-usbport.c >> >> diff --git a/Documentation/leds/ledtrig-usbport.txt >> b/Documentation/leds/ledtrig-usbport.txt >> new file mode 100644 >> index 000..fa42227 >> --- /dev/null >> +++ b/Documentation/leds/ledtrig-usbport.txt >> @@ -0,0 +1,49 @@ >> +USB port LED trigger >> + >> + >> +This LED trigger can be used for signalling to the user a presence of USB >> device >> +in a given port. It simply turns on LED when device appears and turns it >> off >> +when it disappears. >> + >> +It requires specifying a list of USB ports that should be observed. Used >> format >> +matches Linux kernel format and consists of a root hub number and a hub >> port >> +separated by a dash (e.g. 3-1). >> + >> +It is also possible to handle devices with internal hubs (that are always >> +connected to the root hub). User can simply specify internal hub ports >> then >> +(e.g. 1-1.1, 1-1.2, etc.). >> + >> +Please note that this trigger allows assigning multiple USB ports to a >> single >> +LED. This can be useful in two cases: >> + >> +1) Device with single USB LED and few physical ports >> + >> +In such a case LED will be turned on as long as there is at least one >> connected >> +USB device. >> + >> +2) Device with a physical port handled by few controllers >> + >> +Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 >> physical >> +port may be handled by ohci-platform, ehci-platform and xhci-hcd. If >> there is >> +only one LED user will most likely want to assign ports from all 3 hubs. >> + >> + >> +This trigger can be activated from user space on led class devices as >> shown >> +below: >> + >> + echo usbport > trigger >> + >> +This adds the following sysfs attributes to the LED: >> + >> + ports - Reading it lists all USB ports assigned to the trigger. Writing >> USB >> + port number to it will make this driver start observing it. It's >> also >> + possible to remove USB port from observable list by writing it >> with a >> + "-" prefix. >> + >> +Example use-case: >> + >> + echo usbport > trigger >> + echo 4-1 > ports >> + echo 2-1 > ports >> + echo -4-1 > ports >> + cat ports >> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig >> index 3f9ddb9..bdd6fd2 100644 >> --- a/drivers/leds/trigger/Kconfig >> +++ b/drivers/leds/trigger/Kconfig >> @@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC >> a different trigger. >> If unsure, say Y. >> >> +config LEDS_TRIGGER_USBPORT >> + tristate "USB port LED trigger" >> + depends on LEDS_TRIGGERS && USB >> + help >> +
[PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger
From: Rafał Miłecki This commit adds a new trigger responsible for turning on LED when USB device gets connected to the specified USB port. This can can useful for various home routers that have USB port(s) and a proper LED telling user a device is connected. The trigger gets its documentation file but basically it just requires specifying USB port in a Linux format (e.g. echo 1-1 > new_port). During work on this trigger there was a plan to add DT bindings for it, but there wasn't an agreement on the format yet. This can be worked on later, a sysfs interface is needed anyway for platforms not using DT. Signed-off-by: Rafał Miłecki --- V2: Trying to add DT support, idea postponed as it will take more time to discuss the bindings. V3: Fix typos in commit and Documentation (thanks Jacek!) Use "ports" sysfs file for adding and removing USB ports (thx Jacek) Check if there is USB device connected after adding new USB port Fix memory leak or two V3.5: Fix e-mail address (thanks Matthias) Simplify conditions in usbport_trig_notify (thx Matthias) Make "ports" a subdirectory with file per port, to match one value per file sysfs rule (thanks Greg) As "ports" couldn't be used for adding and removing ports anymore, there are now "new_port" and "remove_port". Having them makes this API also common with e.g. pci and usb buses. The last big missing thing is Documentation update (this is why I'm sending RFC). Greg pointed out we should have some entries in Documentation/ABI, but it seems none of triggers have it. Any idea why is that? Do we need to change it? Or is it required for new code only? If so, should I care about Documentation/leds/ledtrig-usbport.txt at all in this patch? For now I didn't update Documentation/leds/ledtrig-usbport.txt with the new new_port and remove_port API, until I get a clue how to proceed. --- Documentation/leds/ledtrig-usbport.txt | 49 ++ drivers/leds/trigger/Kconfig | 8 + drivers/leds/trigger/Makefile | 1 + drivers/leds/trigger/ledtrig-usbport.c | 309 + 4 files changed, 367 insertions(+) create mode 100644 Documentation/leds/ledtrig-usbport.txt create mode 100644 drivers/leds/trigger/ledtrig-usbport.c diff --git a/Documentation/leds/ledtrig-usbport.txt b/Documentation/leds/ledtrig-usbport.txt new file mode 100644 index 000..fa42227 --- /dev/null +++ b/Documentation/leds/ledtrig-usbport.txt @@ -0,0 +1,49 @@ +USB port LED trigger + + +This LED trigger can be used for signalling to the user a presence of USB device +in a given port. It simply turns on LED when device appears and turns it off +when it disappears. + +It requires specifying a list of USB ports that should be observed. Used format +matches Linux kernel format and consists of a root hub number and a hub port +separated by a dash (e.g. 3-1). + +It is also possible to handle devices with internal hubs (that are always +connected to the root hub). User can simply specify internal hub ports then +(e.g. 1-1.1, 1-1.2, etc.). + +Please note that this trigger allows assigning multiple USB ports to a single +LED. This can be useful in two cases: + +1) Device with single USB LED and few physical ports + +In such a case LED will be turned on as long as there is at least one connected +USB device. + +2) Device with a physical port handled by few controllers + +Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical +port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is +only one LED user will most likely want to assign ports from all 3 hubs. + + +This trigger can be activated from user space on led class devices as shown +below: + + echo usbport > trigger + +This adds the following sysfs attributes to the LED: + + ports - Reading it lists all USB ports assigned to the trigger. Writing USB + port number to it will make this driver start observing it. It's also + possible to remove USB port from observable list by writing it with a + "-" prefix. + +Example use-case: + + echo usbport > trigger + echo 4-1 > ports + echo 2-1 > ports + echo -4-1 > ports + cat ports diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig index 3f9ddb9..bdd6fd2 100644 --- a/drivers/leds/trigger/Kconfig +++ b/drivers/leds/trigger/Kconfig @@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC a different trigger. If unsure, say Y. +config LEDS_TRIGGER_USBPORT + tristate "USB port LED trigger" + depends on LEDS_TRIGGERS && USB + help + This allows LEDs to be controlled by USB events. Enabling this option + allows specifying list of USB ports that should turn on LED when some + USB device gets connected. + endif # LEDS_TRIGGERS diff --git a/driver
Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger
On 24 August 2016 at 20:48, Bjørn Mork wrote: > Rafał Miłecki writes: > >> The last big missing thing is Documentation update (this is why I'm >> sending RFC). Greg pointed out we should have some entries in >> Documentation/ABI, but it seems none of triggers have it. > > There's a lot missing, but there is at least one exception: > The "inverted" attribute of the gpio and backlight triggers is > documented as part of Documentation/ABI/testing/sysfs-class-led > >> Any idea why is that? > > Manual enforcement fails from time to time? The requirement was less > strict in the early days of sysfs? Does it matter? > >> Do we need to change it? Or is it required for new code only? > > The lack of ABI docs is a bug. Don't add new code with known bugs. Old > code should be fixed, but there is no immediate *need* to fix it. Don't get me wrong. I care about code quality and documentation. I mean to follow kernel rules. It's just some things may be not clear to ppl and it would be nice to get any explanation/help. That said thanks a lot of your e-mail, I'll try to expand pointed Documentation. -- Rafał
Re: [PATCH V3] leds: trigger: Introduce an USB port trigger
On 24 August 2016 at 23:04, Greg KH wrote: > On Wed, Aug 24, 2016 at 11:29:51AM +0200, Rafał Miłecki wrote: >> On 24 August 2016 at 11:22, Greg KH wrote: >> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote: >> >> +static ssize_t ports_show(struct device *dev, struct device_attribute >> >> *attr, >> >> + char *buf) >> >> +{ >> >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> >> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data; >> >> + struct usbport_trig_port *port; >> >> + ssize_t ret = 0; >> >> + int len; >> >> + >> >> + list_for_each_entry(port, &usbport_data->ports, list) { >> >> + len = sprintf(buf + ret, "%s\n", port->name); >> >> + if (len >= 0) >> >> + ret += len; >> >> + } >> >> + >> >> + return ret; >> >> +} >> > >> > sysfs is "one value per file", here you are listing a bunch of things in >> > one sysfs file. Please don't do that. >> >> OK. What do you think about creating "ports" subdirectory and creating >> file-per-port in it? Then I'd need to bring back something like >> "new_port" and "remove_port". Does it sound OK? > > Maybe, I don't know. Why is "USB" somehow unique here? Why isn't this > the same for PCI slots/ports? pccard? sdcard? thunderbolt? Good question. I would like to extend this USB port trigger in the future by reacting to USB activity. This involves playing with URBs and I believe that at that point it'd be getting too much USB specific to /rule them all/. -- Rafał
[PATCH V4] leds: trigger: Introduce an USB port trigger
From: Rafał Miłecki This commit adds a new trigger responsible for turning on LED when USB device gets connected to the specified USB port. This can can useful for various home routers that have USB port(s) and a proper LED telling user a device is connected. The trigger gets its documentation file but basically it just requires specifying USB port in a Linux format (e.g. echo 1-1 > new_port). During work on this trigger there was a plan to add DT bindings for it, but there wasn't an agreement on the format yet. This can be worked on later, a sysfs interface is needed anyway for platforms not using DT. Another planned feature is support for LED reacting to the USB activity. This can be implemented with another sysfs file for setting mode. The default mode wouldn't change so there won't be ABI breakage and such feature can be safely implemented later. Signed-off-by: Rafał Miłecki --- V2: Trying to add DT support, idea postponed as it will take more time to discuss the bindings. V3: Fix typos in commit and Documentation (thanks Jacek!) Use "ports" sysfs file for adding and removing USB ports (thx Jacek) Check if there is USB device connected after adding new USB port Fix memory leak or two V3.5: Fix e-mail address (thanks Matthias) Simplify conditions in usbport_trig_notify (thx Matthias) Make "ports" a subdirectory with file per port, to match one value per file sysfs rule (thanks Greg) As "ports" couldn't be used for adding and removing ports anymore, there are now "new_port" and "remove_port". Having them makes this API also common with e.g. pci and usb buses. V4: Add Documentation/ABI/testing/sysfs-class-led-trigger-usbport and reference it in Documentation/leds/ledtrig-usbport.txt to avoid doc duplication. --- .../ABI/testing/sysfs-class-led-trigger-usbport| 25 ++ Documentation/leds/ledtrig-usbport.txt | 46 +++ drivers/leds/trigger/Kconfig | 8 + drivers/leds/trigger/Makefile | 1 + drivers/leds/trigger/ledtrig-usbport.c | 309 + 5 files changed, 389 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-usbport create mode 100644 Documentation/leds/ledtrig-usbport.txt create mode 100644 drivers/leds/trigger/ledtrig-usbport.c diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-usbport b/Documentation/ABI/testing/sysfs-class-led-trigger-usbport new file mode 100644 index 000..653bde1 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-usbport @@ -0,0 +1,25 @@ +What: /sys/class/leds//new_port +Date: August 2016 +KernelVersion: 4.9 +Contact: linux-l...@vger.kernel.org +Description: + Adds USB port (specified by a name using Linux format, e.g. 1-1) + to the list of observable ones. This will have immediate effect + if there is device already connected. This will also make + trigger checking for a match on every USB (dis)connection event. + +What: /sys/class/leds//remove_port +Date: August 2016 +KernelVersion: 4.9 +Contact: linux-l...@vger.kernel.org +Description: + This allows removing USB port from the list of observable ones. + +What: /sys/class/leds//ports/ +Date: August 2016 +KernelVersion: 4.9 +Contact: linux-l...@vger.kernel.org +Description: + Every port entry is a name (in Linux format) of USB port that is + being observed by the usbport trigger. It's useful for checking + the current state of trigger setup. diff --git a/Documentation/leds/ledtrig-usbport.txt b/Documentation/leds/ledtrig-usbport.txt new file mode 100644 index 000..8e36e2d --- /dev/null +++ b/Documentation/leds/ledtrig-usbport.txt @@ -0,0 +1,46 @@ +USB port LED trigger + + +This LED trigger can be used for signalling to the user a presence of USB device +in a given port. It simply turns on LED when device appears and turns it off +when it disappears. + +It requires specifying a list of USB ports that should be observed. Used format +matches Linux kernel format and consists of a root hub number and a hub port +separated by a dash (e.g. 3-1). + +It is also possible to handle devices with internal hubs (that are always +connected to the root hub). User can simply specify internal hub ports then +(e.g. 1-1.1, 1-1.2, etc.). + +Please note that this trigger allows assigning multiple USB ports to a single +LED. This can be useful in two cases: + +1) Device with single USB LED and few physical ports + +In such a case LED will be turned on as long as there is at least one connected +USB device. + +2) Device with a physical port handled by few controllers + +Some devices have e.g. one controller per PHY standard. E.g. USB 3.0
Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger
On 25 August 2016 at 10:03, Jacek Anaszewski wrote: > On 08/24/2016 07:52 PM, Rafał Miłecki wrote: >> >> From: Rafał Miłecki >> >> This commit adds a new trigger responsible for turning on LED when USB >> device gets connected to the specified USB port. This can can useful for >> various home routers that have USB port(s) and a proper LED telling user >> a device is connected. >> >> The trigger gets its documentation file but basically it just requires >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port). >> >> During work on this trigger there was a plan to add DT bindings for it, >> but there wasn't an agreement on the format yet. This can be worked on >> later, a sysfs interface is needed anyway for platforms not using DT. >> >> Signed-off-by: Rafał Miłecki >> --- >> V2: Trying to add DT support, idea postponed as it will take more time >> to discuss the bindings. >> V3: Fix typos in commit and Documentation (thanks Jacek!) >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek) >> Check if there is USB device connected after adding new USB port >> Fix memory leak or two >> V3.5: Fix e-mail address (thanks Matthias) >> Simplify conditions in usbport_trig_notify (thx Matthias) >> Make "ports" a subdirectory with file per port, to match one value >> per file sysfs rule (thanks Greg) >> As "ports" couldn't be used for adding and removing ports anymore, >> there are now "new_port" and "remove_port". Having them makes this >> API also common with e.g. pci and usb buses. > > > Now writing new_port with "1-1" produces a file named "1-1" in the > ports directory with 000 permissions. I think that what Greg had > on mind by referring to "one value per file" rule was a set of > files representing ports, like "1-1 1-2 2-1", and each file should be > readable/writeable. > > For instance "echo 1 > 1-1" would enable the trigger for the port 1-1 > and "echo 0 > 1-1" would disable it. The problem is that we don't know > the number of required ports at compilation time and the sysfs > attributes would have to be dynamically created on driver instantiation. > What is more, as the USB ports can dynamically appear/disappear in the > system, the files would have to be created/removed accordingly during > LED class device lifetime, which is not the best design for the sysfs > interface I think. > > Therefore, maybe it would be good to follow the "triggers" sysfs > attribute pattern, where it lists the available LED triggers? > > The question is whether there is some mechanism available for > notifying addition/removal of a USB port? Every port is part of some hub and every hub (even root one) is a USB device. So I could just get struct usb_device and check its "maxchild" property. If e.g. I get USB device "1-1" with maxchild 4, I know there are: 1-1.1 1-1.2 1-1.3 1-1.4 So the sysfs structure you suggested would be possible, I just don't know if it's preferred one or not. Confirmation: yes, right now I create simple files with chmod 000 for every port added to the usbport observable list. > Also a description of the device connected to the port would be a nice > feature, however I am not certain about the feasibility thereof. What kind of description do you mean? Where should it be used / where should it appear? -- Rafał
Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger
On 25 August 2016 at 10:03, Jacek Anaszewski wrote: > On 08/24/2016 07:52 PM, Rafał Miłecki wrote: >> >> From: Rafał Miłecki >> >> This commit adds a new trigger responsible for turning on LED when USB >> device gets connected to the specified USB port. This can can useful for >> various home routers that have USB port(s) and a proper LED telling user >> a device is connected. >> >> The trigger gets its documentation file but basically it just requires >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port). >> >> During work on this trigger there was a plan to add DT bindings for it, >> but there wasn't an agreement on the format yet. This can be worked on >> later, a sysfs interface is needed anyway for platforms not using DT. >> >> Signed-off-by: Rafał Miłecki >> --- >> V2: Trying to add DT support, idea postponed as it will take more time >> to discuss the bindings. >> V3: Fix typos in commit and Documentation (thanks Jacek!) >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek) >> Check if there is USB device connected after adding new USB port >> Fix memory leak or two >> V3.5: Fix e-mail address (thanks Matthias) >> Simplify conditions in usbport_trig_notify (thx Matthias) >> Make "ports" a subdirectory with file per port, to match one value >> per file sysfs rule (thanks Greg) >> As "ports" couldn't be used for adding and removing ports anymore, >> there are now "new_port" and "remove_port". Having them makes this >> API also common with e.g. pci and usb buses. > > > Now writing new_port with "1-1" produces a file named "1-1" in the > ports directory with 000 permissions. I think that what Greg had > on mind by referring to "one value per file" rule was a set of > files representing ports, like "1-1 1-2 2-1", and each file should be > readable/writeable. On the other hand, when I described my idea with "new_port" and "remove_port", Greg replied with "Maybe", so I'm not sure if he really got a design you described in his mind. Greg: could you comment on this sysfs interface, please? :) -- Rafał
[PATCH] Documentation: move oneshot trigger attributes documentation to ABI
From: Rafał Miłecki Documentation of sysfs interface should be in ABI in the first place. This moves relevant part of documentation and mentions where to look for it. Signed-off-by: Rafał Miłecki --- Documentation/ABI/testing/sysfs-class-led | 3 +- .../ABI/testing/sysfs-class-led-trigger-oneshot| 37 ++ Documentation/leds/ledtrig-oneshot.txt | 20 ++-- 3 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-oneshot diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 3646ec8..86ace28 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -24,7 +24,8 @@ Description: of led events. You can change triggers in a similar manner to the way an IO scheduler is chosen. Trigger specific parameters can appear in - /sys/class/leds/ once a given trigger is selected. + /sys/class/leds/ once a given trigger is selected. For + their documentation see sysfs-class-led-trigger-*. What: /sys/class/leds//inverted Date: January 2011 diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot b/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot new file mode 100644 index 000..401cbe6 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot @@ -0,0 +1,37 @@ +What: /sys/class/leds//delay_on +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Specifies for how many milliseconds the LED has to stay at + LED_FULL brightness after it has been armed. + Default to 100 ms. + + +What: /sys/class/leds//delay_off +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Specifies for how many milliseconds the LED has to stay at + LED_OFF brightness after it has been armed. + Default to 100 ms. + +What: /sys/class/leds//invert +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Reverse the blink logic. If set to 0 (default) blink on for + delay_on ms, then blink off for delay_off ms, leaving the LED + normally off. If set to 1, blink off for delay_off ms, then + blink on for delay_on ms, leaving the LED normally on. + Setting this value also immediately change the LED state. + +What: /sys/class/leds//shot +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Write any non-empty string to signal an events, this starts a + blink sequence if not already running. diff --git a/Documentation/leds/ledtrig-oneshot.txt b/Documentation/leds/ledtrig-oneshot.txt index 07cd1fa..fe57474 100644 --- a/Documentation/leds/ledtrig-oneshot.txt +++ b/Documentation/leds/ledtrig-oneshot.txt @@ -21,24 +21,8 @@ below: echo oneshot > trigger -This adds the following sysfs attributes to the LED: - - delay_on - specifies for how many milliseconds the LED has to stay at - LED_FULL brightness after it has been armed. - Default to 100 ms. - - delay_off - specifies for how many milliseconds the LED has to stay at - LED_OFF brightness after it has been armed. - Default to 100 ms. - - invert - reverse the blink logic. If set to 0 (default) blink on for delay_on - ms, then blink off for delay_off ms, leaving the LED normally off. If - set to 1, blink off for delay_off ms, then blink on for delay_on ms, - leaving the LED normally on. - Setting this value also immediately change the LED state. - - shot - write any non-empty string to signal an events, this starts a blink - sequence if not already running. +This adds sysfs attributes to the LED that are documented in: +Documentation/ABI/testing/sysfs-class-led-trigger-oneshot Example use-case: network devices, initialization: -- 2.9.3
Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
On 23 August 2016 at 21:55, Rob Herring wrote: > On Tue, Aug 23, 2016 at 08:25:59AM +0200, Rafał Miłecki wrote: >> From: Rafał Miłecki >> >> This clock is present on BCM53573 devices (including BCM47189) that use >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should >> be defined as one of its subnodes (subdevices). For more details see >> Documentation entry. >> >> Unfortunately there isn't a set of registers related to ILP clock only. >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g. >> "retention*" and "control_ext" regs. This is why this driver maps all >> 0x1000 B of space. > > Then describe the block as a syscon which has several functions of > which clocks are one. This isn't clear to me, sorry, could you describe it? Would you like me to update commit message or documentation? Is code fine as is?
Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
On 24 August 2016 at 10:47, Stephen Boyd wrote: > On 08/23, Rafał Miłecki wrote: >> diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c >> b/drivers/clk/bcm/clk-bcm53573-ilp.c >> new file mode 100644 >> index 000..b7ac0eb >> --- /dev/null >> +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c >> @@ -0,0 +1,146 @@ >> +/* >> + * Copyright (C) 2016 Rafał Miłecki >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include > > Is this include used? No. Good point. >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define PMU_XTAL_FREQ_RATIO 0x66c >> +#define XTAL_ALP_PER_4ILP 0x1fff >> +#define XTAL_CTL_EN 0x8000 >> +#define PMU_SLOW_CLK_PERIOD 0x6dc >> + >> +struct bcm53573_ilp { >> + struct clk *clk; >> + struct clk_hw hw; >> + void __iomem *pmu; >> +}; >> + >> +static int bcm53573_ilp_enable(struct clk_hw *hw) >> +{ >> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); >> + >> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD); >> + writel(0x1, ilp->pmu + 0x674); > > Is there a name for 0x674? >> + >> + return 0; >> +} >> + >> +static unsigned long bcm53573_ilp_recalc_rate(struct clk_hw *hw, >> + unsigned long parent_rate) >> +{ >> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); >> + void __iomem *pmu = ilp->pmu; >> + u32 last_val, cur_val; >> + u32 sum = 0, num = 0, loop_num = 0; > > Should these just be plain ints? Do we care about sizes for these > variables? > >> + u32 avg; > > This one too. Right. >> + >> + /* Enable measurement */ >> + writel(XTAL_CTL_EN, pmu + PMU_XTAL_FREQ_RATIO); >> + >> + /* Read initial value */ >> + last_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; >> + >> + /* >> + * At minimum we should loop for a bit to let hardware do the >> + * measurement. This isn't very accurate however, so for a better >> + * precision lets try getting 20 different values for and use average. >> + */ >> + while (num < 20) { >> + cur_val = readl(pmu + PMU_XTAL_FREQ_RATIO) & XTAL_ALP_PER_4ILP; >> + >> + if (cur_val != last_val) { >> + /* Got different value, use it */ >> + sum += cur_val; >> + num++; >> + loop_num = 0; >> + last_val = cur_val; >> + } else if (++loop_num > 5000) { >> + /* Same value over and over, give up */ >> + sum += cur_val; >> + num++; >> + break; >> + } > > Should there be a udelay() here? Or we're expected to tight loop > read the hardware? If so we should throw in a cpu_relax() here to > indicate tight loop. > >> + } >> + >> + /* Disable measurement to save power */ >> + writel(0x0, pmu + PMU_XTAL_FREQ_RATIO); >> + >> + avg = sum / num; >> + >> + return parent_rate * 4 / avg; >> +} >> + >> +static const struct clk_ops bcm53573_ilp_clk_ops = { >> + .enable = bcm53573_ilp_enable, > > No disable? Or .is_enabled? The beauty of working without datasheets... I'll compare initial reg state with one after enabling and see if there is sth obvious. >> + .recalc_rate = bcm53573_ilp_recalc_rate, >> +}; >> + >> +static void bcm53573_ilp_init(struct device_node *np) >> +{ >> + struct bcm53573_ilp *ilp; >> + struct resource res; >> + struct clk_init_data init = { 0 }; >> + const char *parent_name; >> + int index; >> + int err; >> + >> + ilp = kzalloc(sizeof(*ilp), GFP_KERNEL); >> + if (!ilp) >> + return; >> + >> + parent_name = of_clk_get_parent_name(np, 0); >> + if (!parent_name) { >> + err = -ENOENT; >> + goto err_free_ilp; >> + } >> + >> + /* TODO: This looks generic, try making it OF helper. */ >> +
Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock
On 24 August 2016 at 10:47, Stephen Boyd wrote: > On 08/23, Rafał Miłecki wrote: >> +static int bcm53573_ilp_enable(struct clk_hw *hw) >> +{ >> + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); >> + >> + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD); >> + writel(0x1, ilp->pmu + 0x674); > > Is there a name for 0x674? No in any sources published by Broadcom. I was experimenting with bit 0x1 in PMU_SLOW_CLK_PERIOD and 0x1 in 0x674. Both have to be set. I was suspecting that maybe one of them is just some trigger and it can be switched back to 0, but it's not the case. I definitely need to set both of them to get clock working.
[PATCH V6] clk: bcm: Add driver for BCM53573 ILP clock
From: Rafał Miłecki This clock is present on BCM53573 devices (including BCM47189) that use Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should be defined as one of its subnodes (subdevices). For more details see Documentation entry. Unfortunately there isn't a set of registers related to ILP clock only. We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g. "retention*" and "control_ext" regs. This is why this driver maps all 0x1000 B of space. Signed-off-by: Rafał Miłecki --- V2: Rebase on top of clk-next Use ALP as parent clock Improve comments Switch from ioremap_nocache to ioremap Check of_clk_add_provide result for error V3: Drop #include Make ILP DT entry part of PMU Describe ILP as subdevice of PMU in Documentation V4: Use BCM53573 name as suggested by Jon and Ray. It seems "Northstar" (even if used in some resources) should be used in relation to Cortex-A9 devices only. V5: Rename remaining "ns" references to "bcm53573", sorry, I sent V4 too early. V6: Drop #include Use "int" as type where it matches usage Add cpu_relax() in the loop Add disable callback Use _hw_ functions for registering struct clk_hw (new API) Thanks a lot Stephen! --- .../bindings/clock/brcm,bcm53573-ilp.txt | 40 ++ drivers/clk/bcm/Makefile | 1 + drivers/clk/bcm/clk-bcm53573-ilp.c | 157 + 3 files changed, 198 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt create mode 100644 drivers/clk/bcm/clk-bcm53573-ilp.c diff --git a/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt new file mode 100644 index 000..5ab3107 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/brcm,bcm53573-ilp.txt @@ -0,0 +1,40 @@ +Broadcom BCM53573 ILP clock +=== + +This binding uses the common clock binding: +Documentation/devicetree/bindings/clock/clock-bindings.txt + +This binding is used for ILP clock (sometimes referred as "slow clock") +on Broadcom BCM53573 devices using Cortex-A7 CPU. + +This clock is part of PMU (Power Management Unit), a Broadcom's device +handing power-related aspects. Please note PMU contains more subdevices, +ILP is only one of them. + +ILP's rate has to be calculated on runtime and it depends on ALP clock +which has to be referenced. + +Required properties: +- compatible: "brcm,bcm53573-ilp" +- reg: iomem address range of PMU (Power Management Unit) +- reg-names: "pmu", the only needed & supported reg right now +- clocks: has to reference an ALP clock +- #clock-cells: should be <0> + +Example: + +pmu@18012000 { + compatible = "simple-bus"; + ranges = <0x 0x18012000 0x1000>; + #address-cells = <1>; + #size-cells = <1>; + + ilp: ilp@0 { + compatible = "brcm,bcm53573-ilp"; + reg = <0 0x1000>; + reg-names = "pmu"; + clocks = <&alp>; + #clock-cells = <0>; + clock-output-names = "ilp"; + }; +}; diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 1d79bd2..4b8c56d 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_COMMON_CLK_IPROC)+= clk-ns2.o obj-$(CONFIG_ARCH_BCM_CYGNUS) += clk-cygnus.o obj-$(CONFIG_ARCH_BCM_NSP) += clk-nsp.o obj-$(CONFIG_ARCH_BCM_5301X) += clk-nsp.o +obj-$(CONFIG_ARCH_BCM_53573) += clk-bcm53573-ilp.o diff --git a/drivers/clk/bcm/clk-bcm53573-ilp.c b/drivers/clk/bcm/clk-bcm53573-ilp.c new file mode 100644 index 000..faf2b0a --- /dev/null +++ b/drivers/clk/bcm/clk-bcm53573-ilp.c @@ -0,0 +1,157 @@ +/* + * Copyright (C) 2016 Rafał Miłecki + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +#define PMU_XTAL_FREQ_RATIO0x66c +#define XTAL_ALP_PER_4ILP 0x1fff +#define XTAL_CTL_EN 0x8000 +#define PMU_SLOW_CLK_PERIOD0x6dc + +struct bcm53573_ilp { + struct clk_hw hw; + void __iomem *pmu; +}; + +static int bcm53573_ilp_enable(struct clk_hw *hw) +{ + struct bcm53573_ilp *ilp = container_of(hw, struct bcm53573_ilp, hw); + + writel(0x10199, ilp->pmu + PMU_SLOW_CLK_PERIOD); + writel(0x1, ilp->pmu + 0x674); + + return 0; +} + +static void bcm53573_ilp_disable(struct clk_hw *hw) +{ + struct bcm53573_ilp *ilp = c
[PATCH] clk: return unsigned int in dummy non-OF of_clk_get_parent_count()
From: Rafał Miłecki In the commit 929e7f3bc7b82 ("clk: Make of_clk_get_parent_count() return unsigned ints") of_clk_get_parent_count has been modified to return unsigned int. There is also a dummy implementation of the same function for configs without CONFIG_OF. For the consistency it should be updated as well. Signed-off-by: Rafał Miłecki --- include/linux/clk-provider.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index f403b8a..37b8fdc 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -854,7 +854,7 @@ of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data) { return ERR_PTR(-ENOENT); } -static inline int of_clk_get_parent_count(struct device_node *np) +static inline unsigned int of_clk_get_parent_count(struct device_node *np) { return 0; } -- 2.9.3
[PATCH V2] Documentation: move oneshot trigger attributes documentation to ABI
From: Rafał Miłecki Documentation of sysfs interface should be in ABI in the first place. This moves relevant part of documentation and mentions where to look for it. Fix trivial typos whilst we are at it. Signed-off-by: Rafał Miłecki --- V2: s/Default/Defaults/ s/ / / s/change/changes/ --- Documentation/ABI/testing/sysfs-class-led | 3 +- .../ABI/testing/sysfs-class-led-trigger-oneshot| 36 ++ Documentation/leds/ledtrig-oneshot.txt | 20 ++-- 3 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-oneshot diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 3646ec8..86ace28 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -24,7 +24,8 @@ Description: of led events. You can change triggers in a similar manner to the way an IO scheduler is chosen. Trigger specific parameters can appear in - /sys/class/leds/ once a given trigger is selected. + /sys/class/leds/ once a given trigger is selected. For + their documentation see sysfs-class-led-trigger-*. What: /sys/class/leds//inverted Date: January 2011 diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot b/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot new file mode 100644 index 000..378a3a4 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-oneshot @@ -0,0 +1,36 @@ +What: /sys/class/leds//delay_on +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Specifies for how many milliseconds the LED has to stay at + LED_FULL brightness after it has been armed. + Defaults to 100 ms. + +What: /sys/class/leds//delay_off +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Specifies for how many milliseconds the LED has to stay at + LED_OFF brightness after it has been armed. + Defaults to 100 ms. + +What: /sys/class/leds//invert +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Reverse the blink logic. If set to 0 (default) blink on for + delay_on ms, then blink off for delay_off ms, leaving the LED + normally off. If set to 1, blink off for delay_off ms, then + blink on for delay_on ms, leaving the LED normally on. + Setting this value also immediately changes the LED state. + +What: /sys/class/leds//shot +Date: Jun 2012 +KernelVersion: 3.6 +Contact: linux-l...@vger.kernel.org +Description: + Write any non-empty string to signal an events, this starts a + blink sequence if not already running. diff --git a/Documentation/leds/ledtrig-oneshot.txt b/Documentation/leds/ledtrig-oneshot.txt index 07cd1fa..fe57474 100644 --- a/Documentation/leds/ledtrig-oneshot.txt +++ b/Documentation/leds/ledtrig-oneshot.txt @@ -21,24 +21,8 @@ below: echo oneshot > trigger -This adds the following sysfs attributes to the LED: - - delay_on - specifies for how many milliseconds the LED has to stay at - LED_FULL brightness after it has been armed. - Default to 100 ms. - - delay_off - specifies for how many milliseconds the LED has to stay at - LED_OFF brightness after it has been armed. - Default to 100 ms. - - invert - reverse the blink logic. If set to 0 (default) blink on for delay_on - ms, then blink off for delay_off ms, leaving the LED normally off. If - set to 1, blink off for delay_off ms, then blink on for delay_on ms, - leaving the LED normally on. - Setting this value also immediately change the LED state. - - shot - write any non-empty string to signal an events, this starts a blink - sequence if not already running. +This adds sysfs attributes to the LED that are documented in: +Documentation/ABI/testing/sysfs-class-led-trigger-oneshot Example use-case: network devices, initialization: -- 2.9.3
Re: [PATCH V4] leds: trigger: Introduce an USB port trigger
On 25 August 2016 at 14:49, Greg KH wrote: > On Thu, Aug 25, 2016 at 10:03:52AM +0200, Rafał Miłecki wrote: >> +static void usbport_trig_activate(struct led_classdev *led_cdev) >> +{ >> + struct usbport_trig_data *usbport_data; >> + int err; >> + >> + usbport_data = kzalloc(sizeof(*usbport_data), GFP_KERNEL); >> + if (!usbport_data) >> + return; >> + usbport_data->led_cdev = led_cdev; >> + >> + /* Storing ports */ >> + INIT_LIST_HEAD(&usbport_data->ports); >> + usbport_data->ports_dir = kobject_create_and_add("ports", >> + &led_cdev->dev->kobj); > > If you _ever_ find yourself in a driver having to use kobject calls, > it's a HUGE hint that you are doing something wrong. > > Hint, you are doing this wrong :) > > Use an attribute group if you need a subdirectory in sysfs, using a > "raw" kobject like this hides it from all userspace tools and so no > userspace program can see it (hint, try using libudev to access these > files attached to the device...) Oops. Thanks for pointing groups to me. I was looking at sysfs.h earlier but I didn't realize group can be a subdirectory. I can see these sysfs_create_group(s) and friends now, thanks. >> + if (!usbport_data->ports_dir) >> + goto err_free; >> + >> + /* API for ports management */ >> + err = device_create_file(led_cdev->dev, &dev_attr_new_port); >> + if (err) >> + goto err_put_ports; >> + err = device_create_file(led_cdev->dev, &dev_attr_remove_port); >> + if (err) >> + goto err_remove_new_port; > > Doesn't this race with userspace and fail? Shouldn't the led core be > creating your leds for you? These questions aren't clear to me. What kind of race? Doing echo usbport > trigger results in trigger core calling usbport_trig_activate. We create new attributes and then we return. I'm also not creating any leds there. This already has to be LED available if you want to assign some trigger to it. >> + >> + /* Notifications */ >> + usbport_data->nb.notifier_call = usbport_trig_notify, >> + led_cdev->trigger_data = usbport_data; >> + usb_register_notify(&usbport_data->nb); > > Don't abuse the USB notifier chain with stuff like this please, is that > really necessary? Why can't your hardware just tell you what USB ports > are in use "out of band"? I totally don't understand this part. This LED trigger is supposed to react to USB devices appearing at specified ports. How else can I know there is a new USB device if not by notifications? I'm wondering if we are on the same page. Could it be my idea of this LED trigger is not clear at all? Could you take a look at commit message again, please? Mostly this part: > This commit adds a new trigger responsible for turning on LED when USB > device gets connected to the specified USB port. This can can useful for > various home routers that have USB port(s) and a proper LED telling user > a device is connected. Can I add something more to make it clear what this trigger is responsible for? >> + >> + led_cdev->activated = true; >> + return; >> + >> +err_remove_new_port: >> + device_remove_file(led_cdev->dev, &dev_attr_new_port); >> +err_put_ports: >> + kobject_put(usbport_data->ports_dir); >> +err_free: >> + kfree(usbport_data); >> +} > > And again, why is this USB specific? Why can't you use this same > userspace api and codebase for PCI ports? For a sdcard port? For a > thunderbolt port? I'm leaving this one unanswered as discussion on this continued in V3.5 thread below my reply: On 25 August 2016 at 07:14, Rafał Miłecki wrote: > Good question. I would like to extend this USB port trigger in the > future by reacting to USB activity. This involves playing with URBs > and I believe that at that point it'd be getting too much USB specific > to /rule them all/. -- Rafał
Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger
On 25 August 2016 at 20:48, Jacek Anaszewski wrote: > On 08/25/2016 04:30 PM, Alan Stern wrote: >> >> On Thu, 25 Aug 2016, Jacek Anaszewski wrote: >> >>> I'd see it as follows: >>> >>> #cat available_ports >>> #1-1 1-2 2-1 >>> >>> #echo "1-1" > new_port >>> >>> #cat observed_ports >>> #1-1 >>> >>> #echo "2-1" > new_port >>> >>> #cat observed_ports >>> #1-1 2-1 >>> >>> We've already had few discussions about the sysfs designs trying >>> to break the one-value-per-file rule for LED class device, and >>> there was always strong resistance against. >> >> >> This scheme has multiple values in both the available_ports and >> observed_ports files. :-( Not that I have any better suggestions... > > > Right, I forgot to add a note here, that this follows space > separated list pattern similarly as in case of triggers attribute. > Of course other suggestions are welcome. So ppl have doubts about multiple values in a single sysfs file (whatever we call it: "ports" or "observed_ports"). Greg clearly said: > sysfs is "one value per file", here you are listing a bunch of things in > one sysfs file. Please don't do that. What about my idea of using "ports" subdirectory and having each port as separated file inside that subdir? I think there are two ways of doing this: 1) Having "ports" subdir with 0x chmod files, one per each port specified as observable In this solution we need "new_port" and "remove_port" that can be used for management of observable ports. I think Jacek wasn't happy with this chmod and he believes Greg meant R/W files. 2) Having "ports" subdir with RW files, one per each existing physical port In this situation we don't need "new_port" or "remove_port". If we want port to be observable we just do: echo 1 > 1-1 Implementing this solution needs reading more details from USB subsystem. Do you find any of solutions with "ports" subdir better than dealing with new-line/space separated values in a single sysfs file? -- Rafał
Re: [PATCH 3.10 162/268] bcma: use (get|put)_device when probing/removing device driver
On 2017-06-20 08:14, Willy Tarreau wrote: On Tue, Jun 20, 2017 at 08:12:26AM +0300, Kalle Valo wrote: Willy Tarreau writes: > From: Rafał Miłecki > > commit a971df0b9d04674e325346c17de9a895425ca5e1 upstream. > > This allows tracking device state and e.g. makes devm work as expected. > > Signed-off-by: Rafał Miłecki > Signed-off-by: Kalle Valo > Signed-off-by: Willy Tarreau UTF-8 characters seem to be broken. I've just checked and the commit properly contains C582 here. At least the content type on your mail looks wrong: Content-Type: text/plain; charset=latin1 I'm defaulting to latin1 but I don't know how to change the default only to send a series, so that indeed results in such things from time to time during reviews, I'm sorry. Do you know if my name will appear correctly in git [0]? [0] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-3.10.y
Re: [PATCH 3.10 162/268] bcma: use (get|put)_device when probing/removing device driver
On 2017-06-20 09:58, Willy Tarreau wrote: On Tue, Jun 20, 2017 at 09:31:00AM +0200, Rafal Milecki wrote: Do you know if my name will appear correctly in git [0]? [0] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/log/?h=linux-3.10.y I would have almost promised it was going to be OK but it's bogus so it probably happened during the patch manipulation, thus I'll fix it before the final release : https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/linux-stable.git/commit/?h=linux-3.10.y-queue&id=73148299c53be25 OK, thanks!
Re: [PATCH 3/3] net: wireless: b43: statics Don't init to 0
On 18 October 2015 at 01:01, Paul McQuade wrote: > Don't turn statics to 0 or NULL Same request as Michael's in 2/3. Your "statics Don't init to 0" message sounds strange, statics *are* initialized to 0 by default (as you probably know but described it in a unclear way). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH for 4.13 0/5] phy: bcm-ns-usb3: add MDIO driver
From: Rafał Miłecki As explained in the commit 9200c6f177638 ("Revert "phy: Add USB3 PHY support for Broadcom NSP SoC"") this module should be modified to use MDIO bus as this is how PHY is really attached. This should allow reusing this driver on NSP and any other platform with MDIO bus and this particular PHY. This is material for the next merge window (4.13 kernel) obviously. Rafał Miłecki (5): phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg phy: bcm-ns-usb3: use pointer for PHY writing function phy: bcm-ns-usb3: enable MDIO in the platform specific code dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO phy: bcm-ns-usb3: add MDIO driver using proper bus layer .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt| 27 ++- drivers/phy/Kconfig| 1 + drivers/phy/phy-bcm-ns-usb3.c | 223 +++-- 3 files changed, 179 insertions(+), 72 deletions(-) -- 2.11.0
[PATCH 2/5] phy: bcm-ns-usb3: use pointer for PHY writing function
From: Rafał Miłecki Our current writing function accesses PHY directly bypassing MDIO layer. The aim is to extend this module to also behave as MDIO driver. This will require using different writing function which can be handled cleanly by having an extra pointer like this. Signed-off-by: Rafał Miłecki --- drivers/phy/phy-bcm-ns-usb3.c | 98 --- 1 file changed, 56 insertions(+), 42 deletions(-) diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c index 5e89326886dc..3d0fe5728029 100644 --- a/drivers/phy/phy-bcm-ns-usb3.c +++ b/drivers/phy/phy-bcm-ns-usb3.c @@ -53,6 +53,8 @@ struct bcm_ns_usb3 { void __iomem *dmp; void __iomem *ccb_mii; struct phy *phy; + + int (*phy_write)(struct bcm_ns_usb3 *usb3, u16 reg, u16 value); }; static const struct of_device_id bcm_ns_usb3_id_table[] = { @@ -68,51 +70,10 @@ static const struct of_device_id bcm_ns_usb3_id_table[] = { }; MODULE_DEVICE_TABLE(of, bcm_ns_usb3_id_table); -static int bcm_ns_usb3_wait_reg(struct bcm_ns_usb3 *usb3, void __iomem *addr, - u32 mask, u32 value, unsigned long timeout) -{ - unsigned long deadline = jiffies + timeout; - u32 val; - - do { - val = readl(addr); - if ((val & mask) == value) - return 0; - cpu_relax(); - udelay(10); - } while (!time_after_eq(jiffies, deadline)); - - dev_err(usb3->dev, "Timeout waiting for register %p\n", addr); - - return -EBUSY; -} - -static inline int bcm_ns_usb3_mii_mng_wait_idle(struct bcm_ns_usb3 *usb3) -{ - return bcm_ns_usb3_wait_reg(usb3, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL, - 0x0100, 0x, - usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US)); -} - static int bcm_ns_usb3_mdio_phy_write(struct bcm_ns_usb3 *usb3, u16 reg, u16 value) { - u32 tmp = 0; - int err; - - err = bcm_ns_usb3_mii_mng_wait_idle(usb3); - if (err < 0) { - dev_err(usb3->dev, "Couldn't write 0x%08x value\n", value); - return err; - } - - /* TODO: Use a proper MDIO bus layer */ - tmp |= 0x5802; /* Magic value for MDIO PHY write */ - tmp |= reg << 18; - tmp |= value; - writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA); - - return bcm_ns_usb3_mii_mng_wait_idle(usb3); + return usb3->phy_write(usb3, reg, value); } static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3) @@ -233,6 +194,57 @@ static const struct phy_ops ops = { .owner = THIS_MODULE, }; +/** + * Platform driver code + **/ + +static int bcm_ns_usb3_wait_reg(struct bcm_ns_usb3 *usb3, void __iomem *addr, + u32 mask, u32 value, unsigned long timeout) +{ + unsigned long deadline = jiffies + timeout; + u32 val; + + do { + val = readl(addr); + if ((val & mask) == value) + return 0; + cpu_relax(); + udelay(10); + } while (!time_after_eq(jiffies, deadline)); + + dev_err(usb3->dev, "Timeout waiting for register %p\n", addr); + + return -EBUSY; +} + +static inline int bcm_ns_usb3_mii_mng_wait_idle(struct bcm_ns_usb3 *usb3) +{ + return bcm_ns_usb3_wait_reg(usb3, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL, + 0x0100, 0x, + usecs_to_jiffies(BCM_NS_USB3_MII_MNG_TIMEOUT_US)); +} + +static int bcm_ns_usb3_platform_phy_write(struct bcm_ns_usb3 *usb3, u16 reg, + u16 value) +{ + u32 tmp = 0; + int err; + + err = bcm_ns_usb3_mii_mng_wait_idle(usb3); + if (err < 0) { + dev_err(usb3->dev, "Couldn't write 0x%08x value\n", value); + return err; + } + + /* TODO: Use a proper MDIO bus layer */ + tmp |= 0x5802; /* Magic value for MDIO PHY write */ + tmp |= reg << 18; + tmp |= value; + writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA); + + return bcm_ns_usb3_mii_mng_wait_idle(usb3); +} + static int bcm_ns_usb3_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -266,6 +278,8 @@ static int bcm_ns_usb3_probe(struct platform_device *pdev) return PTR_ERR(usb3->ccb_mii); } + usb3->phy_write = bcm_ns_usb3_platform_phy_write; + usb3->phy = devm_phy_create(dev, NULL, &ops); if (IS_ERR(usb3->phy)) { dev_err(dev, "Failed to create PHY\n"); -- 2.11.0
[PATCH 5/5] phy: bcm-ns-usb3: add MDIO driver using proper bus layer
From: Rafał Miłecki As USB 3.0 PHY is attached to the MDIO bus this module should provide a MDIO driver and use a proper bus layer. This is a proper (cleaner) solution which doesn't require code to know this specific MDIO bus details. It also allows reusing the driver with other MDIO buses. Signed-off-by: Rafał Miłecki --- drivers/phy/Kconfig | 1 + drivers/phy/phy-bcm-ns-usb3.c | 98 ++- 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index afaf7b643eeb..2a9186b98ae0 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -29,6 +29,7 @@ config PHY_BCM_NS_USB3 depends on ARCH_BCM_IPROC || COMPILE_TEST depends on HAS_IOMEM && OF select GENERIC_PHY + select PHYLIB help Enable this to support Broadcom USB 3.0 PHY connected to the USB controller on Northstar family. diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c index 2c9a0d5f43d8..389f5e5a6238 100644 --- a/drivers/phy/phy-bcm-ns-usb3.c +++ b/drivers/phy/phy-bcm-ns-usb3.c @@ -16,7 +16,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -52,6 +54,7 @@ struct bcm_ns_usb3 { enum bcm_ns_family family; void __iomem *dmp; void __iomem *ccb_mii; + struct mdio_device *mdiodev; struct phy *phy; int (*phy_write)(struct bcm_ns_usb3 *usb3, u16 reg, u16 value); @@ -183,6 +186,77 @@ static const struct phy_ops ops = { }; /** + * MDIO driver code + **/ + +static int bcm_ns_usb3_mdiodev_phy_write(struct bcm_ns_usb3 *usb3, u16 reg, +u16 value) +{ + struct mdio_device *mdiodev = usb3->mdiodev; + + return mdiobus_write(mdiodev->bus, mdiodev->addr, reg, value); +} + +static int bcm_ns_usb3_mdio_probe(struct mdio_device *mdiodev) +{ + struct device *dev = &mdiodev->dev; + const struct of_device_id *of_id; + struct phy_provider *phy_provider; + struct device_node *syscon_np; + struct bcm_ns_usb3 *usb3; + struct resource res; + int err; + + usb3 = devm_kzalloc(dev, sizeof(*usb3), GFP_KERNEL); + if (!usb3) + return -ENOMEM; + + usb3->dev = dev; + usb3->mdiodev = mdiodev; + + of_id = of_match_device(bcm_ns_usb3_id_table, dev); + if (!of_id) + return -EINVAL; + usb3->family = (enum bcm_ns_family)of_id->data; + + syscon_np = of_parse_phandle(dev->of_node, "usb3-dmp-syscon", 0); + err = of_address_to_resource(syscon_np, 0, &res); + of_node_put(syscon_np); + if (err) + return err; + + usb3->dmp = devm_ioremap_resource(dev, &res); + if (IS_ERR(usb3->dmp)) { + dev_err(dev, "Failed to map DMP regs\n"); + return PTR_ERR(usb3->dmp); + } + + usb3->phy_write = bcm_ns_usb3_mdiodev_phy_write; + + usb3->phy = devm_phy_create(dev, NULL, &ops); + if (IS_ERR(usb3->phy)) { + dev_err(dev, "Failed to create PHY\n"); + return PTR_ERR(usb3->phy); + } + + phy_set_drvdata(usb3->phy, usb3); + + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); + + return PTR_ERR_OR_ZERO(phy_provider); +} + +static struct mdio_driver bcm_ns_usb3_mdio_driver = { + .mdiodrv = { + .driver = { + .name = "bcm_ns_mdio_usb3", + .of_match_table = bcm_ns_usb3_id_table, + }, + }, + .probe = bcm_ns_usb3_mdio_probe, +}; + +/** * Platform driver code **/ @@ -297,6 +371,28 @@ static struct platform_driver bcm_ns_usb3_driver = { .of_match_table = bcm_ns_usb3_id_table, }, }; -module_platform_driver(bcm_ns_usb3_driver); + +static int __init bcm_ns_usb3_module_init(void) +{ + int err; + + err = mdio_driver_register(&bcm_ns_usb3_mdio_driver); + if (err) + return err; + + err = platform_driver_register(&bcm_ns_usb3_driver); + if (err) + mdio_driver_unregister(&bcm_ns_usb3_mdio_driver); + + return err; +} +module_init(bcm_ns_usb3_module_init); + +static void __exit bcm_ns_usb3_module_exit(void) +{ + platform_driver_unregister(&bcm_ns_usb3_driver); + mdio_driver_unregister(&bcm_ns_usb3_mdio_driver); +} +module_exit(bcm_ns_usb3_module_exit) MODULE_LICENSE("GPL v2"); -- 2.11.0
[PATCH 4/5] dt-bindings: phy: Modify Broadcom NS USB 3.0 PHY binding to use MDIO
From: Rafał Miłecki Thanks to work done by Broadcom explaining their USB 3.0 PHY details we know it's attached to the MDIO bus. Use this knowledge to update the binding: make it a subnode to the MDIO bus and rework way of specifying required registers. Signed-off-by: Rafał Miłecki --- .../devicetree/bindings/phy/bcm-ns-usb3-phy.txt| 27 +++--- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt index 09aeba94538d..32f057260351 100644 --- a/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt +++ b/Documentation/devicetree/bindings/phy/bcm-ns-usb3-phy.txt @@ -3,9 +3,10 @@ Driver for Broadcom Northstar USB 3.0 PHY Required properties: - compatible: one of: "brcm,ns-ax-usb3-phy", "brcm,ns-bx-usb3-phy". -- reg: register mappings for DMP (Device Management Plugin) and ChipCommon B - MMI. -- reg-names: "dmp" and "ccb-mii" +- reg: address of MDIO bus device +- usb3-dmp-syscon: phandle to syscon with DMP (Device Management Plugin) + registers +- #phy-cells: must be 0 Initialization of USB 3.0 PHY depends on Northstar version. There are currently three known series: Ax, Bx and Cx. @@ -15,9 +16,19 @@ Known B1: BCM4707 rev 6 Known C0: BCM47094 rev 0 Example: - usb3-phy { - compatible = "brcm,ns-ax-usb3-phy"; - reg = <0x18105000 0x1000>, <0x18003000 0x1000>; - reg-names = "dmp", "ccb-mii"; - #phy-cells = <0>; + mdio: mdio@0 { + reg = <0x0>; + #size-cells = <1>; + #address-cells = <0>; + + usb3-phy@10 { + compatible = "brcm,ns-ax-usb3-phy"; + reg = <0x10>; + usb3-dmp-syscon = <&usb3_dmp>; + #phy-cells = <0>; + }; + }; + + usb3_dmp: syscon@18105000 { + reg = <0x18105000 0x1000>; }; -- 2.11.0
[PATCH 1/5] phy: bcm-ns-usb3: always wait for idle after writing to the PHY reg
From: Rafał Miłecki Move MDIO specific code to the writing helper function. This makes init code a bit more generic and doesn't require it to track what happens after every write. Signed-off-by: Rafał Miłecki --- drivers/phy/phy-bcm-ns-usb3.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c index 22b5e7047fa6..5e89326886dc 100644 --- a/drivers/phy/phy-bcm-ns-usb3.c +++ b/drivers/phy/phy-bcm-ns-usb3.c @@ -112,7 +112,7 @@ static int bcm_ns_usb3_mdio_phy_write(struct bcm_ns_usb3 *usb3, u16 reg, tmp |= value; writel(tmp, usb3->ccb_mii + BCMA_CCB_MII_MNG_CMD_DATA); - return 0; + return bcm_ns_usb3_mii_mng_wait_idle(usb3); } static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3) @@ -143,9 +143,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3) /* Deaaserting PLL Reset */ bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PLLA_CONTROL1, 0x8000); - /* Waiting MII Mgt interface idle */ - bcm_ns_usb3_mii_mng_wait_idle(usb3); - /* Deasserting USB3 system reset */ writel(0, usb3->dmp + BCMA_RESET_CTL); @@ -169,9 +166,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3) /* Enabling SSC */ bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_TX_PMD_CONTROL1, 0x1003); - /* Waiting MII Mgt interface idle */ - bcm_ns_usb3_mii_mng_wait_idle(usb3); - return 0; } @@ -205,9 +199,6 @@ static int bcm_ns_usb3_phy_init_ns_ax(struct bcm_ns_usb3 *usb3) bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_TX_PMD_CONTROL1, 0x1003); - /* Waiting MII Mgt interface idle */ - bcm_ns_usb3_mii_mng_wait_idle(usb3); - /* Deasserting USB3 system reset */ writel(0, usb3->dmp + BCMA_RESET_CTL); -- 2.11.0
[PATCH 3/5] phy: bcm-ns-usb3: enable MDIO in the platform specific code
From: Rafał Miłecki When we finally start using MDIO layer then bus initialization will be handled in a separated driver. It means our code handling this has to be used for the platform driver only. Signed-off-by: Rafał Miłecki --- drivers/phy/phy-bcm-ns-usb3.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/phy/phy-bcm-ns-usb3.c b/drivers/phy/phy-bcm-ns-usb3.c index 3d0fe5728029..2c9a0d5f43d8 100644 --- a/drivers/phy/phy-bcm-ns-usb3.c +++ b/drivers/phy/phy-bcm-ns-usb3.c @@ -80,12 +80,6 @@ static int bcm_ns_usb3_phy_init_ns_bx(struct bcm_ns_usb3 *usb3) { int err; - /* Enable MDIO. Setting MDCDIV as 26 */ - writel(0x009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL); - - /* Wait for MDIO? */ - udelay(2); - /* USB3 PLL Block */ err = bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PHY_BASE_ADDR_REG, BCM_NS_USB3_PHY_PLL30_BLOCK); @@ -134,12 +128,6 @@ static int bcm_ns_usb3_phy_init_ns_ax(struct bcm_ns_usb3 *usb3) { int err; - /* Enable MDIO. Setting MDCDIV as 26 */ - writel(0x009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL); - - /* Wait for MDIO? */ - udelay(2); - /* PLL30 block */ err = bcm_ns_usb3_mdio_phy_write(usb3, BCM_NS_USB3_PHY_BASE_ADDR_REG, BCM_NS_USB3_PHY_PLL30_BLOCK); @@ -278,6 +266,12 @@ static int bcm_ns_usb3_probe(struct platform_device *pdev) return PTR_ERR(usb3->ccb_mii); } + /* Enable MDIO. Setting MDCDIV as 26 */ + writel(0x009a, usb3->ccb_mii + BCMA_CCB_MII_MNG_CTL); + + /* Wait for MDIO? */ + udelay(2); + usb3->phy_write = bcm_ns_usb3_platform_phy_write; usb3->phy = devm_phy_create(dev, NULL, &ops); -- 2.11.0
Re: [REGRESSION] linux-next panics when trying to mount root
On 2018-01-16 16:30, Boris Brezillon wrote: On Tue, 16 Jan 2018 16:02:35 +0100 Peter Rosin wrote: On 2018-01-16 15:21, Boris Brezillon wrote: > On Tue, 16 Jan 2018 14:56:52 +0100 > Peter Rosin wrote: > >> Hmmm, I guess the question is if the command line should override the >> device tree or not? > > Yep, that's the problem. Now the core parses the compatible to decides > which part parser should be used. The thing is, the "cmdline" parser is > not yet exposing a compatible id, and even if it was, this would > require patching all DTs to add this new compatible. > >partitions { >compatible = "cmdline", "fixed-partitions"; >... >}; > > Not really an option, so I'll drop the 2 patches for now until we find a > better solution. > >> I'm going to send a patch for the above dts change either way... > > If you want, but that does not solve the problem: we should not break > existing users. Well, don't let these devices stop you, they will not get a new kernel w/o also getting a new dtb, and I can handle this just fine. But maybe I'm just the first reporter and you'd rather not risk anything? Anyway, just wanted to let you know where I stand... Unfortunately, at91 is not the only platform to have "fixed-partitions" defined in its DTs, and I guess users of other platforms also like to override the default MTD layout by their own using mtdparts.I'd like to find a solution that keeps everyone happy. I absolutely agree with Boris, we can't risk such regressions. That had to be dropped and I'm looking for a better solution.
Re: [REGRESSION] linux-next panics when trying to mount root
failed to initialize mode setting ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver ehci-atmel: EHCI Atmel driver atmel_usba_udc 50.gadget: MMIO registers at 0xf803 mapped at ff1022ba atmel_usba_udc 50.gadget: FIFO at 0x0050 mapped at 3b2532b1 g_serial gadget: Gadget Serial v2.4 g_serial gadget: g_serial ready at91_rtc feb0.rtc: registered as rtc0 at91_rtc feb0.rtc: AT91 Real Time Clock driver. i2c /dev entries driver AT91: Starting after wakeup atmel_mci f000.mmc: version: 0x505 atmel_mci f000.mmc: using dma0chan2 for DMA transfers atmel_mci f000.mmc: Atmel MCI controller at 0xf000 irq 18, 1 slots atmel_aes f8038000.aes: version: 0x135 atmel_aes f8038000.aes: Atmel AES - Using dma1chan0, dma1chan1 for DMA transfers atmel_sha f8034000.sha: version: 0x410 atmel_sha f8034000.sha: using dma1chan2 for DMA transfers atmel_sha f8034000.sha: Atmel SHA1/SHA256/SHA224/SHA384/SHA512 atmel_tdes f803c000.tdes: version: 0x701 atmel_tdes f803c000.tdes: using dma1chan3, dma1chan4 for DMA transfers atmel_tdes f803c000.tdes: Atmel DES/TDES usbcore: registered new interface driver usbhid usbhid: USB HID core driver nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xac nand: Micron MT29F4G08ABBDAHC nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64 Bad block table found at page 262080, version 0x01 Bad block table found at page 262016, version 0x01 6 ofpart partitions found on MTD device atmel_nand Creating 6 MTD partitions on "atmel_nand": 0x-0x0004 : "at91bootstrap" 0x0004-0x000c : "bootloader" 0x000c-0x0018 : "bootloader env" 0x0018-0x0020 : "device tree" 0x0020-0x0080 : "kernel" 0x0080-0x1000 : "rootfs" NET: Registered protocol family 10 Segment Routing with IPv6 sit: IPv6, IPv4 and MPLS over IPv4 tunneling driver NET: Registered protocol family 17 Loading compiled-in X.509 certificates [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [drm] No driver support for vblank timestamp query. atmel-hlcdc-display-controller atmel-hlcdc-dc: fb0: frame buffer device [drm] Initialized atmel-hlcdc 1.0.0 20141504 for atmel-hlcdc-dc on minor 0 UBI error: cannot open mtd 6, error -19 input: gpio-keys as /devices/platform/gpio-keys/input/input0 at91_rtc feb0.rtc: setting system clock to 2007-01-01 00:00:08 UTC (1167609608) panel-VCC: disabling panel-VDD: disabling atmel_usart ee00.serial: using dma1chan5 for rx DMA transfers Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) CPU: 0 PID: 1 Comm: swapper Not tainted 4.15.0-rc8-next-20180116 #208 Hardware name: Atmel SAMA5 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (panic+0xc8/0x254) [] (panic) from [] (mount_block_root+0x23c/0x2f8) [] (mount_block_root) from [] (prepare_namespace+0xa8/0x19c) [] (prepare_namespace) from [] (kernel_init_freeable+0x1bc/0x1cc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x10c) [] (kernel_init) from [] (ret_from_fork+0x14/0x2c) Exception stack(0xc3821fb0 to 0xc3821ff8) 1fa0: 1fc0: 1fe0: 0013 ---[ end Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) I bisected it to: 4ac9222778478a00c7fc9d347b7ed1e0e595120d is the first bad commit commit 4ac9222778478a00c7fc9d347b7ed1e0e595120d Author: Rafał Miłecki Date: Thu Jan 4 08:05:34 2018 +0100 mtd: ofpart: add of_match_table with "fixed-partitions" This allows using this parser with any flash driver that takes care of setting of_node (using mtd_set_of_node helper) correctly. Up to now support for "fixed-partitions" DT compatibility string was working only with flash drivers that were specifying "ofpart" (manually or by letting mtd use the default set of parsers). This matches existing bindings documentation. Signed-off-by: Rafał Miłecki Reviewed-by: Brian Norris Tested-by: Brian Norris Signed-off-by: Boris Brezillon Reverting that patch on top of next-20180116 fixes the problem. I'm using the arch/arm/boot/dts/at91-nattis-2-natte-2.dts device tree which for the record is kind of new in the upstream kernel, but I have been using that dts for a while previously. However, I'd put my money on at91-tse850-3.dts also being affected, and that one have been upstream for a while. Haven't tested the tse850 device tree though... Peter, above patch was dropped, together with the one adding support for the "of_match_table". I just sent V8 of my changes, hopefully they don't cause regression this time. Would you be able to test
Re: [PATCH] ARM: BCM5301X: Fix NAND ECC parameters for Linksys Panamera
On 10 March 2018 at 18:12, Vivek Unune wrote: > Using BCH8 gives ecc errors and makes the router unsuable. > Switching to BCH1 fixes these errors. Can you provide CFE's log messages starting with "Decompressing...done" and up to the "Press Ctrl+C to stop in CFE" please? I'd like to see what NAND info CFE prints there.
Re: [PATCH] ARM: dts: BCM5301X: add missing LEDs for Buffalo WZR-900DHP
On 5 March 2018 at 15:36, wrote: > From: INAGAKI Hiroshi > > Buffalo WZR-900DHP has 8 LEDs, but there is not LED definitions in the > dts and cannot configure these LEDs. > I Added missing LED definitions for WZR-900DHP. > > Signed-off-by: INAGAKI Hiroshi Looks almost good, thanks for sending this patch to Florian. One comment: please drop all linux,default-trigger = "default-off"; lines. I did the same mistake long time ago, see my fixing commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0b660259e927177dc0c6eb3b1c39f23c6a011c5f When you fix that, please generate a V2 of your patch, you can do it with something like: git format-patch --subject-prefix="PATCH V2" -1 and send again the same way. Thanks!
Re: [PATCH V2] ARM: dts: BCM5301X: add missing LEDs for Buffalo WZR-900DHP
On 7 March 2018 at 12:33, wrote: > From: INAGAKI Hiroshi > > Buffalo WZR-900DHP has 8 LEDs, but there is not LED definitions in the > dts and cannot configure these LEDs. > I Added missing LED definitions for WZR-900DHP. > > Signed-off-by: INAGAKI Hiroshi Nice work, thanks! Reviewed-by: Rafał Miłecki
Re: [QUESTION] Mainline support for B43_PHY_AC wifi cards
On 23 March 2018 at 15:09, Juri Lelli wrote: > On 23/03/18 14:43, Rafał Miłecki wrote: >> Hi, >> >> On 23 March 2018 at 10:47, Juri Lelli wrote: >> > I've got a Dell XPS 13 9343/0TM99H (BIOS A15 01/23/2018) mounting a >> > BCM4352 802.11ac (rev 03) wireless card and so far I've been using it on >> > Fedora with broadcom-wl package (which I believe installs Broadcom's STA >> > driver?). It works good apart from occasional hiccups after suspend. >> > >> > I'd like to get rid of that dependency (you can understand that it's >> > particularly annoying when testing mainline kernels), but I found out >> > that support for my card is BROKEN in mainline [1]. Just to see what >> > happens, I forcibly enabled it witnessing that it indeed crashes like >> > below as Kconfig warns. :) >> > >> > bcma: bus0: Found chip with id 0x4352, rev 0x03 and package 0x00 >> > bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x2B, >> > class 0x0) >> > bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x2A, >> > class 0x0) >> > bcma: bus0: Core 2 found: ARM CR4 (manuf 0x4BF, id 0x83E, rev 0x02, class >> > 0x0) >> > bcma: bus0: Core 3 found: PCIe Gen2 (manuf 0x4BF, id 0x83C, rev 0x01, >> > class 0x0) >> > bcma: bus0: Core 4 found: USB 2.0 Device (manuf 0x4BF, id 0x81A, rev >> > 0x11, class 0x0) >> > bcma: Unsupported SPROM revision: 11 >> > bcma: bus0: Invalid SPROM read from the PCIe card, trying to use fallback >> > SPROM >> > bcma: bus0: Using fallback SPROM failed (err -2) >> > bcma: bus0: No SPROM available >> > bcma: bus0: Bus registered >> > b43-phy0: Broadcom 4352 WLAN found (core revision 42) >> > b43-phy0: Found PHY: Analog 12, Type 11 (AC), Revision 1 >> > b43-phy0: Found Radio: Manuf 0x17F, ID 0x2069, Revision 4, Version 0 >> > BUG: unable to handle kernel NULL pointer dereference at >> >> This isn't really useful without a full backtrace. > > Sure. I cut it here because I didn't expect people to debug what is > already known to be broken (but still it seemed to carry useful > information about the hw). :) Please paste the remaining part if you still got it. >> > So, question: is replacing my card the only way I can get rid of this >> > downstream dependency? :( >> >> It's definitely the cheapest way. Getting AC PHY into anything usable >> (proper setup that will allow Tx & Rx anything) would probably take >> weeks or months of development. I'm not even going to estimate cost of >> adding support for 802.11n and 802.11ac features. I was the last >> person actively working on b43, right now I spend my free time on >> other hobby projects. Few people were planning to help but it seems it >> never worked out for them. > > I see. Just wondering why even if Broadcom's STA solution seems to work > fine, it is not mainline. Maybe a maintenance problem? But Fedora ships > with very recent kernels, so I'd expect the driver to work with mainline > (I tried compiling that against mainline, but I got errors that I didn't > spend time figuring out how to fix). > > Do you know what's the deal w.r.t. the STA driver? Driver being closed source and company not willing to open source it is usually a big problem getting it mainline... -- Rafał
[PATCH 2/2] firmware: bcm47xx_nvram: support small (0x6000 B) NVRAM partitions
From: Rafał Miłecki Some old devices with 4 MiB flashes were using 0x1000 block size and could use smaller (0x6000 bytes) flash partition for storing NVRAM content. This adds support for reading NVRAM on Netgear WNR1000 V3. Signed-off-by: Rafał Miłecki --- drivers/firmware/broadcom/bcm47xx_nvram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/broadcom/bcm47xx_nvram.c b/drivers/firmware/broadcom/bcm47xx_nvram.c index 0b631e5b5b84..d25f080fcb0d 100644 --- a/drivers/firmware/broadcom/bcm47xx_nvram.c +++ b/drivers/firmware/broadcom/bcm47xx_nvram.c @@ -36,7 +36,7 @@ struct nvram_header { static char nvram_buf[NVRAM_SPACE]; static size_t nvram_len; -static const u32 nvram_sizes[] = {0x8000, 0xF000, 0x1}; +static const u32 nvram_sizes[] = {0x6000, 0x8000, 0xF000, 0x1}; static u32 find_nvram_size(void __iomem *end) { -- 2.11.0
[PATCH 1/2] MIPS: BCM47XX: Add support for Netgear WNR1000 V3
From: Rafał Miłecki This adds support for detecting this model board and registers some LEDs and buttons. There are two uncommon things regarding this device: 1) It can use two different "board_id" ID values. Unit I have uses "U12H139T00_NETGEAR" value. This magic is also used in firmware file header. There are two reports (one from an OpenWrt user) of a different "U12H139T50_NETGEAR" magic though. 2) Power LEDs share GPIOs with buttons. Amber one seems to share GPIO 2 with WPS button and green one seems to share GPIO 3 with reset button. It remains unknown how to support them and handle buttons at the same time. For that reason they aren't added to the list of supported LEDs. Signed-off-by: Rafał Miłecki --- arch/mips/bcm47xx/board.c | 2 ++ arch/mips/bcm47xx/buttons.c| 9 + arch/mips/bcm47xx/leds.c | 9 + arch/mips/include/asm/mach-bcm47xx/bcm47xx_board.h | 1 + 4 files changed, 21 insertions(+) diff --git a/arch/mips/bcm47xx/board.c b/arch/mips/bcm47xx/board.c index edfaef0d73a4..a80910d2738c 100644 --- a/arch/mips/bcm47xx/board.c +++ b/arch/mips/bcm47xx/board.c @@ -172,6 +172,8 @@ struct bcm47xx_board_type_list1 bcm47xx_board_list_board_id[] __initconst = { {{BCM47XX_BOARD_NETGEAR_WNDR4000, "Netgear WNDR4000"}, "U12H181T00_NETGEAR"}, {{BCM47XX_BOARD_NETGEAR_WNDR4500V1, "Netgear WNDR4500 V1"}, "U12H189T00_NETGEAR"}, {{BCM47XX_BOARD_NETGEAR_WNDR4500V2, "Netgear WNDR4500 V2"}, "U12H224T00_NETGEAR"}, + {{BCM47XX_BOARD_NETGEAR_WNR1000_V3, "Netgear WNR1000 V3"}, "U12H139T00_NETGEAR"}, + {{BCM47XX_BOARD_NETGEAR_WNR1000_V3, "Netgear WNR1000 V3"}, "U12H139T50_NETGEAR"}, {{BCM47XX_BOARD_NETGEAR_WNR2000, "Netgear WNR2000"}, "U12H114T00_NETGEAR"}, {{BCM47XX_BOARD_NETGEAR_WNR3500L, "Netgear WNR3500L"}, "U12H136T99_NETGEAR"}, {{BCM47XX_BOARD_NETGEAR_WNR3500U, "Netgear WNR3500U"}, "U12H136T00_NETGEAR"}, diff --git a/arch/mips/bcm47xx/buttons.c b/arch/mips/bcm47xx/buttons.c index 88d400d256c4..977990a609ba 100644 --- a/arch/mips/bcm47xx/buttons.c +++ b/arch/mips/bcm47xx/buttons.c @@ -412,6 +412,12 @@ bcm47xx_buttons_netgear_wndr4500v1[] __initconst = { }; static const struct gpio_keys_button +bcm47xx_buttons_netgear_wnr1000_v3[] __initconst = { + BCM47XX_GPIO_KEY(2, KEY_WPS_BUTTON), + BCM47XX_GPIO_KEY(3, KEY_RESTART), +}; + +static const struct gpio_keys_button bcm47xx_buttons_netgear_wnr3500lv1[] __initconst = { BCM47XX_GPIO_KEY(4, KEY_RESTART), BCM47XX_GPIO_KEY(6, KEY_WPS_BUTTON), @@ -670,6 +676,9 @@ int __init bcm47xx_buttons_register(void) case BCM47XX_BOARD_NETGEAR_WNDR4500V1: err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wndr4500v1); break; + case BCM47XX_BOARD_NETGEAR_WNR1000_V3: + err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wnr1000_v3); + break; case BCM47XX_BOARD_NETGEAR_WNR3500L: err = bcm47xx_copy_bdata(bcm47xx_buttons_netgear_wnr3500lv1); break; diff --git a/arch/mips/bcm47xx/leds.c b/arch/mips/bcm47xx/leds.c index 34a7b3fbdfd9..3fe015602945 100644 --- a/arch/mips/bcm47xx/leds.c +++ b/arch/mips/bcm47xx/leds.c @@ -498,6 +498,12 @@ bcm47xx_leds_netgear_wndr4500v1[] __initconst = { }; static const struct gpio_led +bcm47xx_leds_netgear_wnr1000_v3[] __initconst = { + BCM47XX_GPIO_LED(0, "blue", "wlan", 0, LEDS_GPIO_DEFSTATE_OFF), + BCM47XX_GPIO_LED(1, "green", "wps", 0, LEDS_GPIO_DEFSTATE_OFF), +}; + +static const struct gpio_led bcm47xx_leds_netgear_wnr3500lv1[] __initconst = { BCM47XX_GPIO_LED(0, "blue", "wlan", 1, LEDS_GPIO_DEFSTATE_OFF), BCM47XX_GPIO_LED(1, "green", "wps", 1, LEDS_GPIO_DEFSTATE_OFF), @@ -758,6 +764,9 @@ void __init bcm47xx_leds_register(void) case BCM47XX_BOARD_NETGEAR_WNDR4500V1: bcm47xx_set_pdata(bcm47xx_leds_netgear_wndr4500v1); break; + case BCM47XX_BOARD_NETGEAR_WNR1000_V3: + bcm47xx_set_pdata(bcm47xx_leds_netgear_wnr1000_v3); + break; case BCM47XX_BOARD_NETGEAR_WNR3500L: bcm47xx_set_pdata(bcm47xx_leds_netgear_wnr3500lv1); break; diff --git a/arch/mips/include/asm/mach-bcm47xx/bcm47xx_board.h b/arch/mips/include/asm/mach-bcm47xx/bcm47xx_board.h index cbf9da7f2f94..0ef8893e07f8 100644 --- a/arch/mips/include/asm/mach-bcm47xx/bcm47xx_board.h +++ b/arch/mips/include/asm/mach-bcm47xx/bcm47xx_board.h @@ -110,6 +110,7 @@ enum bcm47xx_board { BCM47XX_BOARD_NETGEAR_WNDR4000, BCM47XX_BOARD_NETGEAR_WNDR4500V1, BCM47XX_BOARD_NETGEAR_WNDR4500V2, + BCM47XX_BOARD_NETGEAR_WNR1000_V3, BCM47XX_BOARD_NETGEAR_WNR2000, BCM47XX_BOARD_NETGEAR_WNR3500L, BCM47XX_BOARD_NETGEAR_WNR3500U, -- 2.11.0
Re: [QUESTION] Mainline support for B43_PHY_AC wifi cards
Hi, On 23 March 2018 at 10:47, Juri Lelli wrote: > I've got a Dell XPS 13 9343/0TM99H (BIOS A15 01/23/2018) mounting a > BCM4352 802.11ac (rev 03) wireless card and so far I've been using it on > Fedora with broadcom-wl package (which I believe installs Broadcom's STA > driver?). It works good apart from occasional hiccups after suspend. > > I'd like to get rid of that dependency (you can understand that it's > particularly annoying when testing mainline kernels), but I found out > that support for my card is BROKEN in mainline [1]. Just to see what > happens, I forcibly enabled it witnessing that it indeed crashes like > below as Kconfig warns. :) > > bcma: bus0: Found chip with id 0x4352, rev 0x03 and package 0x00 > bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x2B, class > 0x0) > bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x2A, > class 0x0) > bcma: bus0: Core 2 found: ARM CR4 (manuf 0x4BF, id 0x83E, rev 0x02, class > 0x0) > bcma: bus0: Core 3 found: PCIe Gen2 (manuf 0x4BF, id 0x83C, rev 0x01, class > 0x0) > bcma: bus0: Core 4 found: USB 2.0 Device (manuf 0x4BF, id 0x81A, rev 0x11, > class 0x0) > bcma: Unsupported SPROM revision: 11 > bcma: bus0: Invalid SPROM read from the PCIe card, trying to use fallback > SPROM > bcma: bus0: Using fallback SPROM failed (err -2) > bcma: bus0: No SPROM available > bcma: bus0: Bus registered > b43-phy0: Broadcom 4352 WLAN found (core revision 42) > b43-phy0: Found PHY: Analog 12, Type 11 (AC), Revision 1 > b43-phy0: Found Radio: Manuf 0x17F, ID 0x2069, Revision 4, Version 0 > BUG: unable to handle kernel NULL pointer dereference at This isn't really useful without a full backtrace. > So, question: is replacing my card the only way I can get rid of this > downstream dependency? :( It's definitely the cheapest way. Getting AC PHY into anything usable (proper setup that will allow Tx & Rx anything) would probably take weeks or months of development. I'm not even going to estimate cost of adding support for 802.11n and 802.11ac features. I was the last person actively working on b43, right now I spend my free time on other hobby projects. Few people were planning to help but it seems it never worked out for them.
Re: [PATCH v2] ubifs: Handle re-linking of inodes correctly while recovery
On Wed, 7 Nov 2018 at 23:04, Richard Weinberger wrote: > UBIFS's recovery code strictly assumes that a deleted inode will never > come back, therefore it removes all data which belongs to that inode > as soon it faces an inode with link count 0 in the replay list. > Before O_TMPFILE this assumption was perfectly fine. With O_TMPFILE > it can lead to data loss upon a power-cut. > > Consider a journal with entries like: > 0: inode X (nlink = 0) /* O_TMPFILE was created */ > 1: data for inode X /* Someone writes to the temp file */ > 2: inode X (nlink = 0) /* inode was changed, xattr, chmod, … */ > 3: inode X (nlink = 1) /* inode was re-linked via linkat() */ > > Upon replay of entry #2 UBIFS will drop all data that belongs to inode X, > this will lead to an empty file after mounting. > > As solution for this problem, scan the replay list for a re-link entry > before dropping data. > > Fixes: 474b93704f32 ("ubifs: Implement O_TMPFILE") > Cc: sta...@vger.kernel.org > Cc: Russell Senior > Cc: Rafał Miłecki > Reported-by: Russell Senior > Reported-by: Rafał Miłecki > Signed-off-by: Richard Weinberger Thank you!!! Tested-by: Rafał Miłecki
Re: [PATCH] leds: gpio: Drop unneeded manual of_node assignment
On 2018-12-07 11:10, Krzysztof Kozlowski wrote: This reverts the main change of commit bff23714bc36 ("leds: leds-gpio: Set of_node for created LED devices") because of_node assignment is handled by core since commit 7ea79ae86c28 ("leds: gpio: use OF variant of LED registering function"). Basically the code was overwriting the of_node with same value. No functional change expected. Signed-off-by: Krzysztof Kozlowski Thanks! Tested-by: Rafał Miłecki
Re: [PATCH v3] mtd: spi-nor: cast to u64 to avoid uint overflows
On Wed, 28 Nov 2018 at 09:03, Huijin Park wrote: > From: "huijin.park" > > The "params->size" is defined as "u64". > And "info->sector_size" and "info->n_sectors" are defined as > unsigned int and u16. > Thus, u64 data might have strange data(loss data) if the result > overflows an unsigned int. > This patch casts "info->sector_size" to an u64. > > Signed-off-by: huijin.park You may want to adjust your git's "user.name" config to avoid "malforming" your name (From and Signed-off-by) in the further contributions :) Something like git config --global user.name "John Foo"
Re: [PATCH 3.16 151/366] MIPS: BCM47XX: Enable 74K Core ExternalSync for PCIe erratum
On Sun, 11 Nov 2018 at 21:05, Ben Hutchings wrote: > 3.16.61-rc1 review patch. If anyone has any objections, please let me know. Nack. This patch has caused a regression and had to be reverted. Please check upstream repository for a revert (search git log for 2a027b47dba6).
Re: [PATCH V2 1/2] Revert "ssb: Prevent build of PCI host features in module"
On 2018-05-12 10:01, Michael Büsch wrote: On Sat, 12 May 2018 10:50:42 +0300 Kalle Valo wrote: Larry Finger writes: > On 05/11/2018 05:13 AM, Kalle Valo wrote: >> Rafał Miłecki writes: >> >>> On 11 May 2018 at 11:17, Rafał Miłecki wrote: [...] >>> >>> As these patches fix regression/build error, I believe both should get >>> into 4.17. >> >> How much confidence do we have that we don't need to end up reverting >> patch 2 as well? I rather be pushing patch 2 to 4.18 so that there's >> more time for testing and waiting for feedback. > > Although I do not have the hardware to test the builds, I worked > closely with the OP in the bug at b.r.c noted above. From that effort, > it became clear what configuration variables were missing to cause the > x86 failures. Patch 2 satisfies the requirement, and prevents the > build problems found by the MIPS users. Both patches are needed in > 4.17. And I assume Michael is ok with this approach as well as I haven't heard from him. I'll then push both of these to 4.17. Yes, I'm OK with the patch, if we have a third patch that cleans up the PCI_DRIVERS_LEGACY dependency by moving it to SSB_PCICORE_HOSTMODE where it belongs. (This doesn't need to go into the stable tree.) We currently implicitly get that via dependency chain, so this is OK for now as-is. I'm planning to handle PCI_DRIVERS_LEGACY cleanup once my patches hit net-next.git and then wireless-drivers-next.git. It's to avoid conflicts.
Problem with late AMD microcode reload/feedback
Hi, I'm trying to reload AMD Ryzen Mobile (fam17h) microcode doing: echo 1 > /sys/devices/system/cpu/microcode/reload The problem is I don't get any feedback. No error for the "echo" command, no a single new line in the "dmesg". I have no idea if microcode has been reloaded or not. I did a quick pr_info based debugging and I noticed that: 1) load_microcode_amd() calls __load_microcode_amd() and gets UCODE_OK 2) load_microcode_amd() calls find_patch(0) and gets a NULL because of that NULL load_microcode_amd() doesn't return UCODE_NEW. Seeing above I've decided to debug find_patch(). It seems to be calling __find_equiv_id(0) which returns 0. The last step was debugging __find_equiv_id() and find_equiv_id(). It seems that find_equiv_id() gets sig 8458000 that doesn't exists in the equiv_cpu_table: [19.736770] microcode: [find_equiv_id] sig:8458000 [19.736772] microcode: [find_equiv_id] equiv_table->installed_cpu:8392466 [19.736775] microcode: [find_equiv_id] equiv_table->installed_cpu:8392578 Has my microcode been updated? Is there a way to improve that microcode loading code? Is find_patch(0) returning a NULL expected or maybe a bug? -- Rafał
Re: Problem with late AMD microcode reload/feedback
On 16.12.2018 01:05, Borislav Petkov wrote: On Sun, Dec 16, 2018 at 12:46:05AM +0100, Rafał Miłecki wrote: [19.736770] microcode: [find_equiv_id] sig:8458000 That's your CPU's family/model/stepping: 0x0810f10 [19.736772] microcode: [find_equiv_id] equiv_table->installed_cpu:8392466 [19.736775] microcode: [find_equiv_id] equiv_table->installed_cpu:8392578 and those are present on the system. Best to look at them in hex, btw: 0x0800f12 0x0800f82 Which means, there's no microcode for your CPU so nothing gets updated. Thanks! I had no idea microcode_amd_fam17h.bin is a container with few microcodes. I thought there is a single microcode for a whole family (e.g. 17h). Using hex also makes more sense indeed! [ 44.127941] microcode: verify_and_add_patch: Added patch_id: 0x08001227, proc_id: 0x8012 [ 44.127948] microcode: verify_and_add_patch: Added patch_id: 0x0800820b, proc_id: 0x8082 [ 44.127952] microcode: [find_equiv_id] sig:0x810f10 [ 44.127955] microcode: [find_equiv_id] equiv_table->installed_cpu:0x800f12 [ 44.127958] microcode: [find_equiv_id] equiv_table->installed_cpu:0x800f82 So for now I'm stuck with the default/BIOS-uploaded microcode: [2.604680] microcode: CPU0: patch_level=0x0810100b [2.605617] microcode: CPU1: patch_level=0x0810100b [2.606583] microcode: CPU2: patch_level=0x0810100b [2.607528] microcode: CPU3: patch_level=0x0810100b [2.608408] microcode: CPU4: patch_level=0x0810100b [2.609285] microcode: CPU5: patch_level=0x0810100b [2.610270] microcode: CPU6: patch_level=0x0810100b [2.611135] microcode: CPU7: patch_level=0x0810100b I've one more hacking idea. My notebook has Ryzen 5 PRO 2500U CPU but I also have access to another one with Ryzen 5 2500U running: [2.780949] microcode: CPU0: patch_level=0x08101007 For my hack tests I'd like to replace my 0x0810100b with a 0x08101007. Is that possible to extract/dump current microcode from the CPU and package it as microcode_amd_fam17h.bin? Are there any ready tools for that?
Re: Problem with late AMD microcode reload/feedback
On Sun, 16 Dec 2018 at 11:06, Borislav Petkov wrote: > > For my hack tests I'd like to replace my 0x0810100b with a 0x08101007. > > Why would you even want to downgrade the microcode?! Debugging CPU errors. I have two notebooks: 1) HP EliteBook 745 G5 with Ryzen 5 PRO 2500U It runs 1.03.01 BIOS with 0x0810100b microcode and suffers from MCE logged CPU errors. 2) MateBook D with Ryzen 5 2500U It runs 1.12 BIOS with 0x08101007 microcode and MCE doesn't report any CPU errors. I wanted to downgrade microcode on HP EliteBook and upgrade microcode on MateBook to see if that makes a difference for them. For that I need to: 1) Dump old microcode from MateBook & run it on EliteBook 2) Dump new microcode from EliteBook & run it on MateBook -- Rafał
Re: Problem with late AMD microcode reload/feedback
On Sun, 16 Dec 2018 at 11:44, Borislav Petkov wrote: > On Sun, Dec 16, 2018 at 11:26:29AM +0100, Rafał Miłecki wrote: > > Debugging CPU errors. > > I told you that this issue is being worked on and there will be a fix > of sorts at some point. Don't try any funky business of downgrading the > microcode and maybe break your boxes in the process. Just ignore the MCE > - it is harmless! - until there's a fix. > > Ok? OK, if you say so, I'll try not to panic seeing those errors repeating over and over. I know such issues may take months or years to get fixed, so I was trying to do some hacking on my own. I'll try some patience :) -- Rafał
[PATCH 1/2] dt-bindings: bcm-ns-usb2-phy: rework binding to use CRU syscon
From: Rafał Miłecki USB 2.0 PHY is a hardware block that happens to use two registers from the CRU block to setup a single PLL. It's not part of the CRU or DMU and so its binding shouldn't cover the whole DMU. The correct way of handling this is to reference CRU block node using a syscon. Document that & deprecate the old way. Signed-off-by: Rafał Miłecki --- .../devicetree/bindings/phy/bcm-ns-usb2-phy.txt| 27 ++ 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/bcm-ns-usb2-phy.txt b/Documentation/devicetree/bindings/phy/bcm-ns-usb2-phy.txt index a7aee9ea8926..36b634d2f0ca 100644 --- a/Documentation/devicetree/bindings/phy/bcm-ns-usb2-phy.txt +++ b/Documentation/devicetree/bindings/phy/bcm-ns-usb2-phy.txt @@ -2,19 +2,38 @@ Driver for Broadcom Northstar USB 2.0 PHY Required properties: - compatible: brcm,ns-usb2-phy -- reg: iomem address range of DMU (Device Management Unit) -- reg-names: "dmu", the only needed & supported reg right now +- syscon-cru: phandle to the CRU (Central Resource Unit) syscon - clocks: USB PHY reference clock - clock-names: "phy-ref-clk", the only needed & supported clock right now +Deprecated: + +PHY block should not claim the whole DMU so such a binding has been deprecated. +It only requires to access few CRU (a DMU subblock) registers and that should be +handled with a syscon since CRU is a MFD (Multi-Function Device). + +- reg: iomem address range of DMU (Device Management Unit) +- reg-names: "dmu", the only needed & supported reg right now + To initialize USB 2.0 PHY driver needs to setup PLL correctly. To do this it requires passing phandle to the USB PHY reference clock. Example: + dmu@1800c000 { + compatible = "simple-bus"; + ranges = <0 0x1800c000 0x1000>; + #address-cells = <1>; + #size-cells = <1>; + + cru: cru@100 { + compatible = "syscon", "simple-mfd"; + reg = <0x100 0x1a4>; + }; + }; + usb2-phy { compatible = "brcm,ns-usb2-phy"; - reg = <0x1800c000 0x1000>; - reg-names = "dmu"; + syscon-cru = <&cru>; #phy-cells = <0>; clocks = <&genpll BCM_NSP_GENPLL_USB_PHY_REF_CLK>; clock-names = "phy-ref-clk"; -- 2.13.7
[PATCH 2/2] phy: bcm-ns-usb2: support updated DT binding with the CRU syscon
From: Rafał Miłecki This adds support for the "syscon-cru" DT property which simply requires using regmap to access CRU registers. The old binding has been deprecated and stays as a fallback method. Signed-off-by: Rafał Miłecki --- drivers/phy/broadcom/phy-bcm-ns-usb2.c | 44 +++--- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb2.c b/drivers/phy/broadcom/phy-bcm-ns-usb2.c index 58dff80e9386..8dc6835b941c 100644 --- a/drivers/phy/broadcom/phy-bcm-ns-usb2.c +++ b/drivers/phy/broadcom/phy-bcm-ns-usb2.c @@ -13,17 +13,23 @@ #include #include #include +#include #include #include #include +#include #include #include #include +#define CRU_USB2_CONTROL 0x64 +#define CRU_CLKSET_KEY 0x80 + struct bcm_ns_usb2 { struct device *dev; struct clk *ref_clk; struct phy *phy; + struct regmap *cru; void __iomem *dmu; }; @@ -31,6 +37,7 @@ static int bcm_ns_usb2_phy_init(struct phy *phy) { struct bcm_ns_usb2 *usb2 = phy_get_drvdata(phy); struct device *dev = usb2->dev; + struct regmap *cru = usb2->cru; void __iomem *dmu = usb2->dmu; u32 ref_clk_rate, usb2ctl, usb_pll_ndiv, usb_pll_pdiv; int err = 0; @@ -48,7 +55,10 @@ static int bcm_ns_usb2_phy_init(struct phy *phy) goto err_clk_off; } - usb2ctl = readl(dmu + BCMA_DMU_CRU_USB2_CONTROL); + if (cru) + regmap_read(cru, CRU_USB2_CONTROL, &usb2ctl); + else + usb2ctl = readl(dmu + BCMA_DMU_CRU_USB2_CONTROL); if (usb2ctl & BCMA_DMU_CRU_USB2_CONTROL_USB_PLL_PDIV_MASK) { usb_pll_pdiv = usb2ctl; @@ -62,15 +72,24 @@ static int bcm_ns_usb2_phy_init(struct phy *phy) usb_pll_ndiv = (192000 * usb_pll_pdiv) / ref_clk_rate; /* Unlock DMU PLL settings with some magic value */ - writel(0xea68, dmu + BCMA_DMU_CRU_CLKSET_KEY); + if (cru) + regmap_write(cru, CRU_CLKSET_KEY, 0xea68); + else + writel(0xea68, dmu + BCMA_DMU_CRU_CLKSET_KEY); /* Write USB 2.0 PLL control setting */ usb2ctl &= ~BCMA_DMU_CRU_USB2_CONTROL_USB_PLL_NDIV_MASK; usb2ctl |= usb_pll_ndiv << BCMA_DMU_CRU_USB2_CONTROL_USB_PLL_NDIV_SHIFT; - writel(usb2ctl, dmu + BCMA_DMU_CRU_USB2_CONTROL); + if (cru) + regmap_write(cru, CRU_USB2_CONTROL, usb2ctl); + else + writel(usb2ctl, dmu + BCMA_DMU_CRU_USB2_CONTROL); /* Lock DMU PLL settings */ - writel(0x, dmu + BCMA_DMU_CRU_CLKSET_KEY); + if (cru) + regmap_write(cru, CRU_CLKSET_KEY, 0x); + else + writel(0x, dmu + BCMA_DMU_CRU_CLKSET_KEY); err_clk_off: clk_disable_unprepare(usb2->ref_clk); @@ -86,6 +105,7 @@ static const struct phy_ops ops = { static int bcm_ns_usb2_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + struct device_node *cru_np; struct bcm_ns_usb2 *usb2; struct resource *res; struct phy_provider *phy_provider; @@ -95,11 +115,17 @@ static int bcm_ns_usb2_probe(struct platform_device *pdev) return -ENOMEM; usb2->dev = dev; - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmu"); - usb2->dmu = devm_ioremap_resource(dev, res); - if (IS_ERR(usb2->dmu)) { - dev_err(dev, "Failed to map DMU regs\n"); - return PTR_ERR(usb2->dmu); + cru_np = of_parse_phandle(dev->of_node, "syscon-cru", 0); + usb2->cru = syscon_node_to_regmap(cru_np); + if (IS_ERR(usb2->cru)) { + usb2->cru = NULL; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmu"); + usb2->dmu = devm_ioremap_resource(dev, res); + if (IS_ERR(usb2->dmu)) { + dev_err(dev, "Failed to map DMU regs\n"); + return PTR_ERR(usb2->dmu); + } } usb2->ref_clk = devm_clk_get(dev, "phy-ref-clk"); -- 2.13.7