Re: [PATCH 06/11] bcma: convert bus code to use dev_groups

2013-10-10 Thread Rafał Miłecki
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-09 Thread Rafał Miłecki
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-09 Thread Rafał Miłecki
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-05-31 Thread Rafał Miłecki
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-06-08 Thread Rafał Miłecki
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-04-08 Thread Rafał Miłecki
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-05-21 Thread Rafał Miłecki
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-05-28 Thread Rafał Miłecki
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-01-20 Thread Rafał Miłecki
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

2016-07-19 Thread Rafał Miłecki
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

2016-07-19 Thread Rafał Miłecki
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

2016-07-20 Thread Rafał Miłecki
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

2016-09-19 Thread Rafał Miłecki
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

2016-09-20 Thread Rafał Miłecki
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

2016-09-20 Thread Rafał Miłecki
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

2016-09-21 Thread Rafał Miłecki
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

2016-09-21 Thread Rafał Miłecki
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

2016-09-21 Thread Rafał Miłecki
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

2016-09-21 Thread Rafał Miłecki
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

2016-06-21 Thread Rafał Miłecki
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

2016-06-21 Thread Rafał Miłecki
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

2016-06-21 Thread Rafał Miłecki
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

2016-09-23 Thread Rafał Miłecki
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

2016-09-24 Thread Rafał Miłecki
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

2016-09-24 Thread Rafał Miłecki
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

2016-09-24 Thread Rafał Miłecki
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

2016-09-26 Thread Rafał Miłecki
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

2016-09-26 Thread Rafał Miłecki
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

2016-09-26 Thread Rafał Miłecki
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

2016-09-26 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-09-27 Thread Rafał Miłecki
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

2016-07-27 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-07-29 Thread Rafał Miłecki
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

2016-09-08 Thread Rafał Miłecki
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

2016-09-09 Thread Rafał Miłecki
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

2016-09-13 Thread Rafał Miłecki
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

2016-09-14 Thread Rafał Miłecki
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

2016-09-15 Thread Rafał Miłecki
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

2016-08-31 Thread Rafał Miłecki
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

2016-08-31 Thread Rafał Miłecki
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

2016-08-23 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-24 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-25 Thread Rafał Miłecki
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

2016-08-26 Thread Rafał Miłecki
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()

2016-08-26 Thread Rafał Miłecki
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

2016-08-26 Thread Rafał Miłecki
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

2016-08-26 Thread Rafał Miłecki
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

2016-08-26 Thread Rafał Miłecki
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

2017-06-20 Thread Rafał Miłecki

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

2017-06-20 Thread Rafał Miłecki

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

2015-10-18 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2017-05-11 Thread Rafał Miłecki
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

2018-01-17 Thread Rafał Miłecki

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

2018-01-17 Thread Rafał Miłecki
 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

2018-03-10 Thread Rafał Miłecki
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

2018-03-07 Thread Rafał Miłecki
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

2018-03-07 Thread Rafał Miłecki
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

2018-03-23 Thread Rafał Miłecki
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

2018-04-08 Thread Rafał Miłecki
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

2018-04-08 Thread Rafał Miłecki
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

2018-03-23 Thread Rafał Miłecki
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

2018-11-15 Thread Rafał Miłecki
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

2018-12-07 Thread Rafał Miłecki

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

2018-12-03 Thread Rafał Miłecki
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

2018-11-11 Thread Rafał Miłecki
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"

2018-05-12 Thread Rafał Miłecki

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

2018-12-15 Thread Rafał Miłecki
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

2018-12-16 Thread Rafał Miłecki

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

2018-12-16 Thread Rafał Miłecki
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

2018-12-16 Thread Rafał Miłecki
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

2019-01-07 Thread Rafał Miłecki
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

2019-01-07 Thread Rafał Miłecki
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



  1   2   3   4   5   6   7   >