[PATCH] media: atomisp: ov2722: remove unnecessary debug print
checkpatch.pl emits the following warning: WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string + dev_dbg(>dev, "ov2722_remove...\n"); This is just a "trace" call and therefore should be removed entirely; ftrace should be used instead. Signed-off-by: Filip Kolev --- drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c index eecefcd734d0e..1209492c1826a 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c @@ -1175,8 +1175,6 @@ static int ov2722_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov2722_device *dev = to_ov2722_sensor(sd); - dev_dbg(>dev, "ov2722_remove...\n"); - dev->platform_data->csi_cfg(sd, 0); v4l2_ctrl_handler_free(>ctrl_handler); v4l2_device_unregister_subdev(sd); -- 2.30.0
Re: [PATCH] media: atomisp: ov2722: replace hardcoded function name
On 06-Jan-21 09:51, Greg Kroah-Hartman wrote: On Tue, Jan 05, 2021 at 10:29:18PM +0200, Filip Kolev wrote: There is a debug message using hardcoded function name instead of the __func__ macro. Replace it. Report from checkpatch.pl on the file: WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string + dev_dbg(>dev, "ov2722_remove...\n"); Signed-off-by: Filip Kolev --- drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c index eecefcd734d0e..21d6bc62d452a 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov2722_device *dev = to_ov2722_sensor(sd); - dev_dbg(>dev, "ov2722_remove...\n"); + dev_dbg(>dev, "%s...\n", __func__); dev_dbg() provides the function name already, and this is just a "trace" call, and ftrace should be used instead, so the whole line should be removed entirely. Thank you for the review! How do I go about this? Do I amend the patch and re-send as v2 or create a new patch entirely? Newbie here, doing this as part of the Eudyptula challenge, so I very much appreciate everyone's patience. thanks, greg k-h
[PATCH] media: atomisp: ov2722: replace hardcoded function name
There is a debug message using hardcoded function name instead of the __func__ macro. Replace it. Report from checkpatch.pl on the file: WARNING: Prefer using '"%s...", __func__' to using 'ov2722_remove', this function's name, in a string + dev_dbg(>dev, "ov2722_remove...\n"); Signed-off-by: Filip Kolev --- drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c index eecefcd734d0e..21d6bc62d452a 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct ov2722_device *dev = to_ov2722_sensor(sd); - dev_dbg(>dev, "ov2722_remove...\n"); + dev_dbg(>dev, "%s...\n", __func__); dev->platform_data->csi_cfg(sd, 0); v4l2_ctrl_handler_free(>ctrl_handler); -- 2.30.0
[PATCH] net: usb: qmi_wwan: Set DTR quirk for MR400
LTE module MR400 embedded in TL-MR6400 v4 requires DTR to be set.
Signed-off-by: Filip Moc
---
drivers/net/usb/qmi_wwan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index afeb09b9624e..d166c321ee9b 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1047,7 +1047,7 @@ static const struct usb_device_id products[] = {
{QMI_FIXED_INTF(0x05c6, 0x9011, 4)},
{QMI_FIXED_INTF(0x05c6, 0x9021, 1)},
{QMI_FIXED_INTF(0x05c6, 0x9022, 2)},
- {QMI_FIXED_INTF(0x05c6, 0x9025, 4)},/* Alcatel-sbell ASB TL131 TDD
LTE (China Mobile) */
+ {QMI_QUIRK_SET_DTR(0x05c6, 0x9025, 4)}, /* Alcatel-sbell ASB TL131 TDD
LTE (China Mobile) */
{QMI_FIXED_INTF(0x05c6, 0x9026, 3)},
{QMI_FIXED_INTF(0x05c6, 0x902e, 5)},
{QMI_FIXED_INTF(0x05c6, 0x9031, 5)},
--
2.28.0
Re: [PATCH] sound/tlv320dac33: Add device tree support
Hi, On 01/30/2018 09:53 AM, Pavel Machek wrote: > On Tue 2018-01-30 09:34:46, Ladislav Michl wrote: >> On Tue, Jan 30, 2018 at 12:33:01AM +0100, Pavel Machek wrote: >>> On Tue 2018-01-30 00:20:31, Ladislav Michl wrote: >>>> On Tue, Jan 30, 2018 at 12:05:39AM +0100, Pavel Machek wrote: >>>>> >>>>> This adds device tree support to tlv320dac33.c. >>>>> >>>>> Signed-off-by: Pavel Machek <pa...@ucw.cz> >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> b/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> new file mode 100644 >>>>> index 000..6cbd311 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> @@ -0,0 +1,32 @@ >>>>> +Texas Instruments - tlv320dac33 Codec module >>>>> + >>>>> +The tlv320dac33 serial control bus communicates through I2C protocols. >>>>> + >>>>> +Required properties: >>>>> + >>>>> +- compatible - "ti,tlv320dac33" >>>>> +- reg - I2C slave address >>>>> + >>>>> +Optional properties: >>>>> + >>>>> +- power-gpios - gpio pin to power the device, active high >>>> >>>> While driver used gpio in platform data, isn't it more likely >>>> regulator which powers device? >>> >>> power-gpios = < 28 0>; /* gpio_60 */ >>> >>> Looks like GPIO to me -- example is from Nokia N9. So this appears to >>> be correct. >> >> Device datasheet doesn't list any pin which looks like "power-gpio" >> http://www.ti.com/lit/ds/symlink/tlv320dac32.pdf >> Unfortunately I do not know much about N9, but was able to find Nokia 5610 >> scheme to get clue how could be tlv320dac33 hardwired (see page 2): >> http://mastermobile.spb.ru/service/nokia_5610_rm-242_service_schematics.pdf >> Here AVDD is powered by LP3985 voltage regulator which is enabled using >> VEN pin which might be connected to gpio. Or there could be completely >> different voltage regulator with different controls. And since Linux >> already has voltage regulator class, lets not limit ourselves to gpio >> pins. > > Well, notice I'm converting existing driver to device tree. And that > one already has GPIO dependency. It is possible that more work needs > to be done there, but that should not be a reason to delay this. Feel > free to help. > > Pavel > According to N9 schematics http://www.s-manuals.com/manuals/phone/nokia/nokia_n9_rm-696_service_schematics_v1.pdf it's in fact GPIO pin that is connected to reset line (labeled CODEC_RST). So calling it "power" might be misleading, but the driver code is quite clear as it labels that GPIO as "tlv320dac33 reset" Filip
Re: [PATCH] sound/tlv320dac33: Add device tree support
Hi, On 01/30/2018 09:53 AM, Pavel Machek wrote: > On Tue 2018-01-30 09:34:46, Ladislav Michl wrote: >> On Tue, Jan 30, 2018 at 12:33:01AM +0100, Pavel Machek wrote: >>> On Tue 2018-01-30 00:20:31, Ladislav Michl wrote: >>>> On Tue, Jan 30, 2018 at 12:05:39AM +0100, Pavel Machek wrote: >>>>> >>>>> This adds device tree support to tlv320dac33.c. >>>>> >>>>> Signed-off-by: Pavel Machek >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> b/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> new file mode 100644 >>>>> index 000..6cbd311 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/sound/tlv320dac33.txt >>>>> @@ -0,0 +1,32 @@ >>>>> +Texas Instruments - tlv320dac33 Codec module >>>>> + >>>>> +The tlv320dac33 serial control bus communicates through I2C protocols. >>>>> + >>>>> +Required properties: >>>>> + >>>>> +- compatible - "ti,tlv320dac33" >>>>> +- reg - I2C slave address >>>>> + >>>>> +Optional properties: >>>>> + >>>>> +- power-gpios - gpio pin to power the device, active high >>>> >>>> While driver used gpio in platform data, isn't it more likely >>>> regulator which powers device? >>> >>> power-gpios = < 28 0>; /* gpio_60 */ >>> >>> Looks like GPIO to me -- example is from Nokia N9. So this appears to >>> be correct. >> >> Device datasheet doesn't list any pin which looks like "power-gpio" >> http://www.ti.com/lit/ds/symlink/tlv320dac32.pdf >> Unfortunately I do not know much about N9, but was able to find Nokia 5610 >> scheme to get clue how could be tlv320dac33 hardwired (see page 2): >> http://mastermobile.spb.ru/service/nokia_5610_rm-242_service_schematics.pdf >> Here AVDD is powered by LP3985 voltage regulator which is enabled using >> VEN pin which might be connected to gpio. Or there could be completely >> different voltage regulator with different controls. And since Linux >> already has voltage regulator class, lets not limit ourselves to gpio >> pins. > > Well, notice I'm converting existing driver to device tree. And that > one already has GPIO dependency. It is possible that more work needs > to be done there, but that should not be a reason to delay this. Feel > free to help. > > Pavel > According to N9 schematics http://www.s-manuals.com/manuals/phone/nokia/nokia_n9_rm-696_service_schematics_v1.pdf it's in fact GPIO pin that is connected to reset line (labeled CODEC_RST). So calling it "power" might be misleading, but the driver code is quite clear as it labels that GPIO as "tlv320dac33 reset" Filip
Re: Vibrations, audio, charging, radio on N9/N950
Hi, On 01/03/2018 02:56 PM, Pavel Machek wrote: > Hi! > > Sebasian, you submitted patch to enable vibrations on N950. I am > trying to do the same now on N9... I guess I enabled the dts, but > .. how do I actually ask for vibrations? /dev/input/eventX does not > seem to be present. > > Did anyone get audio to run on N9/N950? It is marked as supported on > https://elinux.org/N950 with dt glue missing... > I have fairly recent patches (for v4.10) here: https://github.com/filippz/pmbootstrap/commit/600473432d8ff61ae80a7e2a198d9442fd3a6f2e (from 0015 trough 0022 w/o 0016). Vibration was working as was audio playback from twl5031 and tvl320dac33 together with tpa6140a2. More details here: http://talk.maemo.org/showpost.php?p=1492590=112 I hope that you can use those patches at least to some degree. > Even more importantly, does battery charging work for someone? It does > not work here :-(. After USB is plugged in, it maybe tries to charge, > but gives up after 30 seconds. > > Maybe least important, but... does anyone have FM radio working? I'd > like to play some music... I haven't used FM radio nor BT (wl1273), so I can't comment on that, but they are in there but probably need some work. > > Pavel > Best regards, Filip
Re: Vibrations, audio, charging, radio on N9/N950
Hi, On 01/03/2018 02:56 PM, Pavel Machek wrote: > Hi! > > Sebasian, you submitted patch to enable vibrations on N950. I am > trying to do the same now on N9... I guess I enabled the dts, but > .. how do I actually ask for vibrations? /dev/input/eventX does not > seem to be present. > > Did anyone get audio to run on N9/N950? It is marked as supported on > https://elinux.org/N950 with dt glue missing... > I have fairly recent patches (for v4.10) here: https://github.com/filippz/pmbootstrap/commit/600473432d8ff61ae80a7e2a198d9442fd3a6f2e (from 0015 trough 0022 w/o 0016). Vibration was working as was audio playback from twl5031 and tvl320dac33 together with tpa6140a2. More details here: http://talk.maemo.org/showpost.php?p=1492590=112 I hope that you can use those patches at least to some degree. > Even more importantly, does battery charging work for someone? It does > not work here :-(. After USB is plugged in, it maybe tries to charge, > but gives up after 30 seconds. > > Maybe least important, but... does anyone have FM radio working? I'd > like to play some music... I haven't used FM radio nor BT (wl1273), so I can't comment on that, but they are in there but probably need some work. > > Pavel > Best regards, Filip
Re: [PATCH] nokia N9: Add support for magnetometer and touchscreen
Hi,
On 01/02/2018 06:27 PM, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Jan 02, 2018 at 02:17:22PM +0100, Pavel Machek wrote:
>> This adds dts support for magnetometer and touchscreen on Nokia N9.
>
> I think it makes sense to have this splitted.
>
>> Signed-off-by: Pavel Machek <pa...@ucw.cz>
>>
>> diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
>> index 39e35f8..57a6679 100644
>> --- a/arch/arm/boot/dts/omap3-n9.dts
>> +++ b/arch/arm/boot/dts/omap3-n9.dts
>> @@ -36,6 +57,22 @@
>> };
>> };
>> };
>> +
>> +touch@4b {
>
> touchscreen@
>
>> +compatible = "atmel,maxtouch";
>> +reg = <0x4b>;
>> +interrupt-parent = <>;
>> +interrupts = <29 2>; /* gpio_61, IRQF_TRIGGER_FALLING*/
>
> reset-gpios = < 17 GPIO_ACTIVE_SOMETHING>;
>
I'm using reset-gpios = < 17 0>;
>> +vdd-supply = <>;
>> +avdd-supply = <>;
>
> Those two are not mentioned in the binding and not supported by the
> driver as far as I can see?
>
Right, but vio and vaux1 need to be on - the reason why it's working at
all is because lis302 uses the same regulators and turns them on. IMHO
either we add the support for regulators to maxtouch driver or we add
regulator-always-on to vio and vaux1.
>> +};
>> +};
>
> Touchscreen with the same settings is required for n950, so it
> should be in the shared n950 + n9 file.
>
As a side-note, there is no pinmux mentioned and usually I'd use
OMAP3_CORE1_IOPAD(0x20c8, PIN_INPUT | MUX_MODE4) /* gpio_61*/
OMAP3_CORE1_IOPAD(0x20f2, PIN_OUTPUT | MUX_MODE4) /* gpio_81*/
For reasons that I can't explain, first line (gpmc_nbe1->gpio_61) breaks
it for me, so I've commented it out. Still, if anyone has an idea what
is wrong with that please let me know.
>> + {
>> +ak8975@0f {
>> +compatible = "asahi-kasei,ak8975";
>> +reg = <0x0f>;
>> +};
>> };
>
> Looking at the N9 board file this is missing a rotation matrix. This
> is supported by the binding:
>
> Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
>
>>
>> {
>
> -- Sebastian
>
Best regards,
Filip
Re: [PATCH] nokia N9: Add support for magnetometer and touchscreen
Hi,
On 01/02/2018 06:27 PM, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Jan 02, 2018 at 02:17:22PM +0100, Pavel Machek wrote:
>> This adds dts support for magnetometer and touchscreen on Nokia N9.
>
> I think it makes sense to have this splitted.
>
>> Signed-off-by: Pavel Machek
>>
>> diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
>> index 39e35f8..57a6679 100644
>> --- a/arch/arm/boot/dts/omap3-n9.dts
>> +++ b/arch/arm/boot/dts/omap3-n9.dts
>> @@ -36,6 +57,22 @@
>> };
>> };
>> };
>> +
>> +touch@4b {
>
> touchscreen@
>
>> +compatible = "atmel,maxtouch";
>> +reg = <0x4b>;
>> +interrupt-parent = <>;
>> +interrupts = <29 2>; /* gpio_61, IRQF_TRIGGER_FALLING*/
>
> reset-gpios = < 17 GPIO_ACTIVE_SOMETHING>;
>
I'm using reset-gpios = < 17 0>;
>> +vdd-supply = <>;
>> +avdd-supply = <>;
>
> Those two are not mentioned in the binding and not supported by the
> driver as far as I can see?
>
Right, but vio and vaux1 need to be on - the reason why it's working at
all is because lis302 uses the same regulators and turns them on. IMHO
either we add the support for regulators to maxtouch driver or we add
regulator-always-on to vio and vaux1.
>> +};
>> +};
>
> Touchscreen with the same settings is required for n950, so it
> should be in the shared n950 + n9 file.
>
As a side-note, there is no pinmux mentioned and usually I'd use
OMAP3_CORE1_IOPAD(0x20c8, PIN_INPUT | MUX_MODE4) /* gpio_61*/
OMAP3_CORE1_IOPAD(0x20f2, PIN_OUTPUT | MUX_MODE4) /* gpio_81*/
For reasons that I can't explain, first line (gpmc_nbe1->gpio_61) breaks
it for me, so I've commented it out. Still, if anyone has an idea what
is wrong with that please let me know.
>> + {
>> +ak8975@0f {
>> +compatible = "asahi-kasei,ak8975";
>> +reg = <0x0f>;
>> +};
>> };
>
> Looking at the N9 board file this is missing a rotation matrix. This
> is supported by the binding:
>
> Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
>
>>
>> {
>
> -- Sebastian
>
Best regards,
Filip
Re: [PATCH] Device tree binding for Avago APDS990X light sensor
Hi Sakari,
and thank you for your input - I've added a few comments below.
On 12/27/2017 07:00 PM, Sakari Ailus wrote:
> Hi Pavel,
>
> Thanks for the patch. Please see my comments below.
>
> On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
>> From: Filip Matijević <filip.matijevic...@gmail.com>
>>
>> This prepares binding for light sensor used in Nokia N9.
>>
>> Signed-off-by: Filip Matijević <filip.matijevic...@gmail.com>
>> Signed-off-by: Pavel machek <pa...@ucw.cz>
>>
>> ---
>>
>> Patches to convert APDS990X driver to device tree and to switch to iio
>> are available.
>>
>> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> new file mode 100644
>> index 000..e038146
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> @@ -0,0 +1,39 @@
>> +Avago APDS990X driver
>> +
>> +Required properties:
>> +- compatible: "avago,apds990x"
>> +- reg: address on the I2C bus
>> +- interrupts: external interrupt line number
>> +- Vdd-supply: power supply for VDD
>> +- Vled-supply: power supply for LEDA
>
> AFAIK the custom is to use lower case letters for regulator supplies.
I've just used the same notation as in current driver. Datasheet calls
those VDD (with DD being in subscript) and LEDA. I see no problem in
changing those to vdd-supply and vled-supply if no one else objects.
>
>> +- ga: Glass attenuation
>> +- cf1: Clear channel factor 1
>> +- irf1: IR channel factor 1
>> +- cf2: Clear channel factor 2
>> +- irf2: IR channel factor 2
>> +- df: Device factor
>> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
>> +- ppcount: Proximity pulse count
>
> Are these device specific? If so, please add the vendor prefix to them.
AFAIK yes - same as before if no one else objects, these will be changed.
>
> I might not use short abbreviations such as "df" either. I wonder what
> others think.
These are also come from current driver - some of these comes from the
datasheet itself, but not all. For example "ga" and "df" are in there
(so I I would leave those alone), but "irf1" is called "B", "cf2" is
called "C" and "irf2" is called "D" (I guess the missing "cf1" should be
"A", but there is no such thing in the datasheet).
IMHO using some other names might just add to the confusion.
>
>> +
>> +Example (Nokia N9):
>> +
>> +als_ps@39 {
>> +compatible = "avago,apds990x";
>> +reg = <0x39>;
>> +
>> +interrupt-parent = <>;
>> +interrupts = <19 10>; /* gpio_83, IRQF_TRIGGER_FALLING |
>> IRQF_TRIGGER_LOW */
>> +
>> +Vdd-supply = <>;
>> +Vled-supply = <>;
>> +
>> +ga = <168834>;
>> +cf1 = <4096>;
>> +irf1= <7824>;
>> +cf2 = <877>;
>> +irf2= <1575>;
>> +df = <52>;
>> +
>> +pdrive = <0x2>; /* APDS_IRLED_CURR_25mA */
>> +ppcount = <5>;
>> +};
>>
>
Best regards,
Filip
Re: [PATCH] Device tree binding for Avago APDS990X light sensor
Hi Sakari,
and thank you for your input - I've added a few comments below.
On 12/27/2017 07:00 PM, Sakari Ailus wrote:
> Hi Pavel,
>
> Thanks for the patch. Please see my comments below.
>
> On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
>> From: Filip Matijević
>>
>> This prepares binding for light sensor used in Nokia N9.
>>
>> Signed-off-by: Filip Matijević
>> Signed-off-by: Pavel machek
>>
>> ---
>>
>> Patches to convert APDS990X driver to device tree and to switch to iio
>> are available.
>>
>> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> new file mode 100644
>> index 000..e038146
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> @@ -0,0 +1,39 @@
>> +Avago APDS990X driver
>> +
>> +Required properties:
>> +- compatible: "avago,apds990x"
>> +- reg: address on the I2C bus
>> +- interrupts: external interrupt line number
>> +- Vdd-supply: power supply for VDD
>> +- Vled-supply: power supply for LEDA
>
> AFAIK the custom is to use lower case letters for regulator supplies.
I've just used the same notation as in current driver. Datasheet calls
those VDD (with DD being in subscript) and LEDA. I see no problem in
changing those to vdd-supply and vled-supply if no one else objects.
>
>> +- ga: Glass attenuation
>> +- cf1: Clear channel factor 1
>> +- irf1: IR channel factor 1
>> +- cf2: Clear channel factor 2
>> +- irf2: IR channel factor 2
>> +- df: Device factor
>> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
>> +- ppcount: Proximity pulse count
>
> Are these device specific? If so, please add the vendor prefix to them.
AFAIK yes - same as before if no one else objects, these will be changed.
>
> I might not use short abbreviations such as "df" either. I wonder what
> others think.
These are also come from current driver - some of these comes from the
datasheet itself, but not all. For example "ga" and "df" are in there
(so I I would leave those alone), but "irf1" is called "B", "cf2" is
called "C" and "irf2" is called "D" (I guess the missing "cf1" should be
"A", but there is no such thing in the datasheet).
IMHO using some other names might just add to the confusion.
>
>> +
>> +Example (Nokia N9):
>> +
>> +als_ps@39 {
>> +compatible = "avago,apds990x";
>> +reg = <0x39>;
>> +
>> +interrupt-parent = <>;
>> +interrupts = <19 10>; /* gpio_83, IRQF_TRIGGER_FALLING |
>> IRQF_TRIGGER_LOW */
>> +
>> +Vdd-supply = <>;
>> +Vled-supply = <>;
>> +
>> +ga = <168834>;
>> +cf1 = <4096>;
>> +irf1= <7824>;
>> +cf2 = <877>;
>> +irf2= <1575>;
>> +df = <52>;
>> +
>> +pdrive = <0x2>; /* APDS_IRLED_CURR_25mA */
>> +ppcount = <5>;
>> +};
>>
>
Best regards,
Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
> If someone knows how to fix the backlight, it would still be useful. In panel-dsi-cm.c it's set to maximum with dsicm_dcs_write_1(ddata, DCS_BRIGHTNESS, 0xff); Sebastian suggested the same but used 100 instead of 0xff, which might be a bit too low. You can also add msleep(1000) after dsicm_dcs_write_0(ddata, MIPI_DCS_SET_DISPLAY_ON); to see if brightness gets overwritten, the whole display goes into sleep mode, is turned off or similar somewhere down the line. I'm still unable to make N9 panel to cooperate (despite having "the right" clocks and able to send image to panel with MIPI_DCS_WRITE_MEMORY_START/MIPI_DCS_READ_MEMORY_CONTINUE) so I hope you'll have more luck than me! Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
> If someone knows how to fix the backlight, it would still be useful. In panel-dsi-cm.c it's set to maximum with dsicm_dcs_write_1(ddata, DCS_BRIGHTNESS, 0xff); Sebastian suggested the same but used 100 instead of 0xff, which might be a bit too low. You can also add msleep(1000) after dsicm_dcs_write_0(ddata, MIPI_DCS_SET_DISPLAY_ON); to see if brightness gets overwritten, the whole display goes into sleep mode, is turned off or similar somewhere down the line. I'm still unable to make N9 panel to cooperate (despite having "the right" clocks and able to send image to panel with MIPI_DCS_WRITE_MEMORY_START/MIPI_DCS_READ_MEMORY_CONTINUE) so I hope you'll have more luck than me! Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
On 10/26/2017 09:16 AM, Pavel Machek wrote:
> "Uart pins not muxed in NOLO" would certainly explain my problems. In
> 3.5.X, output works even in decompressor ("Uncompressing
> kernel... done, booting linux").
IIRC bootloader passes some additional settings for gpio that kernel
should apply (2.6 and 3.5 do that). I'm not sure if that would cause
booting to fail on that specific device, but can you compare kernel
params on S and P devices (by using 3.5.3 for P device)?
Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
On 10/26/2017 09:16 AM, Pavel Machek wrote:
> "Uart pins not muxed in NOLO" would certainly explain my problems. In
> 3.5.X, output works even in decompressor ("Uncompressing
> kernel... done, booting linux").
IIRC bootloader passes some additional settings for gpio that kernel
should apply (2.6 and 3.5 do that). I'm not sure if that would cause
booting to fail on that specific device, but can you compare kernel
params on S and P devices (by using 3.5.3 for P device)?
Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
Hi Pavel, On 10/25/2017 10:34 PM, Pavel Machek wrote: > 3.5.3-nemo kernel seems to work I did take a quick look at the board-rm680.c since there are some conditional statements with regards to system revision but I didn't notice anything that might prevent it from booting. > If you have > any ideas, let me know. Last time I couldn't get output on serial I used CONFIG_DEBUG_LL, CONFIG_DEBUG_OMAP3UART3, CONFIG_EARLY_PRINTK... and ignore_loglevel earlyprintk kernel params to get output early enough - I guess you already tried that. Regards, Filip
Re: 4.13 (and probably all recent) kernels refuse to boot on one Nokia N950, work or another
Hi Pavel, On 10/25/2017 10:34 PM, Pavel Machek wrote: > 3.5.3-nemo kernel seems to work I did take a quick look at the board-rm680.c since there are some conditional statements with regards to system revision but I didn't notice anything that might prevent it from booting. > If you have > any ideas, let me know. Last time I couldn't get output on serial I used CONFIG_DEBUG_LL, CONFIG_DEBUG_OMAP3UART3, CONFIG_EARLY_PRINTK... and ignore_loglevel earlyprintk kernel params to get output early enough - I guess you already tried that. Regards, Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Hi, On Sun, Mar 19, 2017 at 07:04:13PM +0100, Jan Kara wrote: > How come? In current kernel the call looks like: > > vfs_mknod(d_inode(path.dentry), dentry, mode, dev->devt); > > so the path is available there... I've actually quickly checked all > vfs_mknod() callers and they all seem to have path available. terribly sorry, must have misremembered something. Been staring at the code long into the night. You are quite right. But it is an internal mount so userspace never gets the notifications. The same goes for the cloned upper mount in overlayfs. This might be considered ok, because the change is semantically "internal" and does not originate through any userspace-visible mountpoint. Superblock watches would solve this case. Otherwise it seems feasible to pass a path to all VFS functions. Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Hi, On Sun, Mar 19, 2017 at 07:04:13PM +0100, Jan Kara wrote: > How come? In current kernel the call looks like: > > vfs_mknod(d_inode(path.dentry), dentry, mode, dev->devt); > > so the path is available there... I've actually quickly checked all > vfs_mknod() callers and they all seem to have path available. terribly sorry, must have misremembered something. Been staring at the code long into the night. You are quite right. But it is an internal mount so userspace never gets the notifications. The same goes for the cloned upper mount in overlayfs. This might be considered ok, because the change is semantically "internal" and does not originate through any userspace-visible mountpoint. Superblock watches would solve this case. Otherwise it seems feasible to pass a path to all VFS functions. Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
On Sun, Mar 19, 2017 at 11:19:43AM +0100, Jan Kara wrote: > However if you can really call fsnotify hooks with 'path' available in all > the places, it should be equally hard to just pass 'path' to > vfs_(create|mkdir|...) and that way we don't have to sprinkle fsnotify > calls into several call sites but keep them local to vfs_(create|mkdir|...) > helpers. Hmm? the problem is: not absolutely all. One illuminating example is the use of vfs_mknod in devtmpfs. There a struct path is not only unavailable but makes not semantic sense: the changes do not go thru any mountpoint. And in general I think there will be situations where you would need to call VFS functions without paths. Thus I suggested either (a) wrapping the VFS functions with path variants, or (b) giving them an optional vfsmount argument that can be set to NULL when it does not make sense Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
On Sun, Mar 19, 2017 at 11:19:43AM +0100, Jan Kara wrote: > However if you can really call fsnotify hooks with 'path' available in all > the places, it should be equally hard to just pass 'path' to > vfs_(create|mkdir|...) and that way we don't have to sprinkle fsnotify > calls into several call sites but keep them local to vfs_(create|mkdir|...) > helpers. Hmm? the problem is: not absolutely all. One illuminating example is the use of vfs_mknod in devtmpfs. There a struct path is not only unavailable but makes not semantic sense: the changes do not go thru any mountpoint. And in general I think there will be situations where you would need to call VFS functions without paths. Thus I suggested either (a) wrapping the VFS functions with path variants, or (b) giving them an optional vfsmount argument that can be set to NULL when it does not make sense Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Hi, On Tue, Mar 14, 2017 at 01:18:01PM +0200, Amir Goldstein wrote: > I claim that fanotify filters event by mount not because it > was a requirement, but because it was an implementation challenge > to do otherwise. > > And I claim that what mount watchers are really interested in is > "all the changes that happen in the file system in the area > that is visible to me through this mount point". > > In other words, an indexer needs to know if files were modified\ > create/deleted if that indexer sits in container host namespace > regardless if those files were modified from within a container > namespace. > > It's not a matter of security/isolation. It's a matter of functionality. > I agree that for some event (e.g. permission events) it is possible > to argue both ways (i.e. that the namespace context should be used > as a filter for events). > But for the new proposed events (FS_MODIFY_DIR), I really don't > see the point in isolation by mount/namespace. there are basically two classes of uses for a fantotify-like interface: (1) Keeping an up-to-date representation of the file system. For this, superblock watches are clearly what you want. * You are interested to know the current state of the filesystem so you need to know about every change, regardless of where it came from. * As I mentioned earlier, in case of remote, ditributed and virtual filesystems, the change might come from within the filesystem itself (if the protocol supports reporting such changes). This can probably be implemented only with superblock-scoped watches because the change is fundamentally not related to any mount. * Some filesystems might also support change journalling and it might be concievable to extend the API in the future to report "past" events (for example by passing sequence number of last seen event or similar). * The argument about containers escaping change notification you mentioned earlier. All those factors speak greatly in favour of superblock watches. (2) Tracking filesystem *activity*. Now you are not building an image of current filesystem state but rather a log of what happened. Perhaps you are also interested in who (user/process/...) did what. Permission events also fit mostly in this category. For those it *might* make sense to have mount-scoped watches, for example if you want to monitor only one container or a subset of processes. We both concentrate on the first but we shouldn't forget about the second, which was one of the original motivations for fanotify. Thus I conclude that it might be desirable to implement mount-scoped filename events in the long run. Even though I agree that the sb-scoped events are more important because they cover more use cases and you can do additional filtering (e.g. by pid) if deemed necessary. This would require: (a) Sprinkling the callers of vfs_* with fanotify calls as I did, or (b) Creating wrapper functions like vfs_path_unlink & co. that would make the necessary fanotify call (and probably tell the lower function not to generate another notification), as I suggested earlier. (c) Give the vfs_* functions an *optional* vfsmount argument. In the end I probably find (c) the most elegant but this can be discussed later, even after your changes are merged. Filip
Re: [RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Hi, On Tue, Mar 14, 2017 at 01:18:01PM +0200, Amir Goldstein wrote: > I claim that fanotify filters event by mount not because it > was a requirement, but because it was an implementation challenge > to do otherwise. > > And I claim that what mount watchers are really interested in is > "all the changes that happen in the file system in the area > that is visible to me through this mount point". > > In other words, an indexer needs to know if files were modified\ > create/deleted if that indexer sits in container host namespace > regardless if those files were modified from within a container > namespace. > > It's not a matter of security/isolation. It's a matter of functionality. > I agree that for some event (e.g. permission events) it is possible > to argue both ways (i.e. that the namespace context should be used > as a filter for events). > But for the new proposed events (FS_MODIFY_DIR), I really don't > see the point in isolation by mount/namespace. there are basically two classes of uses for a fantotify-like interface: (1) Keeping an up-to-date representation of the file system. For this, superblock watches are clearly what you want. * You are interested to know the current state of the filesystem so you need to know about every change, regardless of where it came from. * As I mentioned earlier, in case of remote, ditributed and virtual filesystems, the change might come from within the filesystem itself (if the protocol supports reporting such changes). This can probably be implemented only with superblock-scoped watches because the change is fundamentally not related to any mount. * Some filesystems might also support change journalling and it might be concievable to extend the API in the future to report "past" events (for example by passing sequence number of last seen event or similar). * The argument about containers escaping change notification you mentioned earlier. All those factors speak greatly in favour of superblock watches. (2) Tracking filesystem *activity*. Now you are not building an image of current filesystem state but rather a log of what happened. Perhaps you are also interested in who (user/process/...) did what. Permission events also fit mostly in this category. For those it *might* make sense to have mount-scoped watches, for example if you want to monitor only one container or a subset of processes. We both concentrate on the first but we shouldn't forget about the second, which was one of the original motivations for fanotify. Thus I conclude that it might be desirable to implement mount-scoped filename events in the long run. Even though I agree that the sb-scoped events are more important because they cover more use cases and you can do additional filtering (e.g. by pid) if deemed necessary. This would require: (a) Sprinkling the callers of vfs_* with fanotify calls as I did, or (b) Creating wrapper functions like vfs_path_unlink & co. that would make the necessary fanotify call (and probably tell the lower function not to generate another notification), as I suggested earlier. (c) Give the vfs_* functions an *optional* vfsmount argument. In the end I probably find (c) the most elegant but this can be discussed later, even after your changes are merged. Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 03:55:19PM +0200, Amir Goldstein wrote: > Please let me know if that is sufficient for your needs > or if you need me to prepare a version that delivers filename events > without filename info, therefore allowing to merge filename events. > > Sounds to me like if you get an event FAN_DELETE, "aaa", > your implementation can update the db directly without having > to scan the directory, so it should be useful. > For your consideration. both approaches might be useful and there are tradeoffs. Direct updates save us from rescanning but give more events and more context switches. On the other hand, with an unlimited queue and well-mergable events I could e.g. sleep for five seconds each time after emptying the queue, thus giving the events more potential to merge and reducing context switches. But one risks blowing up the queue. However, I have some ideas. Basically we could implement some kind of "bulk read" mode for fanotify that would pass events to userspace only when (a) a given event count thresold is hit (e.g. half the queue limit if queue is limited), or (b) the oldest event is older than a specified maximum latency. That might vary from 1 second to 5 minutes depending on specific application needs (e.g. background backup does not need to be low-latency). This would greatly reduce extra context switches when watching a large portion (or whole) of the file system. All of this presumably could be implemented on top of your suggested patches. Either way, I suggest that implementing the nameless filename events is a good idea. I do not know whether they will be the best choice for my application but they probably will be useful for some applications. Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 03:55:19PM +0200, Amir Goldstein wrote: > Please let me know if that is sufficient for your needs > or if you need me to prepare a version that delivers filename events > without filename info, therefore allowing to merge filename events. > > Sounds to me like if you get an event FAN_DELETE, "aaa", > your implementation can update the db directly without having > to scan the directory, so it should be useful. > For your consideration. both approaches might be useful and there are tradeoffs. Direct updates save us from rescanning but give more events and more context switches. On the other hand, with an unlimited queue and well-mergable events I could e.g. sleep for five seconds each time after emptying the queue, thus giving the events more potential to merge and reducing context switches. But one risks blowing up the queue. However, I have some ideas. Basically we could implement some kind of "bulk read" mode for fanotify that would pass events to userspace only when (a) a given event count thresold is hit (e.g. half the queue limit if queue is limited), or (b) the oldest event is older than a specified maximum latency. That might vary from 1 second to 5 minutes depending on specific application needs (e.g. background backup does not need to be low-latency). This would greatly reduce extra context switches when watching a large portion (or whole) of the file system. All of this presumably could be implemented on top of your suggested patches. Either way, I suggest that implementing the nameless filename events is a good idea. I do not know whether they will be the best choice for my application but they probably will be useful for some applications. Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 12:40:56PM +0200, Amir Goldstein wrote: > An I am very happy that you used filehandles to keep track of objects > in your example, because it fits my proposal really well. you can read more about the rationales in the WIP draft of my thesis (especially the chapter Identifying Inodes and its surroundings): http://regnarg.cz/tmp/thesis.pdf https://github.com/regnarg/bc Either way, one can never use pathnames as identifiers because in a world with parallel renames on multiple levels of the hierarchy (between which no ordering is guaranteed unless they are cross-dir), pathnames are not even a well-defined concept. NB: I used a simple script for testing that does precisely this (a lot of parallel within-directory renames at many levels): https://github.com/regnarg/bc/blob/master/experiments/fanotify/parallel_renamist.sh It seems like a good stress test for any application that tries to do reliable filesystem monitoring. > See, if you used my proposed API, you would have had > > fan_fd = CHK(fanotify_init(FAN_UNLIMITED_QUEUE| \ > FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_FH, > O_RDONLY)); Sure, I'll definitely try to implement your interface as an optional backend and mention it in my thesis and definitely give it some heavy testing. > Furthermore, my proposal records the filehandle at the time of the event > and therefore, does not have to keep an elevated refcount of the object > in the events queue. That sounds good but from what I've understood, not all filesystems support handles. Is this a concern? (Maybe the right answer is to extend handle support...) Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 12:40:56PM +0200, Amir Goldstein wrote: > An I am very happy that you used filehandles to keep track of objects > in your example, because it fits my proposal really well. you can read more about the rationales in the WIP draft of my thesis (especially the chapter Identifying Inodes and its surroundings): http://regnarg.cz/tmp/thesis.pdf https://github.com/regnarg/bc Either way, one can never use pathnames as identifiers because in a world with parallel renames on multiple levels of the hierarchy (between which no ordering is guaranteed unless they are cross-dir), pathnames are not even a well-defined concept. NB: I used a simple script for testing that does precisely this (a lot of parallel within-directory renames at many levels): https://github.com/regnarg/bc/blob/master/experiments/fanotify/parallel_renamist.sh It seems like a good stress test for any application that tries to do reliable filesystem monitoring. > See, if you used my proposed API, you would have had > > fan_fd = CHK(fanotify_init(FAN_UNLIMITED_QUEUE| \ > FAN_EVENT_INFO_PARENT | FAN_EVENT_INFO_FH, > O_RDONLY)); Sure, I'll definitely try to implement your interface as an optional backend and mention it in my thesis and definitely give it some heavy testing. > Furthermore, my proposal records the filehandle at the time of the event > and therefore, does not have to keep an elevated refcount of the object > in the events queue. That sounds good but from what I've understood, not all filesystems support handles. Is this a concern? (Maybe the right answer is to extend handle support...) Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 12:11:40PM +0200, Amir Goldstein wrote: > > - file system indexers / desktop search tools > > - file synchronization tools (like Dropbox, Nextcloud, etc.), > > online backup tools > > This last one is the use case of my employer, Ctera Networks. > Out of curiosity, what is the use case that you are focusing on? I'm working on a file synchronization tool as part of my bachelor thesis at Charles University. When I started (now over a year ago ... long story), there were AFIK no attempts at solving the recursive inotify issue, only a lot of complaints, so I cobbled up together something simple (I'm not a kernel developer by trade, this was my first patch) that would allow me to work on the userspace parts, which are the main bulk. I try to focus on algorithmic and implementation efficiency as opposed to fancy GUIs and similar. I want it to be fast with 100k directories and 10M files in home dir. But it's very WIP so we'll see how that turns out. > I had the feeling that all recursive inotify users are hiding in the shadows, > but was missing more concrete evidence. Yes, even Dropbox uses inotify. They have articles in their help on how to increase inotify.max_user_watches: https://www.dropbox.com/help/145. That's not good PR. ;-) (I'm not affiliated with DB, just pointing that out.) > About the argument of not having to change in-kernel framework, > I don't think it should be a consideration at all. Understood. I tried to stay conservative and non-controversial because I imagined that radical framework changes would take months of discussions (look at the history of statx) and this issue seems to be pressing for quite a lot of people. But rushing is of course not the best strategy either, there are always compromises. > If you don't specify FAN_EVENT_INFO_NAME, you can get filename events > FAN_MOVE|FAN_CREATE|FAN_DELETE without the name. > > What you do get is the file descriptor of the parent. sounds familiar? ;-) I didn't notice this bit. That sounds like a win-win. Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Hi, On Tue, Mar 14, 2017 at 12:11:40PM +0200, Amir Goldstein wrote: > > - file system indexers / desktop search tools > > - file synchronization tools (like Dropbox, Nextcloud, etc.), > > online backup tools > > This last one is the use case of my employer, Ctera Networks. > Out of curiosity, what is the use case that you are focusing on? I'm working on a file synchronization tool as part of my bachelor thesis at Charles University. When I started (now over a year ago ... long story), there were AFIK no attempts at solving the recursive inotify issue, only a lot of complaints, so I cobbled up together something simple (I'm not a kernel developer by trade, this was my first patch) that would allow me to work on the userspace parts, which are the main bulk. I try to focus on algorithmic and implementation efficiency as opposed to fancy GUIs and similar. I want it to be fast with 100k directories and 10M files in home dir. But it's very WIP so we'll see how that turns out. > I had the feeling that all recursive inotify users are hiding in the shadows, > but was missing more concrete evidence. Yes, even Dropbox uses inotify. They have articles in their help on how to increase inotify.max_user_watches: https://www.dropbox.com/help/145. That's not good PR. ;-) (I'm not affiliated with DB, just pointing that out.) > About the argument of not having to change in-kernel framework, > I don't think it should be a consideration at all. Understood. I tried to stay conservative and non-controversial because I imagined that radical framework changes would take months of discussions (look at the history of statx) and this issue seems to be pressing for quite a lot of people. But rushing is of course not the best strategy either, there are always compromises. > If you don't specify FAN_EVENT_INFO_NAME, you can get filename events > FAN_MOVE|FAN_CREATE|FAN_DELETE without the name. > > What you do get is the file descriptor of the parent. sounds familiar? ;-) I didn't notice this bit. That sounds like a win-win. Filip
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
An example userspace program that uses FAN_MODIFY_DIR to reliably keep
an up-to-date internal representation of the file system. It uses some
filehandle trickery to identify inodes, other heuristics could be also
used.
---
//#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
using namespace std;
#ifndef FAN_MODIFY_DIR
#define FAN_MODIFY_DIR 0x0004
#endif
// die-on-error helpers
#define CHK(x) ({ __typeof__(x) r = x; if (r == -1) { perror(#x); abort(); } r;
})
#define CHKN(x) ({ __typeof__(x) r = x; if (r == NULL) { perror(#x); abort(); }
r; })
struct inode_info;
struct dentry_info;
struct inode_info {
ino_t ino;
mode_t mode;
char handle[MAX_HANDLE_SZ];
set links;
map children; // for directory inodes
};
struct dentry_info {
struct inode_info *parent, *inode;
string name;
};
map inodes;
int root_fd;
int fan_fd;
bool compare_handles(const void *h1, const void *h2) {
const struct file_handle *fh1 = (const struct file_handle*) h1;
const struct file_handle *fh2 = (const struct file_handle*) h2;
return (fh1->handle_bytes == fh2->handle_bytes
&& memcmp(h1, h2, fh1->handle_bytes) == 0);
}
bool handle_valid(void *handle) {
int check_fd = open_by_handle_at(root_fd, (struct file_handle*)handle,
O_PATH);
if (check_fd >= 0) {
CHK(close(check_fd));
return true;
} else if (errno == ESTALE) {
return false;
} else {
perror("open_by_handle_at");
exit(1);
}
}
// Get the path corresponding to an inode (one of its paths, in the presence of
// hardlinks).
void inode_path(const struct inode_info *inode, char *buf, size_t bufsiz) {
list components;
while (true) {
if (inode->links.empty()) break;
struct dentry_info *dentry = *inode->links.begin();
components.push_front(dentry->name);
inode = dentry->parent;
}
buf[0] = '\0';
for (auto name: components) {
int len = snprintf(buf, bufsiz, "/%s", name.c_str());
buf += len;
bufsiz -= len;
}
}
void delete_dentry(struct dentry_info *dentry) {
assert(dentry->parent->children[dentry->name] == dentry);
char path_buf[4096];
inode_path(dentry->parent, path_buf, sizeof(path_buf));
printf("unlinked %s/%s (ino %lu, parent %lu)\n", path_buf,
dentry->name.c_str(),
dentry->inode->ino, dentry->parent->ino);
dentry->parent->children.erase(dentry->name.c_str());
dentry->inode->links.erase(dentry);
// TODO: If this was the last dentry pointing to an inode, schedule removing
// the inode after a timeout (we cannot remove it immediately because
// the zero-link situation might occur during a rename when the source
// directory has been processed but the target directory hasn't).
delete dentry;
}
struct dentry_info *add_dentry(struct inode_info *parent, const char *name,
struct inode_info *child) {
struct dentry_info *dentry = new dentry_info();
dentry->parent = parent;
dentry->name = name;
dentry->inode = child;
parent->children[name] = dentry;
child->links.insert(dentry);
char path_buf[4096] = "\0";
inode_path(parent, path_buf, sizeof(path_buf));
printf("linked %s/%s (ino %lu, parent %lu)\n", path_buf, name, child->ino,
parent->ino);
return dentry;
}
void delete_inode(struct inode_info *inode) {
for (auto dentry: inode->links) {
delete_dentry(dentry);
}
delete inode;
}
// Given a file descriptor, find the corresponding inode object in our database,
// or create a new one if it does not exist. An O_PATH fd suffices.
struct inode_info *find_inode(int fd) {
struct stat st;
CHK(fstat(fd, ));
char handle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
struct file_handle *fh = (struct file_handle*)handle;
fh->handle_bytes = sizeof(handle);
int mntid;
CHK(name_to_handle_at(fd, "", (struct file_handle*)handle, ,
AT_EMPTY_PATH));
struct inode_info *info = inodes[st.st_ino];
if (info) {
// Handles can refer to the same file despite not being equal.
// If the old handle can still be opened, we can be assured
// that the inode number has not been recycled.
if (compare_handles(handle, info->handle) ||
handle_valid(info->handle)) {
return info;
} else {
delete_inode(info);
info = NULL;
}
}
inodes[st.st_ino] = info = new inode_info();
info->ino = st.st_ino;
info->mode = st.st_mode;
memcpy(info->handle, handle, fh->handle_bytes);
return info;
}
// Scan directory and update internal filesystem representation accordingly.
// Closes `dirfd`.
void scan(int dirfd, bool recursive) {
struct inode_info *dir =
Re: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR
An example userspace program that uses FAN_MODIFY_DIR to reliably keep
an up-to-date internal representation of the file system. It uses some
filehandle trickery to identify inodes, other heuristics could be also
used.
---
//#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
using namespace std;
#ifndef FAN_MODIFY_DIR
#define FAN_MODIFY_DIR 0x0004
#endif
// die-on-error helpers
#define CHK(x) ({ __typeof__(x) r = x; if (r == -1) { perror(#x); abort(); } r;
})
#define CHKN(x) ({ __typeof__(x) r = x; if (r == NULL) { perror(#x); abort(); }
r; })
struct inode_info;
struct dentry_info;
struct inode_info {
ino_t ino;
mode_t mode;
char handle[MAX_HANDLE_SZ];
set links;
map children; // for directory inodes
};
struct dentry_info {
struct inode_info *parent, *inode;
string name;
};
map inodes;
int root_fd;
int fan_fd;
bool compare_handles(const void *h1, const void *h2) {
const struct file_handle *fh1 = (const struct file_handle*) h1;
const struct file_handle *fh2 = (const struct file_handle*) h2;
return (fh1->handle_bytes == fh2->handle_bytes
&& memcmp(h1, h2, fh1->handle_bytes) == 0);
}
bool handle_valid(void *handle) {
int check_fd = open_by_handle_at(root_fd, (struct file_handle*)handle,
O_PATH);
if (check_fd >= 0) {
CHK(close(check_fd));
return true;
} else if (errno == ESTALE) {
return false;
} else {
perror("open_by_handle_at");
exit(1);
}
}
// Get the path corresponding to an inode (one of its paths, in the presence of
// hardlinks).
void inode_path(const struct inode_info *inode, char *buf, size_t bufsiz) {
list components;
while (true) {
if (inode->links.empty()) break;
struct dentry_info *dentry = *inode->links.begin();
components.push_front(dentry->name);
inode = dentry->parent;
}
buf[0] = '\0';
for (auto name: components) {
int len = snprintf(buf, bufsiz, "/%s", name.c_str());
buf += len;
bufsiz -= len;
}
}
void delete_dentry(struct dentry_info *dentry) {
assert(dentry->parent->children[dentry->name] == dentry);
char path_buf[4096];
inode_path(dentry->parent, path_buf, sizeof(path_buf));
printf("unlinked %s/%s (ino %lu, parent %lu)\n", path_buf,
dentry->name.c_str(),
dentry->inode->ino, dentry->parent->ino);
dentry->parent->children.erase(dentry->name.c_str());
dentry->inode->links.erase(dentry);
// TODO: If this was the last dentry pointing to an inode, schedule removing
// the inode after a timeout (we cannot remove it immediately because
// the zero-link situation might occur during a rename when the source
// directory has been processed but the target directory hasn't).
delete dentry;
}
struct dentry_info *add_dentry(struct inode_info *parent, const char *name,
struct inode_info *child) {
struct dentry_info *dentry = new dentry_info();
dentry->parent = parent;
dentry->name = name;
dentry->inode = child;
parent->children[name] = dentry;
child->links.insert(dentry);
char path_buf[4096] = "\0";
inode_path(parent, path_buf, sizeof(path_buf));
printf("linked %s/%s (ino %lu, parent %lu)\n", path_buf, name, child->ino,
parent->ino);
return dentry;
}
void delete_inode(struct inode_info *inode) {
for (auto dentry: inode->links) {
delete_dentry(dentry);
}
delete inode;
}
// Given a file descriptor, find the corresponding inode object in our database,
// or create a new one if it does not exist. An O_PATH fd suffices.
struct inode_info *find_inode(int fd) {
struct stat st;
CHK(fstat(fd, ));
char handle[sizeof(struct file_handle) + MAX_HANDLE_SZ];
struct file_handle *fh = (struct file_handle*)handle;
fh->handle_bytes = sizeof(handle);
int mntid;
CHK(name_to_handle_at(fd, "", (struct file_handle*)handle, ,
AT_EMPTY_PATH));
struct inode_info *info = inodes[st.st_ino];
if (info) {
// Handles can refer to the same file despite not being equal.
// If the old handle can still be opened, we can be assured
// that the inode number has not been recycled.
if (compare_handles(handle, info->handle) ||
handle_valid(info->handle)) {
return info;
} else {
delete_inode(info);
info = NULL;
}
}
inodes[st.st_ino] = info = new inode_info();
info->ino = st.st_ino;
info->mode = st.st_mode;
memcpy(info->handle, handle, fh->handle_bytes);
return info;
}
// Scan directory and update internal filesystem representation accordingly.
// Closes `dirfd`.
void scan(int dirfd, bool recursive) {
struct inode_info *dir = find_inode(dirfd);
char path_buf[4096] = "\0";
[RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Besause fanotify requires `struct path`, the event cannot be generated
directly in `fsnotify_move` and friends because they only get the inode
(and their callers, `vfs_rename` cannot supply any better info).
So instead it needs to be generated higher in the call chain, i.e. in
the callers of functions like `vfs_rename`.
This leads to some code duplication. Currently, there are several places
whence functions like `vfs_rename` or `vfs_unlink` are called:
* syscall handlers (done)
* NFS server (done)
* stacked filesystems
- ecryptfs (done)
- overlayfs
(Currently doesn't report even ordinary fanotify events, because
it internally clones the upper mount; not sure about the
rationale. One can always watch the overlay mount instead.)
* few rather minor things
- devtmpfs
(its internal changes are not tied to any vfsmount so it cannot
emit mount-scoped events)
- cachefiles (done)
- ipc/mqueue.c (done)
- fs/nfsd/nfs4recover.c (done)
- kernel/bpf/inode.c (done)
net/unix/af_unix.c (done)
(grep -rE
'\bvfs_(rename|unlink|mknod|whiteout|create|mkdir|rmdir|symlink|link)\(')
Signed-off-by: Filip Štědronský <r.l...@regnarg.cz>
---
An alternative might be to create wrapper functions like
vfs_path_(rename|unlink|...). They could also take care of calling
security_path_(rename|unlink|...), which is currently also up to
the indvidual callers (possibly with a flag because it might not
be always desired).
---
fs/cachefiles/namei.c | 9 +++
fs/ecryptfs/inode.c | 67 +++
fs/namei.c| 23 +-
fs/nfsd/nfs4recover.c | 7 ++
fs/nfsd/vfs.c | 24 --
ipc/mqueue.c | 9 +++
kernel/bpf/inode.c| 3 +++
net/unix/af_unix.c| 2 ++
8 files changed, 141 insertions(+), 3 deletions(-)
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 41df8a27d7eb..8c86699424d1 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -313,6 +313,8 @@ static int cachefiles_bury_object(struct cachefiles_cache
*cache,
cachefiles_io_error(cache, "Unlink security error");
} else {
ret = vfs_unlink(d_inode(dir), rep, NULL);
+ if (ret == 0)
+ fsnotify_modify_dir();
if (preemptive)
cachefiles_mark_object_buried(cache, rep, why);
@@ -418,6 +420,10 @@ static int cachefiles_bury_object(struct cachefiles_cache
*cache,
if (ret != 0 && ret != -ENOMEM)
cachefiles_io_error(cache,
"Rename failed with error %d", ret);
+ if (ret == 0) {
+ fsnotify_modify_dir();
+ fsnotify_modify_dir(_to_graveyard);
+ }
if (preemptive)
cachefiles_mark_object_buried(cache, rep, why);
@@ -560,6 +566,7 @@ int cachefiles_walk_to_object(struct cachefiles_object
*parent,
cachefiles_hist(cachefiles_mkdir_histogram, start);
if (ret < 0)
goto create_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(next));
@@ -589,6 +596,7 @@ int cachefiles_walk_to_object(struct cachefiles_object
*parent,
cachefiles_hist(cachefiles_create_histogram, start);
if (ret < 0)
goto create_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(next));
@@ -779,6 +787,7 @@ struct dentry *cachefiles_get_directory(struct
cachefiles_cache *cache,
ret = vfs_mkdir(d_inode(dir), subdir, 0700);
if (ret < 0)
goto mkdir_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(subdir));
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index e7413f82d27b..88a41b270bcc 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -29,6 +29,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -144,16 +146,22 @@ static int ecryptfs_do_unlink(struct inode *dir, struct
dentry *dentry,
{
struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
+ struct vfsmount *lower_mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+ struct path lower_dir_path = {lower_mnt, NULL};
struct dentry *lower_dir_dentry;
int rc;
dget(lower_dentry);
lower_dir_dentry = lock_parent(lower_dentry);
+ lower_dir_path.dentry = lower_dir_dentry;
rc = vfs_unlink(lower_dir
[RFC 2/2] fanotify: emit FAN_MODIFY_DIR on filesystem changes
Besause fanotify requires `struct path`, the event cannot be generated
directly in `fsnotify_move` and friends because they only get the inode
(and their callers, `vfs_rename` cannot supply any better info).
So instead it needs to be generated higher in the call chain, i.e. in
the callers of functions like `vfs_rename`.
This leads to some code duplication. Currently, there are several places
whence functions like `vfs_rename` or `vfs_unlink` are called:
* syscall handlers (done)
* NFS server (done)
* stacked filesystems
- ecryptfs (done)
- overlayfs
(Currently doesn't report even ordinary fanotify events, because
it internally clones the upper mount; not sure about the
rationale. One can always watch the overlay mount instead.)
* few rather minor things
- devtmpfs
(its internal changes are not tied to any vfsmount so it cannot
emit mount-scoped events)
- cachefiles (done)
- ipc/mqueue.c (done)
- fs/nfsd/nfs4recover.c (done)
- kernel/bpf/inode.c (done)
net/unix/af_unix.c (done)
(grep -rE
'\bvfs_(rename|unlink|mknod|whiteout|create|mkdir|rmdir|symlink|link)\(')
Signed-off-by: Filip Štědronský
---
An alternative might be to create wrapper functions like
vfs_path_(rename|unlink|...). They could also take care of calling
security_path_(rename|unlink|...), which is currently also up to
the indvidual callers (possibly with a flag because it might not
be always desired).
---
fs/cachefiles/namei.c | 9 +++
fs/ecryptfs/inode.c | 67 +++
fs/namei.c| 23 +-
fs/nfsd/nfs4recover.c | 7 ++
fs/nfsd/vfs.c | 24 --
ipc/mqueue.c | 9 +++
kernel/bpf/inode.c| 3 +++
net/unix/af_unix.c| 2 ++
8 files changed, 141 insertions(+), 3 deletions(-)
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 41df8a27d7eb..8c86699424d1 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -313,6 +313,8 @@ static int cachefiles_bury_object(struct cachefiles_cache
*cache,
cachefiles_io_error(cache, "Unlink security error");
} else {
ret = vfs_unlink(d_inode(dir), rep, NULL);
+ if (ret == 0)
+ fsnotify_modify_dir();
if (preemptive)
cachefiles_mark_object_buried(cache, rep, why);
@@ -418,6 +420,10 @@ static int cachefiles_bury_object(struct cachefiles_cache
*cache,
if (ret != 0 && ret != -ENOMEM)
cachefiles_io_error(cache,
"Rename failed with error %d", ret);
+ if (ret == 0) {
+ fsnotify_modify_dir();
+ fsnotify_modify_dir(_to_graveyard);
+ }
if (preemptive)
cachefiles_mark_object_buried(cache, rep, why);
@@ -560,6 +566,7 @@ int cachefiles_walk_to_object(struct cachefiles_object
*parent,
cachefiles_hist(cachefiles_mkdir_histogram, start);
if (ret < 0)
goto create_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(next));
@@ -589,6 +596,7 @@ int cachefiles_walk_to_object(struct cachefiles_object
*parent,
cachefiles_hist(cachefiles_create_histogram, start);
if (ret < 0)
goto create_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(next));
@@ -779,6 +787,7 @@ struct dentry *cachefiles_get_directory(struct
cachefiles_cache *cache,
ret = vfs_mkdir(d_inode(dir), subdir, 0700);
if (ret < 0)
goto mkdir_error;
+ fsnotify_modify_dir();
ASSERT(d_backing_inode(subdir));
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index e7413f82d27b..88a41b270bcc 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -29,6 +29,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -144,16 +146,22 @@ static int ecryptfs_do_unlink(struct inode *dir, struct
dentry *dentry,
{
struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
+ struct vfsmount *lower_mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+ struct path lower_dir_path = {lower_mnt, NULL};
struct dentry *lower_dir_dentry;
int rc;
dget(lower_dentry);
lower_dir_dentry = lock_parent(lower_dentry);
+ lower_dir_path.dentry = lower_dir_dentry;
rc = vfs_unlink(lower_dir_inode
[RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Fanotify currently does not report directory modification events (rename, unlink, etc.). But these events are essential for about half of concievable fanotify use cases, especially: - file system indexers / desktop search tools - file synchronization tools (like Dropbox, Nextcloud, etc.), online backup tools and pretty much any app that needs to maintain and up-to-date internal representation of current contents of the file system. All applications of the above classes that I'm aware of currently use recursive inotify watches, which do not scale (my home dir has ~200k directories, creating all the watches takes ~2min and eats several tens of MB of kernel memory). There have been many calls for such a feature, pretty much since the creation of fanotify in 2009: * By GNOME developers: https://wiki.gnome.org/BastienNocera/KernelWishlist#VFS.2C_filesystems * By KDE developers: http://lkml.kernel.org/r/201211011352.42476.Martin%40lichtvoll.de 'Better support for (desktop) file search / indexing applications' * And others: http://lkml.kernel.org/r/AANLkTi=owK=WZW4oNtpm5WpAZhqCQUdTR2K5gzJ_MqZ+%40mail.gmail.com 'Fanotify mv/rename?' http://lkml.kernel.org/r/1238158043.23703.20.camel%40fatty 'Issues with using fanotify for a filesystem indexer' Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the contents of a directory change (a directory entry is added, removed or renamed). This covers all the currently missing events: rename, unlink, mknod, mkdir, rmdir, etc. Included with the event is a file descriptor to the modified directory but no further info. The userspace application is expected to rescan the whole directory and update its model accordingly. This needs to be done carefully to prevent race conditions. A cross-directory rename generates two FAN_MODIFY_DIR events. This minimalistic approach has several advantages: * Does not require changes to the fanotify userspace API or the fsnotify in-kernel framework, apart from adding a new event. Especially doesn't complicate it by adding string fields. * Has simple and clear semantics, even with multiple renames occurring in parallel etc. In case of any inconsistencies, one can simply wait for a while and rescan again. * FAN_MODIFY_DIR events are easily merged in case of multiple operations on a directory (e.g. deleting all files). Signed-off-by: Filip Štědronský <r.l...@regnarg.cz> --- An alternative might be to create wrapper functions like vfs_path_(rename|unlink|...). They could also take care of calling security_path_(rename|unlink|...), which is currently also up to the indvidual callers (possibly with a flag because it might not be always desired). An alternative was proposed by Amir Goldstein in several long series of patches that add superblock-scoped (as opposed to vfsmount-scoped) fanotify watches and specific dentry manipulation events with filenames: http://lkml.kernel.org/r/1481984434-13293-1-git-send-email-amir73il%40gmail.com http://lkml.kernel.org/r/1482247207-4424-1-git-send-email-amir73il%40gmail.com http://lkml.kernel.org/r/1476126784-12520-1-git-send-email-amir73il%40gmail.com http://lkml.kernel.org/r/1489411223-12081-1-git-send-email-amir73il%40gmail.com There is large but not complete overlap between that proposal and mine (which is was originally created over a year ago, before Amir's work, but never posted). I think the superblock watch idea is very interesting because it might in the future allow reporing fsnotify events from remote and virtual filesystems. So I'm posting this more as a potential source of more ideas for discussion, or a fallback proposal in case Amir's patches don't make it. --- fs/notify/fanotify/fanotify.c| 1 + include/linux/fsnotify.h | 17 + include/linux/fsnotify_backend.h | 1 + include/uapi/linux/fanotify.h| 5 - 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index bbc175d4213d..5178b06c338c 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -186,6 +186,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS); BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY); + BUILD_BUG_ON(FAN_MODIFY_DIR != FS_MODIFY_DIR); BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE); BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE); BUILD_BUG_ON(FAN_OPEN != FS_OPEN); diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index b43d3f5bd9ea..00fb87c975d6 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -208,6 +208,23 @@ static inline void fsnotify_modify(struct file *file) } /* + * fsnotify_modifydir - directory contents were changed + * (as a result of rename, creat, unlink, etc.) + */ +static inline void fsnotify_modify_dir(struct path
[RFC 1/2] fanotify: new event FAN_MODIFY_DIR
Fanotify currently does not report directory modification events
(rename, unlink, etc.). But these events are essential for about half of
concievable fanotify use cases, especially:
- file system indexers / desktop search tools
- file synchronization tools (like Dropbox, Nextcloud, etc.),
online backup tools
and pretty much any app that needs to maintain and up-to-date internal
representation of current contents of the file system.
All applications of the above classes that I'm aware of currently use
recursive inotify watches, which do not scale (my home dir has ~200k
directories, creating all the watches takes ~2min and eats several tens
of MB of kernel memory).
There have been many calls for such a feature, pretty much since the
creation of fanotify in 2009:
* By GNOME developers:
https://wiki.gnome.org/BastienNocera/KernelWishlist#VFS.2C_filesystems
* By KDE developers:
http://lkml.kernel.org/r/201211011352.42476.Martin%40lichtvoll.de
'Better support for (desktop) file search / indexing applications'
* And others:
http://lkml.kernel.org/r/AANLkTi=owK=WZW4oNtpm5WpAZhqCQUdTR2K5gzJ_MqZ+%40mail.gmail.com
'Fanotify mv/rename?'
http://lkml.kernel.org/r/1238158043.23703.20.camel%40fatty
'Issues with using fanotify for a filesystem indexer'
Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
contents of a directory change (a directory entry is added, removed or
renamed). This covers all the currently missing events: rename, unlink,
mknod, mkdir, rmdir, etc.
Included with the event is a file descriptor to the modified directory
but no further info. The userspace application is expected to rescan the
whole directory and update its model accordingly. This needs to be done
carefully to prevent race conditions. A cross-directory rename generates
two FAN_MODIFY_DIR events.
This minimalistic approach has several advantages:
* Does not require changes to the fanotify userspace API or the
fsnotify in-kernel framework, apart from adding a new event.
Especially doesn't complicate it by adding string fields.
* Has simple and clear semantics, even with multiple renames occurring
in parallel etc. In case of any inconsistencies, one can simply wait
for a while and rescan again.
* FAN_MODIFY_DIR events are easily merged in case of multiple
operations on a directory (e.g. deleting all files).
Signed-off-by: Filip Štědronský
---
An alternative might be to create wrapper functions like
vfs_path_(rename|unlink|...). They could also take care of calling
security_path_(rename|unlink|...), which is currently also up to
the indvidual callers (possibly with a flag because it might not
be always desired).
An alternative was proposed by Amir Goldstein in several long series of
patches that add superblock-scoped (as opposed to vfsmount-scoped)
fanotify watches and specific dentry manipulation events with filenames:
http://lkml.kernel.org/r/1481984434-13293-1-git-send-email-amir73il%40gmail.com
http://lkml.kernel.org/r/1482247207-4424-1-git-send-email-amir73il%40gmail.com
http://lkml.kernel.org/r/1476126784-12520-1-git-send-email-amir73il%40gmail.com
http://lkml.kernel.org/r/1489411223-12081-1-git-send-email-amir73il%40gmail.com
There is large but not complete overlap between that proposal and
mine (which is was originally created over a year ago, before Amir's
work, but never posted).
I think the superblock watch idea is very interesting because it might
in the future allow reporing fsnotify events from remote and virtual
filesystems. So I'm posting this more as a potential source of more
ideas for discussion, or a fallback proposal in case Amir's patches
don't make it.
---
fs/notify/fanotify/fanotify.c| 1 +
include/linux/fsnotify.h | 17 +
include/linux/fsnotify_backend.h | 1 +
include/uapi/linux/fanotify.h| 5 -
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index bbc175d4213d..5178b06c338c 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -186,6 +186,7 @@ static int fanotify_handle_event(struct fsnotify_group
*group,
BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
+ BUILD_BUG_ON(FAN_MODIFY_DIR != FS_MODIFY_DIR);
BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index b43d3f5bd9ea..00fb87c975d6 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -208,6 +208,23 @@ static inline void fsnotify_modify(struct file *file)
}
/*
+ * fsnotify_modifydir - directory contents were changed
+ * (as a result of rename, creat, unlink, etc.)
+ */
+static inline void fsnotify_modify_dir(struct path *path)
+{
+ struct inode
[PATCH V2] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
This is a workaround for VHT-enabled STAs which break the spec
and have the VHT-MCS Rx map filled in with value 3 for all eight
spacial streams, an example is AR9462 in AP mode.
As per spec, in section 22.1.1 Introduction to the VHT PHY
A VHT STA shall support at least single spactial stream VHT-MCSs
0 to 7 (transmit and receive) in all supported channel widths.
Some devices in STA mode will get firmware assert when trying to
associate, examples are QCA9377 & QCA6174.
Packet example of broken VHT Cap IE of AR9462:
Tag: VHT Capabilities (IEEE Std 802.11ac/D3.1)
Tag Number: VHT Capabilities (IEEE Std 802.11ac/D3.1) (191)
Tag length: 12
VHT Capabilities Info: 0x
VHT Supported MCS Set
Rx MCS Map: 0x
..11 = Rx 1 SS: Not Supported (0x0003)
11.. = Rx 2 SS: Not Supported (0x0003)
..11 = Rx 3 SS: Not Supported (0x0003)
11.. = Rx 4 SS: Not Supported (0x0003)
..11 = Rx 5 SS: Not Supported (0x0003)
11.. = Rx 6 SS: Not Supported (0x0003)
..11 = Rx 7 SS: Not Supported (0x0003)
11.. = Rx 8 SS: Not Supported (0x0003)
...0 = Rx Highest Long GI Data Rate (in Mb/s, 0 =
subfield not in use): 0x
Tx MCS Map: 0x
...0 = Tx Highest Long GI Data Rate (in Mb/s, 0 =
subfield not in use): 0x
Signed-off-by: Filip Matusiak <filip.matus...@tieto.com>
---
net/mac80211/vht.c | 16
1 file changed, 16 insertions(+)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ee71576..6832bf6 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -270,6 +270,22 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
ieee80211_sub_if_data *sdata,
vht_cap->vht_mcs.tx_mcs_map |= cpu_to_le16(peer_tx << i * 2);
}
+ /*
+* This is a workaround for VHT-enabled STAs which break the spec
+* and have the VHT-MCS Rx map filled in with value 3 for all eight
+* spacial streams, an example is AR9462.
+*
+* As per spec, in section 22.1.1 Introduction to the VHT PHY
+* A VHT STA shall support at least single spactial stream VHT-MCSs
+* 0 to 7 (transmit and receive) in all supported channel widths.
+*/
+ if (vht_cap->vht_mcs.rx_mcs_map == cpu_to_le16(0x)) {
+ vht_cap->vht_supported = false;
+ sdata_info(sdata, "Ignoring VHT IE from %pM due to invalid
rx_mcs_map\n",
+ sta->addr);
+ return;
+ }
+
/* finally set up the bandwidth */
switch (vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
case IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ:
--
2.7.4
[PATCH V2] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
This is a workaround for VHT-enabled STAs which break the spec
and have the VHT-MCS Rx map filled in with value 3 for all eight
spacial streams, an example is AR9462 in AP mode.
As per spec, in section 22.1.1 Introduction to the VHT PHY
A VHT STA shall support at least single spactial stream VHT-MCSs
0 to 7 (transmit and receive) in all supported channel widths.
Some devices in STA mode will get firmware assert when trying to
associate, examples are QCA9377 & QCA6174.
Packet example of broken VHT Cap IE of AR9462:
Tag: VHT Capabilities (IEEE Std 802.11ac/D3.1)
Tag Number: VHT Capabilities (IEEE Std 802.11ac/D3.1) (191)
Tag length: 12
VHT Capabilities Info: 0x
VHT Supported MCS Set
Rx MCS Map: 0x
..11 = Rx 1 SS: Not Supported (0x0003)
11.. = Rx 2 SS: Not Supported (0x0003)
..11 = Rx 3 SS: Not Supported (0x0003)
11.. = Rx 4 SS: Not Supported (0x0003)
..11 = Rx 5 SS: Not Supported (0x0003)
11.. = Rx 6 SS: Not Supported (0x0003)
..11 = Rx 7 SS: Not Supported (0x0003)
11.. = Rx 8 SS: Not Supported (0x0003)
...0 = Rx Highest Long GI Data Rate (in Mb/s, 0 =
subfield not in use): 0x
Tx MCS Map: 0x
...0 = Tx Highest Long GI Data Rate (in Mb/s, 0 =
subfield not in use): 0x
Signed-off-by: Filip Matusiak
---
net/mac80211/vht.c | 16
1 file changed, 16 insertions(+)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ee71576..6832bf6 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -270,6 +270,22 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
ieee80211_sub_if_data *sdata,
vht_cap->vht_mcs.tx_mcs_map |= cpu_to_le16(peer_tx << i * 2);
}
+ /*
+* This is a workaround for VHT-enabled STAs which break the spec
+* and have the VHT-MCS Rx map filled in with value 3 for all eight
+* spacial streams, an example is AR9462.
+*
+* As per spec, in section 22.1.1 Introduction to the VHT PHY
+* A VHT STA shall support at least single spactial stream VHT-MCSs
+* 0 to 7 (transmit and receive) in all supported channel widths.
+*/
+ if (vht_cap->vht_mcs.rx_mcs_map == cpu_to_le16(0x)) {
+ vht_cap->vht_supported = false;
+ sdata_info(sdata, "Ignoring VHT IE from %pM due to invalid
rx_mcs_map\n",
+ sta->addr);
+ return;
+ }
+
/* finally set up the bandwidth */
switch (vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
case IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ:
--
2.7.4
[PATCH] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
This is a workaround for VHT-enabled STAs which break the spec
and have the VHT-MCS Rx map filled in with value 3 for all eight
spacial streams.
As per spec, in section 22.1.1 Introduction to the VHT PHY
A VHT STA shall support at least single spactial stream VHT-MCSs
0 to 7 (transmit and receive) in all supported channel widths.
For some devices firmware asserts if such situation occurs.
Signed-off-by: Filip Matusiak <filip.matus...@tieto.com>
---
net/mac80211/vht.c | 16
1 file changed, 16 insertions(+)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ee71576..ce93cff 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -270,6 +270,22 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
ieee80211_sub_if_data *sdata,
vht_cap->vht_mcs.tx_mcs_map |= cpu_to_le16(peer_tx << i * 2);
}
+ /*
+* This is a workaround for VHT-enabled STAs which break the spec
+* and have the VHT-MCS Rx map filled in with value 3 for all eight
+* spacial streams.
+*
+* As per spec, in section 22.1.1 Introduction to the VHT PHY
+* A VHT STA shall support at least single spactial stream VHT-MCSs
+* 0 to 7 (transmit and receive) in all supported channel widths.
+*/
+ if (vht_cap->vht_mcs.rx_mcs_map == cpu_to_le16(0x)) {
+ vht_cap->vht_supported = false;
+ sdata_info(sdata, "Ignoring VHT IE from %pM due to invalid
rx_mcs_map\n",
+ sta->addr);
+ return;
+ }
+
/* finally set up the bandwidth */
switch (vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
case IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ:
--
2.7.4
[PATCH] mac80211: Ignore VHT IE from peer with wrong rx_mcs_map
This is a workaround for VHT-enabled STAs which break the spec
and have the VHT-MCS Rx map filled in with value 3 for all eight
spacial streams.
As per spec, in section 22.1.1 Introduction to the VHT PHY
A VHT STA shall support at least single spactial stream VHT-MCSs
0 to 7 (transmit and receive) in all supported channel widths.
For some devices firmware asserts if such situation occurs.
Signed-off-by: Filip Matusiak
---
net/mac80211/vht.c | 16
1 file changed, 16 insertions(+)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index ee71576..ce93cff 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -270,6 +270,22 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct
ieee80211_sub_if_data *sdata,
vht_cap->vht_mcs.tx_mcs_map |= cpu_to_le16(peer_tx << i * 2);
}
+ /*
+* This is a workaround for VHT-enabled STAs which break the spec
+* and have the VHT-MCS Rx map filled in with value 3 for all eight
+* spacial streams.
+*
+* As per spec, in section 22.1.1 Introduction to the VHT PHY
+* A VHT STA shall support at least single spactial stream VHT-MCSs
+* 0 to 7 (transmit and receive) in all supported channel widths.
+*/
+ if (vht_cap->vht_mcs.rx_mcs_map == cpu_to_le16(0x)) {
+ vht_cap->vht_supported = false;
+ sdata_info(sdata, "Ignoring VHT IE from %pM due to invalid
rx_mcs_map\n",
+ sta->addr);
+ return;
+ }
+
/* finally set up the bandwidth */
switch (vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK) {
case IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ:
--
2.7.4
[PATCH v3] kdb: use kstrto* instead of simple_strto
Replace simple_strto with appropriate kstrto functions as recommended
by checkpatch, removing no longer required variables.
Change pid, sig types from long to pid_t, int respectively.
Signed-off-by: Filip Ayazi
Reviewed-by: Daniel Thompson
---
changes from v2:
store kstrto return value in a variable instead of calling it within if itself
delete != 0 from ifs
change pid type to pid_t, use kstrtoint for parsing
remove another temporary variable (val in kdbgetularg)
(All pointed out by Daniel Thompson, who reviewed v2)
kernel/debug/kdb/kdb_main.c | 74 +
1 file changed, 28 insertions(+), 46 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..513250c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -289,6 +289,7 @@ static char *kdballocenv(size_t bytes)
static int kdbgetulenv(const char *match, unsigned long *value)
{
char *ep;
+ int err;
ep = kdbgetenv(match);
if (!ep)
@@ -296,7 +297,9 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ err = kstrtoul(ep, 0, value);
+ if (err)
+ return KDB_BADINT;
return 0;
}
@@ -334,41 +337,22 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
- unsigned long val;
-
- val = simple_strtoul(arg, , 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, , 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, value))
+ if (kstrtoul(arg, 16, value))
return KDB_BADINT;
- }
-
- *value = val;
return 0;
}
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
- u64 val;
-
- val = simple_strtoull(arg, , 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, , 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, value))
+ if (kstrtoull(arg, 16, value))
return KDB_BADINT;
- }
-
- *value = val;
return 0;
}
@@ -379,7 +363,7 @@ int kdbgetu64arg(const char *arg, u64 *value)
*/
int kdb_set(int argc, const char **argv)
{
- int i;
+ int i, err;
char *ep;
size_t varlen, vallen;
@@ -402,10 +386,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], "KDBDEBUG") == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], , 0);
- if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
+ err = kstrtouint(argv[2], 0, );
+ if (err || debugflags & ~KDB_DEBUG_FLAG_MASK) {
kdb_printf("kdb: illegal debug flags '%s'\n",
argv[2]);
return 0;
@@ -1588,10 +1571,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' && argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, , 10);
+ valid = !kstrtoint(argv[0]+4, 10, );
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], "md") == 0)
@@ -2080,6 +2061,7 @@ static int kdb_dmesg(int argc, const char **argv)
{
int diag;
int logging;
+ int err;
int lines = 0;
int adjust = 0;
int n = 0;
@@ -2091,13 +2073,12 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc > 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], , 0);
- if (*cp)
+ err = kstrtoint(argv[1], 0, );
+ if (err)
lines = 0;
if (argc > 1) {
- adjust = simple_strtoul(argv[2], , 0);
- if (*cp || adjust < 0)
+ err = kstrtoint(argv[2], 0, );
+ if (err || adjust < 0)
adjust = 0;
}
}
@@ -2436,16 +2417,17 @@ static int kdb_help(int argc, const char **argv)
*/
static int kdb_kill(int argc, const char **argv)
{
- long si
Re: [PATCH v2] kdb: use kstrto* instead of simple_strto
On 06/01/2015 11:28 AM, Daniel Thompson wrote:
> On 31/05/15 18:59, Filip Ayazi wrote:
>> Replace simple_strto with appropriate kstrto functions as recommended
>> by checkpatch, change pid, sig types to unsigned int, int respectively.
>
> A changelog describing the changes are in v2 would be nice here.
My apologies, I will certainly include a changelog in the next version.
>
>> Signed-off-by: Filip Ayazi
>> ---
>> kernel/debug/kdb/kdb_main.c | 56
>> +++--
>> 1 file changed, 19 insertions(+), 37 deletions(-)
>
> I can't find any bugs introduced by this patch although I do have some
> cosmetic nitpicks below. You can add my reviewed-by to the next release:
>
> Reviewed-by: Daniel Thompson
Thanks.
>
>> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
>> index 4121345..6458330 100644
>> --- a/kernel/debug/kdb/kdb_main.c
>> +++ b/kernel/debug/kdb/kdb_main.c
>> @@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
>> *value)
>> if (strlen(ep) == 0)
>> return KDB_NOENVVALUE;
>>
>> -*value = simple_strtoul(ep, NULL, 0);
>> +if (kstrtoul(ep, 0, value) != 0)
> ^
> Checking for != 0 is what if does anyway. A generally clearer code pattern
> for kernel code that returns 0 on success is:
>
> err = kstrtoul(...);
> if (err)
> handle_error();
>
I wasn't sure whether to write the != 0 as it has no function there,
but it seemed more clear with it (as I am used to returning true on success).
It will be fixed in the next version.
>
>> +return KDB_BADINT;
>>
>> return 0;
>> }
>> @@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
>>*/
>> int kdbgetularg(const char *arg, unsigned long *value)
>> {
>> -char *endp;
>> unsigned long val;
>>
>> -val = simple_strtoul(arg, , 0);
>> -
>> -if (endp == arg) {
>> -/*
>> - * Also try base 16, for us folks too lazy to type the
>> - * leading 0x...
>> - */
>> -val = simple_strtoul(arg, , 16);
>> -if (endp == arg)
>> +/*
>> + * Also try base 16, for us folks too lazy to type the
>> + * leading 0x...
>> + */
>> +if (kstrtoul(arg, 0, ) != 0)
>> +if (kstrtoul(arg, 16, ) != 0)
>> return KDB_BADINT;
>
> Whether or not it is good taste to introduce 'err' here is debatable since it
> makes the code flow pretty verbose. However it would still be good to get rid
> of the '!= 0'.
I'd prefer not to introduce err, as I believe its quite clear what the code
does.
>
>> -}
>>
>> *value = val;
>
> kstrtoul() does not write to its argument unless it is successful. That means
> we no longer need a temporary variable here; just pass value into kstrtoul().
>
>
>> @@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
>>
>> int kdbgetu64arg(const char *arg, u64 *value)
>> {
>> -char *endp;
>> u64 val;
>>
>> -val = simple_strtoull(arg, , 0);
>> -
>> -if (endp == arg) {
>> -
>> -val = simple_strtoull(arg, , 16);
>> -if (endp == arg)
>> +if (kstrtoull(arg, 0, ) != 0)
>> +if (kstrtoull(arg, 16, ) != 0)
>> return KDB_BADINT;
>
> +1
>
>
>> -}
>>
>> *value = val;
>>
>> @@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
>>*/
>> if (strcmp(argv[1], "KDBDEBUG") == 0) {
>> unsigned int debugflags;
>> -char *cp;
>>
>> -debugflags = simple_strtoul(argv[2], , 0);
>> -if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
>> +if (kstrtouint(argv[2], 0, ) != 0
>> +|| debugflags & ~KDB_DEBUG_FLAG_MASK) {
>
> +1
>
>
>> kdb_printf("kdb: illegal debug flags '%s'\n",
>> argv[2]);
>> return 0;
>> @@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
>> if (!argv[0][3])
>> valid = 1;
>> else if (argv[0][3] == 'c' && argv[0][4]) {
>> -char *p;
>> -repeat = simple_strtoul(argv[0] + 4, , 10);
>> +valid = !kstrtoint(argv[0]+4, 10, );
>> mdcount = ((repeat * bytesperword) + 15) / 16;
>> -va
Re: [PATCH v2] kdb: use kstrto* instead of simple_strto
On 06/01/2015 11:28 AM, Daniel Thompson wrote:
On 31/05/15 18:59, Filip Ayazi wrote:
Replace simple_strto with appropriate kstrto functions as recommended
by checkpatch, change pid, sig types to unsigned int, int respectively.
A changelog describing the changes are in v2 would be nice here.
My apologies, I will certainly include a changelog in the next version.
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
kernel/debug/kdb/kdb_main.c | 56
+++--
1 file changed, 19 insertions(+), 37 deletions(-)
I can't find any bugs introduced by this patch although I do have some
cosmetic nitpicks below. You can add my reviewed-by to the next release:
Reviewed-by: Daniel Thompson daniel.thomp...@linaro.org
Thanks.
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..6458330 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
-*value = simple_strtoul(ep, NULL, 0);
+if (kstrtoul(ep, 0, value) != 0)
^
Checking for != 0 is what if does anyway. A generally clearer code pattern
for kernel code that returns 0 on success is:
err = kstrtoul(...);
if (err)
handle_error();
I wasn't sure whether to write the != 0 as it has no function there,
but it seemed more clear with it (as I am used to returning true on success).
It will be fixed in the next version.
+return KDB_BADINT;
return 0;
}
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
-char *endp;
unsigned long val;
-val = simple_strtoul(arg, endp, 0);
-
-if (endp == arg) {
-/*
- * Also try base 16, for us folks too lazy to type the
- * leading 0x...
- */
-val = simple_strtoul(arg, endp, 16);
-if (endp == arg)
+/*
+ * Also try base 16, for us folks too lazy to type the
+ * leading 0x...
+ */
+if (kstrtoul(arg, 0, val) != 0)
+if (kstrtoul(arg, 16, val) != 0)
return KDB_BADINT;
Whether or not it is good taste to introduce 'err' here is debatable since it
makes the code flow pretty verbose. However it would still be good to get rid
of the '!= 0'.
I'd prefer not to introduce err, as I believe its quite clear what the code
does.
-}
*value = val;
kstrtoul() does not write to its argument unless it is successful. That means
we no longer need a temporary variable here; just pass value into kstrtoul().
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
int kdbgetu64arg(const char *arg, u64 *value)
{
-char *endp;
u64 val;
-val = simple_strtoull(arg, endp, 0);
-
-if (endp == arg) {
-
-val = simple_strtoull(arg, endp, 16);
-if (endp == arg)
+if (kstrtoull(arg, 0, val) != 0)
+if (kstrtoull(arg, 16, val) != 0)
return KDB_BADINT;
+1
-}
*value = val;
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], KDBDEBUG) == 0) {
unsigned int debugflags;
-char *cp;
-debugflags = simple_strtoul(argv[2], cp, 0);
-if (cp == argv[2] || debugflags ~KDB_DEBUG_FLAG_MASK) {
+if (kstrtouint(argv[2], 0, debugflags) != 0
+|| debugflags ~KDB_DEBUG_FLAG_MASK) {
+1
kdb_printf(kdb: illegal debug flags '%s'\n,
argv[2]);
return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' argv[0][4]) {
-char *p;
-repeat = simple_strtoul(argv[0] + 4, p, 10);
+valid = !kstrtoint(argv[0]+4, 10, repeat);
mdcount = ((repeat * bytesperword) + 15) / 16;
-valid = !*p;
}
last_repeat = repeat;
Interesting! Your changes mean we no longer store the, known invalid, return
value from simple_strtoul into last_repeat. Since last_repeat is a static
variable and can influence future commands that's a *good* change.
} else if (strcmp(argv[0], md) == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc 2)
return KDB_ARGCOUNT;
if (argc) {
-char *cp;
-lines = simple_strtol(argv[1], cp, 0);
-if (*cp)
+if (kstrtoint(argv[1], 0, lines) != 0)
lines = 0;
if (argc 1) {
-adjust = simple_strtoul(argv[2], cp, 0);
-if (*cp || adjust 0)
+if (kstrtoint(argv[2], 0, adjust) != 0 || adjust 0
[PATCH v3] kdb: use kstrto* instead of simple_strto
Replace simple_strto with appropriate kstrto functions as recommended
by checkpatch, removing no longer required variables.
Change pid, sig types from long to pid_t, int respectively.
Signed-off-by: Filip Ayazi filipay...@gmail.com
Reviewed-by: Daniel Thompson daniel.thomp...@linaro.org
---
changes from v2:
store kstrto return value in a variable instead of calling it within if itself
delete != 0 from ifs
change pid type to pid_t, use kstrtoint for parsing
remove another temporary variable (val in kdbgetularg)
(All pointed out by Daniel Thompson, who reviewed v2)
kernel/debug/kdb/kdb_main.c | 74 +
1 file changed, 28 insertions(+), 46 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..513250c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -289,6 +289,7 @@ static char *kdballocenv(size_t bytes)
static int kdbgetulenv(const char *match, unsigned long *value)
{
char *ep;
+ int err;
ep = kdbgetenv(match);
if (!ep)
@@ -296,7 +297,9 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ err = kstrtoul(ep, 0, value);
+ if (err)
+ return KDB_BADINT;
return 0;
}
@@ -334,41 +337,22 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
- unsigned long val;
-
- val = simple_strtoul(arg, endp, 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, endp, 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, value))
+ if (kstrtoul(arg, 16, value))
return KDB_BADINT;
- }
-
- *value = val;
return 0;
}
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
- u64 val;
-
- val = simple_strtoull(arg, endp, 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, endp, 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, value))
+ if (kstrtoull(arg, 16, value))
return KDB_BADINT;
- }
-
- *value = val;
return 0;
}
@@ -379,7 +363,7 @@ int kdbgetu64arg(const char *arg, u64 *value)
*/
int kdb_set(int argc, const char **argv)
{
- int i;
+ int i, err;
char *ep;
size_t varlen, vallen;
@@ -402,10 +386,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], KDBDEBUG) == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], cp, 0);
- if (cp == argv[2] || debugflags ~KDB_DEBUG_FLAG_MASK) {
+ err = kstrtouint(argv[2], 0, debugflags);
+ if (err || debugflags ~KDB_DEBUG_FLAG_MASK) {
kdb_printf(kdb: illegal debug flags '%s'\n,
argv[2]);
return 0;
@@ -1588,10 +1571,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, p, 10);
+ valid = !kstrtoint(argv[0]+4, 10, repeat);
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], md) == 0)
@@ -2080,6 +2061,7 @@ static int kdb_dmesg(int argc, const char **argv)
{
int diag;
int logging;
+ int err;
int lines = 0;
int adjust = 0;
int n = 0;
@@ -2091,13 +2073,12 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], cp, 0);
- if (*cp)
+ err = kstrtoint(argv[1], 0, lines);
+ if (err)
lines = 0;
if (argc 1) {
- adjust = simple_strtoul(argv[2], cp, 0);
- if (*cp || adjust 0)
+ err = kstrtoint(argv[2], 0, adjust);
+ if (err || adjust 0)
adjust = 0;
}
}
@@ -2436,16 +2417,17 @@ static int kdb_help(int argc, const char **argv)
*/
static int kdb_kill(int argc, const char
[PATCH v2] kdb: use kstrto* instead of simple_strto
Replace simple_strto with appropriate kstrto functions as recommended
by checkpatch, change pid, sig types to unsigned int, int respectively.
Signed-off-by: Filip Ayazi
---
kernel/debug/kdb/kdb_main.c | 56 +++--
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..6458330 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ if (kstrtoul(ep, 0, value) != 0)
+ return KDB_BADINT;
return 0;
}
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
unsigned long val;
- val = simple_strtoul(arg, , 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, , 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, ) != 0)
+ if (kstrtoul(arg, 16, ) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
u64 val;
- val = simple_strtoull(arg, , 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, , 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, ) != 0)
+ if (kstrtoull(arg, 16, ) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], "KDBDEBUG") == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], , 0);
- if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
+ if (kstrtouint(argv[2], 0, ) != 0
+ || debugflags & ~KDB_DEBUG_FLAG_MASK) {
kdb_printf("kdb: illegal debug flags '%s'\n",
argv[2]);
return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' && argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, , 10);
+ valid = !kstrtoint(argv[0]+4, 10, );
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], "md") == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc > 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], , 0);
- if (*cp)
+ if (kstrtoint(argv[1], 0, ) != 0)
lines = 0;
if (argc > 1) {
- adjust = simple_strtoul(argv[2], , 0);
- if (*cp || adjust < 0)
+ if (kstrtoint(argv[2], 0, ) != 0 || adjust < 0)
adjust = 0;
}
}
@@ -2436,16 +2420,15 @@ static int kdb_help(int argc, const char **argv)
*/
static int kdb_kill(int argc, const char **argv)
{
- long sig, pid;
- char *endp;
+ unsigned int pid;
+ int sig;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], , 0);
- if (*endp)
+ if (kstrtoint(argv[1], 0, ) != 0)
return KDB_BADINT;
if (sig >= 0) {
kdb_printf("Invalid signal parameter.<-signal>\n");
@@ -2453,8 +2436,7 @@ static int kdb_kill(int argc, const char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], , 0);
- if (*endp)
+ if (kstrtouint(argv[2], 0, ) != 0)
return KDB_BADINT;
if (pid <= 0) {
kdb_printf("Process ID must be large than 0.\n");
--
1.9.1
--
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] kdb: use kstrto instead of simple_strto
On 05/31/2015 02:29 PM, Alexey Dobriyan wrote:
Replace obsolete simple_strto* calls with appropriate kstrto*
functions.
static int kdb_kill(int argc, const char **argv)
{
long sig, pid;
- char *endp;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], , 0);
- if (*endp)
+ if (kstrtol(argv[1], 0, ) != 0)
return KDB_BADINT;
if (sig >= 0) {
kdb_printf("Invalid signal parameter.<-signal>\n");
-2453,8 +2435,7 static int kdb_kill(int argc, const
char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], , 0);
- if (*endp)
+ if (kstrtol(argv[2], 0, ) != 0)
return KDB_BADINT;
if (pid <= 0) {
kdb_printf("Process ID must be large than 0.\n");
This patch does easy mistake, namely trivial switch from simple_strtoul() to
kstrtoul().
But you have to remember that there are NO simple_strtoint() or something like
that,
so real type of data is anchor not function name.
In the code above data are "sig" and "pid" which are "int" or "unsigned int" at
most.
Later "sig" is negated, so "int sig" is OK.
Thank you for the feedback, I know there is no simple_strtoint and the patch
uses kstrtoint/uint where the variable is declared as int/unsigned int
(not doing
so would cause compilation warnings).
Pids are always unsigned logically so "unsigned int" is correct.
Don't be afraid to switch type of variable to correct one. Most of
the time "unsigned long foo" is wrong becase C-derived api didn't offer
anything better.
pid and sig variables are declared as long, I didn't know why so I
didn't change
that, I will change their types to uint and int as you said and resend
the patch soon.
--
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] kdb: use kstrto instead of simple_strto
Replace obsolete simple_strto* calls with appropriate kstrto* functions.
Signed-off-by: Filip Ayazi
---
kernel/debug/kdb/kdb_main.c | 53 +++--
1 file changed, 17 insertions(+), 36 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..0e0371c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ if (kstrtoul(ep, 0, value) != 0)
+ return KDB_BADINT;
return 0;
}
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
unsigned long val;
- val = simple_strtoul(arg, , 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, , 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, ) != 0)
+ if (kstrtoul(arg, 16, ) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
u64 val;
- val = simple_strtoull(arg, , 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, , 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, ) != 0)
+ if (kstrtoull(arg, 16, ) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], "KDBDEBUG") == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], , 0);
- if (cp == argv[2] || debugflags & ~KDB_DEBUG_FLAG_MASK) {
+ if (kstrtouint(argv[2], 0, ) != 0
+ || debugflags & ~KDB_DEBUG_FLAG_MASK) {
kdb_printf("kdb: illegal debug flags '%s'\n",
argv[2]);
return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' && argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, , 10);
+ valid = !kstrtoint(argv[0]+4, 10, );
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], "md") == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc > 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], , 0);
- if (*cp)
+ if (kstrtoint(argv[1], 0, ) != 0)
lines = 0;
if (argc > 1) {
- adjust = simple_strtoul(argv[2], , 0);
- if (*cp || adjust < 0)
+ if (kstrtoint(argv[2], 0, ) != 0 || adjust < 0)
adjust = 0;
}
}
@@ -2437,15 +2421,13 @@ static int kdb_help(int argc, const char **argv)
static int kdb_kill(int argc, const char **argv)
{
long sig, pid;
- char *endp;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], , 0);
- if (*endp)
+ if (kstrtol(argv[1], 0, ) != 0)
return KDB_BADINT;
if (sig >= 0) {
kdb_printf("Invalid signal parameter.<-signal>\n");
@@ -2453,8 +2435,7 @@ static int kdb_kill(int argc, const char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], , 0);
- if (*endp)
+ if (kstrtol(argv[2], 0, ) != 0)
return KDB_BADINT;
if (pid <= 0) {
kdb_printf("Process ID must be large than 0.\n");
--
1.9.1
--
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] kdb: use kstrto instead of simple_strto
On 05/31/2015 02:29 PM, Alexey Dobriyan wrote:
Replace obsolete simple_strto* calls with appropriate kstrto*
functions.
static int kdb_kill(int argc, const char **argv)
{
long sig, pid;
- char *endp;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], endp, 0);
- if (*endp)
+ if (kstrtol(argv[1], 0, sig) != 0)
return KDB_BADINT;
if (sig = 0) {
kdb_printf(Invalid signal parameter.-signal\n);
at at -2453,8 +2435,7 at at static int kdb_kill(int argc, const
char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], endp, 0);
- if (*endp)
+ if (kstrtol(argv[2], 0, pid) != 0)
return KDB_BADINT;
if (pid = 0) {
kdb_printf(Process ID must be large than 0.\n);
This patch does easy mistake, namely trivial switch from simple_strtoul() to
kstrtoul().
But you have to remember that there are NO simple_strtoint() or something like
that,
so real type of data is anchor not function name.
In the code above data are sig and pid which are int or unsigned int at
most.
Later sig is negated, so int sig is OK.
Thank you for the feedback, I know there is no simple_strtoint and the patch
uses kstrtoint/uint where the variable is declared as int/unsigned int
(not doing
so would cause compilation warnings).
Pids are always unsigned logically so unsigned int is correct.
Don't be afraid to switch type of variable to correct one. Most of
the time unsigned long foo is wrong becase C-derived api didn't offer
anything better.
pid and sig variables are declared as long, I didn't know why so I
didn't change
that, I will change their types to uint and int as you said and resend
the patch soon.
--
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 v2] kdb: use kstrto* instead of simple_strto
Replace simple_strto with appropriate kstrto functions as recommended
by checkpatch, change pid, sig types to unsigned int, int respectively.
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
kernel/debug/kdb/kdb_main.c | 56 +++--
1 file changed, 19 insertions(+), 37 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..6458330 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ if (kstrtoul(ep, 0, value) != 0)
+ return KDB_BADINT;
return 0;
}
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
unsigned long val;
- val = simple_strtoul(arg, endp, 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, endp, 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, val) != 0)
+ if (kstrtoul(arg, 16, val) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
u64 val;
- val = simple_strtoull(arg, endp, 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, endp, 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, val) != 0)
+ if (kstrtoull(arg, 16, val) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], KDBDEBUG) == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], cp, 0);
- if (cp == argv[2] || debugflags ~KDB_DEBUG_FLAG_MASK) {
+ if (kstrtouint(argv[2], 0, debugflags) != 0
+ || debugflags ~KDB_DEBUG_FLAG_MASK) {
kdb_printf(kdb: illegal debug flags '%s'\n,
argv[2]);
return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, p, 10);
+ valid = !kstrtoint(argv[0]+4, 10, repeat);
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], md) == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], cp, 0);
- if (*cp)
+ if (kstrtoint(argv[1], 0, lines) != 0)
lines = 0;
if (argc 1) {
- adjust = simple_strtoul(argv[2], cp, 0);
- if (*cp || adjust 0)
+ if (kstrtoint(argv[2], 0, adjust) != 0 || adjust 0)
adjust = 0;
}
}
@@ -2436,16 +2420,15 @@ static int kdb_help(int argc, const char **argv)
*/
static int kdb_kill(int argc, const char **argv)
{
- long sig, pid;
- char *endp;
+ unsigned int pid;
+ int sig;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], endp, 0);
- if (*endp)
+ if (kstrtoint(argv[1], 0, sig) != 0)
return KDB_BADINT;
if (sig = 0) {
kdb_printf(Invalid signal parameter.-signal\n);
@@ -2453,8 +2436,7 @@ static int kdb_kill(int argc, const char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], endp, 0);
- if (*endp)
+ if (kstrtouint(argv[2], 0, pid) != 0)
return KDB_BADINT;
if (pid = 0) {
kdb_printf(Process ID must be large than 0.\n);
--
1.9.1
--
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
[PATCH] kdb: use kstrto instead of simple_strto
Replace obsolete simple_strto* calls with appropriate kstrto* functions.
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
kernel/debug/kdb/kdb_main.c | 53 +++--
1 file changed, 17 insertions(+), 36 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4121345..0e0371c 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -296,7 +296,8 @@ static int kdbgetulenv(const char *match, unsigned long
*value)
if (strlen(ep) == 0)
return KDB_NOENVVALUE;
- *value = simple_strtoul(ep, NULL, 0);
+ if (kstrtoul(ep, 0, value) != 0)
+ return KDB_BADINT;
return 0;
}
@@ -334,20 +335,15 @@ int kdbgetintenv(const char *match, int *value)
*/
int kdbgetularg(const char *arg, unsigned long *value)
{
- char *endp;
unsigned long val;
- val = simple_strtoul(arg, endp, 0);
-
- if (endp == arg) {
- /*
-* Also try base 16, for us folks too lazy to type the
-* leading 0x...
-*/
- val = simple_strtoul(arg, endp, 16);
- if (endp == arg)
+ /*
+* Also try base 16, for us folks too lazy to type the
+* leading 0x...
+*/
+ if (kstrtoul(arg, 0, val) != 0)
+ if (kstrtoul(arg, 16, val) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -356,17 +352,11 @@ int kdbgetularg(const char *arg, unsigned long *value)
int kdbgetu64arg(const char *arg, u64 *value)
{
- char *endp;
u64 val;
- val = simple_strtoull(arg, endp, 0);
-
- if (endp == arg) {
-
- val = simple_strtoull(arg, endp, 16);
- if (endp == arg)
+ if (kstrtoull(arg, 0, val) != 0)
+ if (kstrtoull(arg, 16, val) != 0)
return KDB_BADINT;
- }
*value = val;
@@ -402,10 +392,9 @@ int kdb_set(int argc, const char **argv)
*/
if (strcmp(argv[1], KDBDEBUG) == 0) {
unsigned int debugflags;
- char *cp;
- debugflags = simple_strtoul(argv[2], cp, 0);
- if (cp == argv[2] || debugflags ~KDB_DEBUG_FLAG_MASK) {
+ if (kstrtouint(argv[2], 0, debugflags) != 0
+ || debugflags ~KDB_DEBUG_FLAG_MASK) {
kdb_printf(kdb: illegal debug flags '%s'\n,
argv[2]);
return 0;
@@ -1588,10 +1577,8 @@ static int kdb_md(int argc, const char **argv)
if (!argv[0][3])
valid = 1;
else if (argv[0][3] == 'c' argv[0][4]) {
- char *p;
- repeat = simple_strtoul(argv[0] + 4, p, 10);
+ valid = !kstrtoint(argv[0]+4, 10, repeat);
mdcount = ((repeat * bytesperword) + 15) / 16;
- valid = !*p;
}
last_repeat = repeat;
} else if (strcmp(argv[0], md) == 0)
@@ -2091,13 +2078,10 @@ static int kdb_dmesg(int argc, const char **argv)
if (argc 2)
return KDB_ARGCOUNT;
if (argc) {
- char *cp;
- lines = simple_strtol(argv[1], cp, 0);
- if (*cp)
+ if (kstrtoint(argv[1], 0, lines) != 0)
lines = 0;
if (argc 1) {
- adjust = simple_strtoul(argv[2], cp, 0);
- if (*cp || adjust 0)
+ if (kstrtoint(argv[2], 0, adjust) != 0 || adjust 0)
adjust = 0;
}
}
@@ -2437,15 +2421,13 @@ static int kdb_help(int argc, const char **argv)
static int kdb_kill(int argc, const char **argv)
{
long sig, pid;
- char *endp;
struct task_struct *p;
struct siginfo info;
if (argc != 2)
return KDB_ARGCOUNT;
- sig = simple_strtol(argv[1], endp, 0);
- if (*endp)
+ if (kstrtol(argv[1], 0, sig) != 0)
return KDB_BADINT;
if (sig = 0) {
kdb_printf(Invalid signal parameter.-signal\n);
@@ -2453,8 +2435,7 @@ static int kdb_kill(int argc, const char **argv)
}
sig = -sig;
- pid = simple_strtol(argv[2], endp, 0);
- if (*endp)
+ if (kstrtol(argv[2], 0, pid) != 0)
return KDB_BADINT;
if (pid = 0) {
kdb_printf(Process ID must be large than 0.\n);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc/83xx: add support for mpc8306
On 4/3/2015 10:24 PM, Scott Wood wrote: What are you using PPC_MPC8306 for in your custom board code? Sorry for the late reply, I was a bit busy over the Easter weekend. I'm not currently using it for anything, so I guess I could remove it and just use PPC_MPC830x in my board code. However, I think it's best if this patch is ignored (for now), and I'll submit a patch series for the 8306 and a custom board in a couple of weeks/months, when I find some time to clean up the board code. - Filip -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc/83xx: add support for mpc8306
On 4/3/2015 10:24 PM, Scott Wood wrote: What are you using PPC_MPC8306 for in your custom board code? Sorry for the late reply, I was a bit busy over the Easter weekend. I'm not currently using it for anything, so I guess I could remove it and just use PPC_MPC830x in my board code. However, I think it's best if this patch is ignored (for now), and I'll submit a patch series for the 8306 and a custom board in a couple of weeks/months, when I find some time to clean up the board code. - Filip -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] powerpc/83xx: add support for mpc8306
On 4/3/2015 2:01 PM, Paul Bolle wrote: On Fri, 2015-04-03 at 12:44 +0200, Filip Brozovic wrote: --- a/arch/powerpc/platforms/83xx/Kconfig +++ b/arch/powerpc/platforms/83xx/Kconfig +# used for gpio +config PPC_MPC830x + bool + select ARCH_WANT_OPTIONAL_GPIOLIB + +config PPC_MPC8306 + bool To me these two new Kconfig symbols look pointless: - they have no prompt, so one cannot set them manually; - no other Kconfig symbol selects them; - they do not default to 'y'. I'm not aware of a way to set these symbols to 'y' outside of those three. Is there perhaps a way for kconfig to set these symbols to 'y' that I have missed? Or do you expect to do one of these three things in a separate patch? The idea was that boards in the Kconfig file would select these symbols in order to enable support for the 8306. I mainly wanted to get this patch into mainline in order to make kernel maintenance for a couple of custom in-house developed boards easier. Since these boards are not widely available and our customers are unlikely to want to change and recompile the kernel, I have so far leaned towards not including support for them in mainline. As far as I can see, boards which are included in mainline right now are mostly evaluation boards which are easily available at most electronics distributors. That being said, I don't know what the "official" stance on this is; is adding custom boards encouraged regardless of their availability (e.g. if I develop a custom board with the intention of only ever actually making a single prototype for personal use, should I go and submit patches so that support makes it into the mainline kernel?), or should there be a minimum level of public interest before incorporating custom boards into mainline? If it's the latter, I suppose a solution would be to include support for the Freescale MPC8306SOM in mainline. Of course, this has its own problems, since someone would have to write and maintain it (and I don't have an MPC8306SOM nor the time needed to do maintenance). - Filip -- 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 v2] powerpc/83xx: add support for mpc8306
Add chip specific initialization for the MPC8306.
Signed-off-by: Filip Brozovic
---
Changes from v1:
- Fix multiplatform support for setting QE SNUMs
arch/powerpc/platforms/83xx/Kconfig | 8
arch/powerpc/platforms/83xx/mpc83xx.h | 4
arch/powerpc/platforms/83xx/usb.c | 14 +++---
arch/powerpc/sysdev/qe_lib/qe.c | 10 --
drivers/gpio/Kconfig | 6 +++---
5 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/Kconfig
b/arch/powerpc/platforms/83xx/Kconfig
index 2bdc8c8..904991d 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -113,6 +113,14 @@ config KMETER1
endif
+# used for gpio
+config PPC_MPC830x
+ bool
+ select ARCH_WANT_OPTIONAL_GPIOLIB
+
+config PPC_MPC8306
+ bool
+
# used for usb & gpio
config PPC_MPC831x
bool
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index 0cf74d7..4f21fb9 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -21,6 +21,8 @@
/* system i/o configuration register low */
#define MPC83XX_SICRL_OFFS 0x114
+#define MPC8306_SICRL_USB_MASK 0x003C
+#define MPC8306_SICRL_USB_ULPI 0x
#define MPC834X_SICRL_USB_MASK 0x6000
#define MPC834X_SICRL_USB0 0x2000
#define MPC834X_SICRL_USB1 0x4000
@@ -35,6 +37,8 @@
/* system i/o configuration register high */
#define MPC83XX_SICRH_OFFS 0x118
+#define MPC8306_SICRH_USB_MASK 0x0F00F300
+#define MPC8306_SICRH_USB_ULPI 0x
#define MPC8308_SICRH_USB_MASK 0x000c
#define MPC8308_SICRH_USB_ULPI 0x0004
#define MPC834X_SICRH_USB_UTMI 0x0002
diff --git a/arch/powerpc/platforms/83xx/usb.c
b/arch/powerpc/platforms/83xx/usb.c
index 5c31d82..a9db44b 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
}
#endif /* CONFIG_PPC_MPC834x */
-#ifdef CONFIG_PPC_MPC831x
+#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
int mpc831x_usb_cfg(void)
{
u32 temp;
@@ -128,6 +128,7 @@ int mpc831x_usb_cfg(void)
/* Configure clock */
immr_node = of_get_parent(np);
if (immr_node && (of_device_is_compatible(immr_node,
"fsl,mpc8315-immr") ||
+ of_device_is_compatible(immr_node, "fsl,mpc8306-immr")
||
of_device_is_compatible(immr_node, "fsl,mpc8308-immr")))
clrsetbits_be32(immap + MPC83XX_SCCR_OFFS,
MPC8315_SCCR_USB_MASK,
@@ -139,7 +140,14 @@ int mpc831x_usb_cfg(void)
/* Configure pin mux for ULPI. There is no pin mux for UTMI */
if (prop && !strcmp(prop, "ulpi")) {
- if (of_device_is_compatible(immr_node, "fsl,mpc8308-immr")) {
+ if (of_device_is_compatible(immr_node, "fsl,mpc8306-immr")) {
+ clrsetbits_be32(immap + MPC83XX_SICRL_OFFS,
+ MPC8306_SICRL_USB_MASK,
+ MPC8306_SICRL_USB_ULPI);
+ clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
+ MPC8306_SICRH_USB_MASK,
+ MPC8306_SICRH_USB_ULPI);
+ } else if (of_device_is_compatible(immr_node,
"fsl,mpc8308-immr")) {
clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
MPC8308_SICRH_USB_MASK,
MPC8308_SICRH_USB_ULPI);
@@ -210,7 +218,7 @@ out:
of_node_put(np);
return ret;
}
-#endif /* CONFIG_PPC_MPC831x */
+#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */
#ifdef CONFIG_PPC_MPC837x
int mpc837x_usb_cfg(void)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index c2518cd..f0c45f0 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -285,12 +285,18 @@ static void qe_snums_init(void)
0x28, 0x29, 0x38, 0x39, 0x48, 0x49, 0x58, 0x59,
0x68, 0x69, 0x78, 0x79, 0x80, 0x81,
};
+ static const u8 snum_init_14[] = {
+ 0x88, 0x89, 0x98, 0x99, 0xA8, 0xA9, 0xB8, 0xB9,
+ 0xC8, 0xC9, 0xD8, 0xD9, 0xE8, 0xE9,
+ };
static const u8 *snum_init;
qe_num_of_snum = qe_get_num_of_snums();
if (qe_num_of_snum == 76)
snum_init = snum_init_76;
+ else if (qe_num_of_snum == 14)
+ snum_init = snum_init_14;
else
snum_init = snum_init_46;
@@ -657,8 +663,8 @@ unsigned int qe_get_num_of_snums(void)
prop = of_get_property(qe, "fsl,qe-num-snums", );
if (prop &
[PATCH v2] powerpc/83xx: add support for mpc8306
Add chip specific initialization for the MPC8306.
Signed-off-by: Filip Brozovic fbrozo...@gmail.com
---
Changes from v1:
- Fix multiplatform support for setting QE SNUMs
arch/powerpc/platforms/83xx/Kconfig | 8
arch/powerpc/platforms/83xx/mpc83xx.h | 4
arch/powerpc/platforms/83xx/usb.c | 14 +++---
arch/powerpc/sysdev/qe_lib/qe.c | 10 --
drivers/gpio/Kconfig | 6 +++---
5 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/Kconfig
b/arch/powerpc/platforms/83xx/Kconfig
index 2bdc8c8..904991d 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -113,6 +113,14 @@ config KMETER1
endif
+# used for gpio
+config PPC_MPC830x
+ bool
+ select ARCH_WANT_OPTIONAL_GPIOLIB
+
+config PPC_MPC8306
+ bool
+
# used for usb gpio
config PPC_MPC831x
bool
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index 0cf74d7..4f21fb9 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -21,6 +21,8 @@
/* system i/o configuration register low */
#define MPC83XX_SICRL_OFFS 0x114
+#define MPC8306_SICRL_USB_MASK 0x003C
+#define MPC8306_SICRL_USB_ULPI 0x
#define MPC834X_SICRL_USB_MASK 0x6000
#define MPC834X_SICRL_USB0 0x2000
#define MPC834X_SICRL_USB1 0x4000
@@ -35,6 +37,8 @@
/* system i/o configuration register high */
#define MPC83XX_SICRH_OFFS 0x118
+#define MPC8306_SICRH_USB_MASK 0x0F00F300
+#define MPC8306_SICRH_USB_ULPI 0x
#define MPC8308_SICRH_USB_MASK 0x000c
#define MPC8308_SICRH_USB_ULPI 0x0004
#define MPC834X_SICRH_USB_UTMI 0x0002
diff --git a/arch/powerpc/platforms/83xx/usb.c
b/arch/powerpc/platforms/83xx/usb.c
index 5c31d82..a9db44b 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
}
#endif /* CONFIG_PPC_MPC834x */
-#ifdef CONFIG_PPC_MPC831x
+#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
int mpc831x_usb_cfg(void)
{
u32 temp;
@@ -128,6 +128,7 @@ int mpc831x_usb_cfg(void)
/* Configure clock */
immr_node = of_get_parent(np);
if (immr_node (of_device_is_compatible(immr_node,
fsl,mpc8315-immr) ||
+ of_device_is_compatible(immr_node, fsl,mpc8306-immr)
||
of_device_is_compatible(immr_node, fsl,mpc8308-immr)))
clrsetbits_be32(immap + MPC83XX_SCCR_OFFS,
MPC8315_SCCR_USB_MASK,
@@ -139,7 +140,14 @@ int mpc831x_usb_cfg(void)
/* Configure pin mux for ULPI. There is no pin mux for UTMI */
if (prop !strcmp(prop, ulpi)) {
- if (of_device_is_compatible(immr_node, fsl,mpc8308-immr)) {
+ if (of_device_is_compatible(immr_node, fsl,mpc8306-immr)) {
+ clrsetbits_be32(immap + MPC83XX_SICRL_OFFS,
+ MPC8306_SICRL_USB_MASK,
+ MPC8306_SICRL_USB_ULPI);
+ clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
+ MPC8306_SICRH_USB_MASK,
+ MPC8306_SICRH_USB_ULPI);
+ } else if (of_device_is_compatible(immr_node,
fsl,mpc8308-immr)) {
clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
MPC8308_SICRH_USB_MASK,
MPC8308_SICRH_USB_ULPI);
@@ -210,7 +218,7 @@ out:
of_node_put(np);
return ret;
}
-#endif /* CONFIG_PPC_MPC831x */
+#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */
#ifdef CONFIG_PPC_MPC837x
int mpc837x_usb_cfg(void)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index c2518cd..f0c45f0 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -285,12 +285,18 @@ static void qe_snums_init(void)
0x28, 0x29, 0x38, 0x39, 0x48, 0x49, 0x58, 0x59,
0x68, 0x69, 0x78, 0x79, 0x80, 0x81,
};
+ static const u8 snum_init_14[] = {
+ 0x88, 0x89, 0x98, 0x99, 0xA8, 0xA9, 0xB8, 0xB9,
+ 0xC8, 0xC9, 0xD8, 0xD9, 0xE8, 0xE9,
+ };
static const u8 *snum_init;
qe_num_of_snum = qe_get_num_of_snums();
if (qe_num_of_snum == 76)
snum_init = snum_init_76;
+ else if (qe_num_of_snum == 14)
+ snum_init = snum_init_14;
else
snum_init = snum_init_46;
@@ -657,8 +663,8 @@ unsigned int qe_get_num_of_snums(void)
prop = of_get_property(qe, fsl,qe-num-snums, size);
if (prop size == sizeof(*prop)) {
num_of_snums = *prop;
- if ((num_of_snums 28
Re: [PATCH v2] powerpc/83xx: add support for mpc8306
On 4/3/2015 2:01 PM, Paul Bolle wrote: On Fri, 2015-04-03 at 12:44 +0200, Filip Brozovic wrote: --- a/arch/powerpc/platforms/83xx/Kconfig +++ b/arch/powerpc/platforms/83xx/Kconfig +# used for gpio +config PPC_MPC830x + bool + select ARCH_WANT_OPTIONAL_GPIOLIB + +config PPC_MPC8306 + bool To me these two new Kconfig symbols look pointless: - they have no prompt, so one cannot set them manually; - no other Kconfig symbol selects them; - they do not default to 'y'. I'm not aware of a way to set these symbols to 'y' outside of those three. Is there perhaps a way for kconfig to set these symbols to 'y' that I have missed? Or do you expect to do one of these three things in a separate patch? The idea was that boards in the Kconfig file would select these symbols in order to enable support for the 8306. I mainly wanted to get this patch into mainline in order to make kernel maintenance for a couple of custom in-house developed boards easier. Since these boards are not widely available and our customers are unlikely to want to change and recompile the kernel, I have so far leaned towards not including support for them in mainline. As far as I can see, boards which are included in mainline right now are mostly evaluation boards which are easily available at most electronics distributors. That being said, I don't know what the official stance on this is; is adding custom boards encouraged regardless of their availability (e.g. if I develop a custom board with the intention of only ever actually making a single prototype for personal use, should I go and submit patches so that support makes it into the mainline kernel?), or should there be a minimum level of public interest before incorporating custom boards into mainline? If it's the latter, I suppose a solution would be to include support for the Freescale MPC8306SOM in mainline. Of course, this has its own problems, since someone would have to write and maintain it (and I don't have an MPC8306SOM nor the time needed to do maintenance). - Filip -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc/83xx: add support for mpc8306
On 3/31/2015 7:54 PM, Scott Wood wrote: This breaks multiplatform support. You need to determine this at runtime. Understood, but I'm unsure of how to do this exactly. Would it be appropriate to define another array, snum_init_14, with the SNUM values for the MPC8306 QE, change the minimum number of SNUMs in qe_get_num_of_snums() to 14 and set the correct values this way? In that case, compatibility with other platforms would be kept intact, and the QE device tree node for MPC8306 (and MPC8309) boards would just have to have the fsl,qe-num-snums property set to 14 in order for the driver to set the correct values. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] powerpc/83xx: add support for mpc8306
On 3/31/2015 7:54 PM, Scott Wood wrote: This breaks multiplatform support. You need to determine this at runtime. Understood, but I'm unsure of how to do this exactly. Would it be appropriate to define another array, snum_init_14, with the SNUM values for the MPC8306 QE, change the minimum number of SNUMs in qe_get_num_of_snums() to 14 and set the correct values this way? In that case, compatibility with other platforms would be kept intact, and the QE device tree node for MPC8306 (and MPC8309) boards would just have to have the fsl,qe-num-snums property set to 14 in order for the driver to set the correct values. -- 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] powerpc/83xx: add support for mpc8306
Add chip specific initialization for the MPC8306.
Signed-off-by: Filip Brozovic
---
arch/powerpc/platforms/83xx/Kconfig | 8
arch/powerpc/platforms/83xx/mpc83xx.h | 4
arch/powerpc/platforms/83xx/usb.c | 14 +++---
arch/powerpc/platforms/Kconfig| 10 ++
arch/powerpc/sysdev/qe_lib/qe.c | 15 ++-
drivers/gpio/Kconfig | 6 +++---
6 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/Kconfig
b/arch/powerpc/platforms/83xx/Kconfig
index 2bdc8c8..904991d 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -113,6 +113,14 @@ config KMETER1
endif
+# used for gpio
+config PPC_MPC830x
+ bool
+ select ARCH_WANT_OPTIONAL_GPIOLIB
+
+config PPC_MPC8306
+ bool
+
# used for usb & gpio
config PPC_MPC831x
bool
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index 0cf74d7..4f21fb9 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -21,6 +21,8 @@
/* system i/o configuration register low */
#define MPC83XX_SICRL_OFFS 0x114
+#define MPC8306_SICRL_USB_MASK 0x003C
+#define MPC8306_SICRL_USB_ULPI 0x
#define MPC834X_SICRL_USB_MASK 0x6000
#define MPC834X_SICRL_USB0 0x2000
#define MPC834X_SICRL_USB1 0x4000
@@ -35,6 +37,8 @@
/* system i/o configuration register high */
#define MPC83XX_SICRH_OFFS 0x118
+#define MPC8306_SICRH_USB_MASK 0x0F00F300
+#define MPC8306_SICRH_USB_ULPI 0x
#define MPC8308_SICRH_USB_MASK 0x000c
#define MPC8308_SICRH_USB_ULPI 0x0004
#define MPC834X_SICRH_USB_UTMI 0x0002
diff --git a/arch/powerpc/platforms/83xx/usb.c
b/arch/powerpc/platforms/83xx/usb.c
index 5c31d82..a9db44b 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
}
#endif /* CONFIG_PPC_MPC834x */
-#ifdef CONFIG_PPC_MPC831x
+#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
int mpc831x_usb_cfg(void)
{
u32 temp;
@@ -128,6 +128,7 @@ int mpc831x_usb_cfg(void)
/* Configure clock */
immr_node = of_get_parent(np);
if (immr_node && (of_device_is_compatible(immr_node,
"fsl,mpc8315-immr") ||
+ of_device_is_compatible(immr_node, "fsl,mpc8306-immr")
||
of_device_is_compatible(immr_node, "fsl,mpc8308-immr")))
clrsetbits_be32(immap + MPC83XX_SCCR_OFFS,
MPC8315_SCCR_USB_MASK,
@@ -139,7 +140,14 @@ int mpc831x_usb_cfg(void)
/* Configure pin mux for ULPI. There is no pin mux for UTMI */
if (prop && !strcmp(prop, "ulpi")) {
- if (of_device_is_compatible(immr_node, "fsl,mpc8308-immr")) {
+ if (of_device_is_compatible(immr_node, "fsl,mpc8306-immr")) {
+ clrsetbits_be32(immap + MPC83XX_SICRL_OFFS,
+ MPC8306_SICRL_USB_MASK,
+ MPC8306_SICRL_USB_ULPI);
+ clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
+ MPC8306_SICRH_USB_MASK,
+ MPC8306_SICRH_USB_ULPI);
+ } else if (of_device_is_compatible(immr_node,
"fsl,mpc8308-immr")) {
clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
MPC8308_SICRH_USB_MASK,
MPC8308_SICRH_USB_ULPI);
@@ -210,7 +218,7 @@ out:
of_node_put(np);
return ret;
}
-#endif /* CONFIG_PPC_MPC831x */
+#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */
#ifdef CONFIG_PPC_MPC837x
int mpc837x_usb_cfg(void)
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 391b3f6..ecb7cad 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -296,6 +296,16 @@ config QE_GPIO
Say Y here if you're going to use hardware that connects to the
QE GPIOs.
+config QE_8306
+ bool "MPC8306 QE support"
+ depends on QUICC_ENGINE
+ default y if PPC_MPC8306
+ help
+ The QUICC Engine in the MPC8306 does not support all SNUM threads.
+
+ Say Y here only if you wish to build a kernel for a machine with an
+ MPC8306.
+
config CPM2
bool "Enable support for the CPM2 (Communications Processor Module)"
depends on (FSL_SOC_BOOKE && PPC32) || 8260
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index c2518cd..f967ff6 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -278,12 +278,17 @@ s
[PATCH] powerpc/83xx: add support for mpc8306
Add chip specific initialization for the MPC8306.
Signed-off-by: Filip Brozovic fbrozo...@gmail.com
---
arch/powerpc/platforms/83xx/Kconfig | 8
arch/powerpc/platforms/83xx/mpc83xx.h | 4
arch/powerpc/platforms/83xx/usb.c | 14 +++---
arch/powerpc/platforms/Kconfig| 10 ++
arch/powerpc/sysdev/qe_lib/qe.c | 15 ++-
drivers/gpio/Kconfig | 6 +++---
6 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/Kconfig
b/arch/powerpc/platforms/83xx/Kconfig
index 2bdc8c8..904991d 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -113,6 +113,14 @@ config KMETER1
endif
+# used for gpio
+config PPC_MPC830x
+ bool
+ select ARCH_WANT_OPTIONAL_GPIOLIB
+
+config PPC_MPC8306
+ bool
+
# used for usb gpio
config PPC_MPC831x
bool
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index 0cf74d7..4f21fb9 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -21,6 +21,8 @@
/* system i/o configuration register low */
#define MPC83XX_SICRL_OFFS 0x114
+#define MPC8306_SICRL_USB_MASK 0x003C
+#define MPC8306_SICRL_USB_ULPI 0x
#define MPC834X_SICRL_USB_MASK 0x6000
#define MPC834X_SICRL_USB0 0x2000
#define MPC834X_SICRL_USB1 0x4000
@@ -35,6 +37,8 @@
/* system i/o configuration register high */
#define MPC83XX_SICRH_OFFS 0x118
+#define MPC8306_SICRH_USB_MASK 0x0F00F300
+#define MPC8306_SICRH_USB_ULPI 0x
#define MPC8308_SICRH_USB_MASK 0x000c
#define MPC8308_SICRH_USB_ULPI 0x0004
#define MPC834X_SICRH_USB_UTMI 0x0002
diff --git a/arch/powerpc/platforms/83xx/usb.c
b/arch/powerpc/platforms/83xx/usb.c
index 5c31d82..a9db44b 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
}
#endif /* CONFIG_PPC_MPC834x */
-#ifdef CONFIG_PPC_MPC831x
+#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
int mpc831x_usb_cfg(void)
{
u32 temp;
@@ -128,6 +128,7 @@ int mpc831x_usb_cfg(void)
/* Configure clock */
immr_node = of_get_parent(np);
if (immr_node (of_device_is_compatible(immr_node,
fsl,mpc8315-immr) ||
+ of_device_is_compatible(immr_node, fsl,mpc8306-immr)
||
of_device_is_compatible(immr_node, fsl,mpc8308-immr)))
clrsetbits_be32(immap + MPC83XX_SCCR_OFFS,
MPC8315_SCCR_USB_MASK,
@@ -139,7 +140,14 @@ int mpc831x_usb_cfg(void)
/* Configure pin mux for ULPI. There is no pin mux for UTMI */
if (prop !strcmp(prop, ulpi)) {
- if (of_device_is_compatible(immr_node, fsl,mpc8308-immr)) {
+ if (of_device_is_compatible(immr_node, fsl,mpc8306-immr)) {
+ clrsetbits_be32(immap + MPC83XX_SICRL_OFFS,
+ MPC8306_SICRL_USB_MASK,
+ MPC8306_SICRL_USB_ULPI);
+ clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
+ MPC8306_SICRH_USB_MASK,
+ MPC8306_SICRH_USB_ULPI);
+ } else if (of_device_is_compatible(immr_node,
fsl,mpc8308-immr)) {
clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
MPC8308_SICRH_USB_MASK,
MPC8308_SICRH_USB_ULPI);
@@ -210,7 +218,7 @@ out:
of_node_put(np);
return ret;
}
-#endif /* CONFIG_PPC_MPC831x */
+#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */
#ifdef CONFIG_PPC_MPC837x
int mpc837x_usb_cfg(void)
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 391b3f6..ecb7cad 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -296,6 +296,16 @@ config QE_GPIO
Say Y here if you're going to use hardware that connects to the
QE GPIOs.
+config QE_8306
+ bool MPC8306 QE support
+ depends on QUICC_ENGINE
+ default y if PPC_MPC8306
+ help
+ The QUICC Engine in the MPC8306 does not support all SNUM threads.
+
+ Say Y here only if you wish to build a kernel for a machine with an
+ MPC8306.
+
config CPM2
bool Enable support for the CPM2 (Communications Processor Module)
depends on (FSL_SOC_BOOKE PPC32) || 8260
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index c2518cd..f967ff6 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -278,12 +278,17 @@ static void qe_snums_init(void)
0xF4, 0xF5, 0xFC, 0xFD,
};
static const u8 snum_init_46
[PATCH] input: synaptics: fix min-max quirk value for E440
Commit 98dc070373 ("Input: synaptics - add quirk for Thinkpad E440") had
a typo in ymax, this changes the value to the one reported by
touchpad-edge-detector and mentioned in the commit.
Signed-off-by: Filip Ayazi
---
drivers/input/mouse/synaptics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index f6a3a7b..3b06c8a 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -155,7 +155,7 @@ static const struct min_max_quirk min_max_pnpid_table[] = {
{
(const char * const []){"LEN2006", NULL},
{2691, 2691},
- 1024, 5045, 2457, 4632
+ 1024, 5045, 2457, 4832
},
{
(const char * const []){"LEN2006", NULL},
--
1.9.1
--
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] input: synaptics: min_max quirk for Thinkpad E440
On 03/25/2015 06:22 PM, Dmitry Torokhov wrote: On Wed, Mar 25, 2015 at 09:57:51AM -0300, Ramiro Morales wrote: On Wed, Mar 25, 2015 at 6:04 AM, Daniel Martin wrote: On 25 March 2015 at 02:20, Filip Ayazi wrote: Sets min_max_quirk values for LEN2006 touchpad found in Lenovo Thinkpad E440 (board_id=2691). Dmitry already applied a patch for the E440: http://www.spinics.net/lists/linux-input/msg37497.html There are two great things about this second submission from Filip. First, it confirms the values sent by the hardware in a second laptop as reported by touchpad-edge-detector. Second, it reveals I made a stupid mistake transcribing the y_max value to the quirks able. The correct value is 4832 and my patch wrongly contains 4632. I'm very sorry for this. I see my patch has already been pushed to the 'for-linus' branch and I don't know which process should we follow to fix the issue. My 'for-linus' branch is stable so we need a fixup on top on the incorrect patch, the same way as if it was already in mainline. I too have noticed the typo in the commit earlier today and have a patch to apply on top to fix it, which I will sent in another email. Sincerely, Filip Ayazi -- 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] input: synaptics: min_max quirk for Thinkpad E440
On 03/25/2015 06:22 PM, Dmitry Torokhov wrote: On Wed, Mar 25, 2015 at 09:57:51AM -0300, Ramiro Morales wrote: On Wed, Mar 25, 2015 at 6:04 AM, Daniel Martin consume.no...@gmail.com wrote: On 25 March 2015 at 02:20, Filip Ayazi filipay...@gmail.com wrote: Sets min_max_quirk values for LEN2006 touchpad found in Lenovo Thinkpad E440 (board_id=2691). Dmitry already applied a patch for the E440: http://www.spinics.net/lists/linux-input/msg37497.html There are two great things about this second submission from Filip. First, it confirms the values sent by the hardware in a second laptop as reported by touchpad-edge-detector. Second, it reveals I made a stupid mistake transcribing the y_max value to the quirks able. The correct value is 4832 and my patch wrongly contains 4632. I'm very sorry for this. I see my patch has already been pushed to the 'for-linus' branch and I don't know which process should we follow to fix the issue. My 'for-linus' branch is stable so we need a fixup on top on the incorrect patch, the same way as if it was already in mainline. I too have noticed the typo in the commit earlier today and have a patch to apply on top to fix it, which I will sent in another email. Sincerely, Filip Ayazi -- 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] input: synaptics: fix min-max quirk value for E440
Commit 98dc070373 (Input: synaptics - add quirk for Thinkpad E440) had
a typo in ymax, this changes the value to the one reported by
touchpad-edge-detector and mentioned in the commit.
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
drivers/input/mouse/synaptics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index f6a3a7b..3b06c8a 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -155,7 +155,7 @@ static const struct min_max_quirk min_max_pnpid_table[] = {
{
(const char * const []){LEN2006, NULL},
{2691, 2691},
- 1024, 5045, 2457, 4632
+ 1024, 5045, 2457, 4832
},
{
(const char * const []){LEN2006, NULL},
--
1.9.1
--
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] input: synaptics: min_max quirk for Thinkpad E440
Sets min_max_quirk values for LEN2006 touchpad found in Lenovo Thinkpad E440
(board_id=2691).
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=89641
Signed-off-by: Filip Ayazi
---
drivers/input/mouse/synaptics.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index dda6058..63942d4 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -154,6 +154,11 @@ static const struct min_max_quirk min_max_pnpid_table[] = {
},
{
(const char * const []){"LEN2006", NULL},
+ {2691, 2691},
+ 1024, 5045, 2457, 4832
+ },
+ {
+ (const char * const []){"LEN2006", NULL},
{ANY_BOARD_ID, ANY_BOARD_ID},
1264, 5675, 1171, 4688
},
--
1.9.1
--
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] input: synaptics: min_max quirk for Thinkpad E440
Sets min_max_quirk values for LEN2006 touchpad found in Lenovo Thinkpad E440
(board_id=2691).
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=89641
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
drivers/input/mouse/synaptics.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index dda6058..63942d4 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -154,6 +154,11 @@ static const struct min_max_quirk min_max_pnpid_table[] = {
},
{
(const char * const []){LEN2006, NULL},
+ {2691, 2691},
+ 1024, 5045, 2457, 4832
+ },
+ {
+ (const char * const []){LEN2006, NULL},
{ANY_BOARD_ID, ANY_BOARD_ID},
1264, 5675, 1171, 4688
},
--
1.9.1
--
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] iwlwifi: mvm: check time event is over before disconnecting
On 02/28/2015 08:55 PM, Emmanuel Grumbach wrote:
On Sat, Feb 28, 2015 at 1:10 AM, Filip Ayazi wrote:
On the 7260 time event was often ended before end_time and connections failed
with "No association and the time event is over already...".
This checks that the time event is actually over before disconnecting.
Signed-off-by: Filip Ayazi
While this patch is wrong I'd like to know if it helps.
It helped here (7260 on Lenovo E440).
I get a TE with unknown action message and successful connection now,
used to be TE ended with current time < end time and a disconnect,
even on networks with excellent signal strength. It used to happen
about 30% of the time with no apparent pattern, sometimes restarting
the interface fixed the issue.
The patch is wrong because even if the driver thinks the time event
should still be running, if the firmware indicates it has already
ended, we should still disconnect. The reason for the disconnection is
that we can't be sure that the firmware will have the proper timing
for the beacon and hence for the DTIM. Both are critical to get a
decent behavior while saving power.
You are right, few hours ago, after waking from suspend, I could not switch
the interface on (ifconfig said SIOCSIFFLAGS: Input/output error), reloading
iwlmvm module helped, so this was likely the cause.
In any case, I doubt this patch does actually something because the
firmware and the driver should really be close in their timings. If
they aren't, it is fundamental bug in the firmware. While the firmware
does have bugs just like any piece of software, I doubt it has such
big ones :)
I do not see any other explanation apart from faulty hardware,
which doesn't seem likely :)
---
drivers/net/wireless/iwlwifi/mvm/time-event.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/mvm/time-event.c
b/drivers/net/wireless/iwlwifi/mvm/time-event.c
index 54fafbf..b0aa892 100644
--- a/drivers/net/wireless/iwlwifi/mvm/time-event.c
+++ b/drivers/net/wireless/iwlwifi/mvm/time-event.c
@@ -256,7 +256,8 @@ static void iwl_mvm_te_handle_notif(struct iwl_mvm *mvm,
}
}
- if (le32_to_cpu(notif->action) & TE_V2_NOTIF_HOST_EVENT_END) {
+ if (le32_to_cpu(notif->action) & TE_V2_NOTIF_HOST_EVENT_END &&
+ time_after(jiffies, te_data->end_jiffies)) {
IWL_DEBUG_TE(mvm,
"TE ended - current time %lu, estimated end
%lu\n",
jiffies, te_data->end_jiffies);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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] iwlwifi: mvm: check time event is over before disconnecting
On 02/28/2015 08:55 PM, Emmanuel Grumbach wrote:
On Sat, Feb 28, 2015 at 1:10 AM, Filip Ayazi filipay...@gmail.com wrote:
On the 7260 time event was often ended before end_time and connections failed
with No association and the time event is over already
This checks that the time event is actually over before disconnecting.
Signed-off-by: Filip Ayazi filipay...@gmail.com
While this patch is wrong I'd like to know if it helps.
It helped here (7260 on Lenovo E440).
I get a TE with unknown action message and successful connection now,
used to be TE ended with current time end time and a disconnect,
even on networks with excellent signal strength. It used to happen
about 30% of the time with no apparent pattern, sometimes restarting
the interface fixed the issue.
The patch is wrong because even if the driver thinks the time event
should still be running, if the firmware indicates it has already
ended, we should still disconnect. The reason for the disconnection is
that we can't be sure that the firmware will have the proper timing
for the beacon and hence for the DTIM. Both are critical to get a
decent behavior while saving power.
You are right, few hours ago, after waking from suspend, I could not switch
the interface on (ifconfig said SIOCSIFFLAGS: Input/output error), reloading
iwlmvm module helped, so this was likely the cause.
In any case, I doubt this patch does actually something because the
firmware and the driver should really be close in their timings. If
they aren't, it is fundamental bug in the firmware. While the firmware
does have bugs just like any piece of software, I doubt it has such
big ones :)
I do not see any other explanation apart from faulty hardware,
which doesn't seem likely :)
---
drivers/net/wireless/iwlwifi/mvm/time-event.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/mvm/time-event.c
b/drivers/net/wireless/iwlwifi/mvm/time-event.c
index 54fafbf..b0aa892 100644
--- a/drivers/net/wireless/iwlwifi/mvm/time-event.c
+++ b/drivers/net/wireless/iwlwifi/mvm/time-event.c
@@ -256,7 +256,8 @@ static void iwl_mvm_te_handle_notif(struct iwl_mvm *mvm,
}
}
- if (le32_to_cpu(notif-action) TE_V2_NOTIF_HOST_EVENT_END) {
+ if (le32_to_cpu(notif-action) TE_V2_NOTIF_HOST_EVENT_END
+ time_after(jiffies, te_data-end_jiffies)) {
IWL_DEBUG_TE(mvm,
TE ended - current time %lu, estimated end
%lu\n,
jiffies, te_data-end_jiffies);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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] iwlwifi: mvm: check time event is over before disconnecting
On the 7260 time event was often ended before end_time and connections failed
with "No association and the time event is over already...".
This checks that the time event is actually over before disconnecting.
Signed-off-by: Filip Ayazi
---
drivers/net/wireless/iwlwifi/mvm/time-event.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/mvm/time-event.c
b/drivers/net/wireless/iwlwifi/mvm/time-event.c
index 54fafbf..b0aa892 100644
--- a/drivers/net/wireless/iwlwifi/mvm/time-event.c
+++ b/drivers/net/wireless/iwlwifi/mvm/time-event.c
@@ -256,7 +256,8 @@ static void iwl_mvm_te_handle_notif(struct iwl_mvm *mvm,
}
}
- if (le32_to_cpu(notif->action) & TE_V2_NOTIF_HOST_EVENT_END) {
+ if (le32_to_cpu(notif->action) & TE_V2_NOTIF_HOST_EVENT_END &&
+ time_after(jiffies, te_data->end_jiffies)) {
IWL_DEBUG_TE(mvm,
"TE ended - current time %lu, estimated end %lu\n",
jiffies, te_data->end_jiffies);
--
1.9.1
--
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] iwlwifi: mvm: check time event is over before disconnecting
On the 7260 time event was often ended before end_time and connections failed
with No association and the time event is over already
This checks that the time event is actually over before disconnecting.
Signed-off-by: Filip Ayazi filipay...@gmail.com
---
drivers/net/wireless/iwlwifi/mvm/time-event.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/mvm/time-event.c
b/drivers/net/wireless/iwlwifi/mvm/time-event.c
index 54fafbf..b0aa892 100644
--- a/drivers/net/wireless/iwlwifi/mvm/time-event.c
+++ b/drivers/net/wireless/iwlwifi/mvm/time-event.c
@@ -256,7 +256,8 @@ static void iwl_mvm_te_handle_notif(struct iwl_mvm *mvm,
}
}
- if (le32_to_cpu(notif-action) TE_V2_NOTIF_HOST_EVENT_END) {
+ if (le32_to_cpu(notif-action) TE_V2_NOTIF_HOST_EVENT_END
+ time_after(jiffies, te_data-end_jiffies)) {
IWL_DEBUG_TE(mvm,
TE ended - current time %lu, estimated end %lu\n,
jiffies, te_data-end_jiffies);
--
1.9.1
--
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] ASoC: sgtl5000: Use shift mask when setting codec mode
Shift the I2S mode value by the necessary amount before writing the
registers. This makes Right Justified and PCM mode work in addition to
the default Left Justified mode.
Signed-off-by: Filip Brozovic
---
sound/soc/codecs/sgtl5000.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 29cf7ce..7665016 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -483,21 +483,21 @@ static int sgtl5000_set_dai_fmt(struct snd_soc_dai
*codec_dai, unsigned int fmt)
/* setting i2s data format */
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_DSP_A:
- i2sctl |= SGTL5000_I2S_MODE_PCM;
+ i2sctl |= SGTL5000_I2S_MODE_PCM << SGTL5000_I2S_MODE_SHIFT;
break;
case SND_SOC_DAIFMT_DSP_B:
- i2sctl |= SGTL5000_I2S_MODE_PCM;
+ i2sctl |= SGTL5000_I2S_MODE_PCM << SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRALIGN;
break;
case SND_SOC_DAIFMT_I2S:
- i2sctl |= SGTL5000_I2S_MODE_I2S_LJ;
+ i2sctl |= SGTL5000_I2S_MODE_I2S_LJ << SGTL5000_I2S_MODE_SHIFT;
break;
case SND_SOC_DAIFMT_RIGHT_J:
- i2sctl |= SGTL5000_I2S_MODE_RJ;
+ i2sctl |= SGTL5000_I2S_MODE_RJ << SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRPOL;
break;
case SND_SOC_DAIFMT_LEFT_J:
- i2sctl |= SGTL5000_I2S_MODE_I2S_LJ;
+ i2sctl |= SGTL5000_I2S_MODE_I2S_LJ << SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRALIGN;
break;
default:
--
2.1.4
--
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] ASoC: sgtl5000: Use shift mask when setting codec mode
Shift the I2S mode value by the necessary amount before writing the
registers. This makes Right Justified and PCM mode work in addition to
the default Left Justified mode.
Signed-off-by: Filip Brozovic fbrozo...@gmail.com
---
sound/soc/codecs/sgtl5000.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 29cf7ce..7665016 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -483,21 +483,21 @@ static int sgtl5000_set_dai_fmt(struct snd_soc_dai
*codec_dai, unsigned int fmt)
/* setting i2s data format */
switch (fmt SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_DSP_A:
- i2sctl |= SGTL5000_I2S_MODE_PCM;
+ i2sctl |= SGTL5000_I2S_MODE_PCM SGTL5000_I2S_MODE_SHIFT;
break;
case SND_SOC_DAIFMT_DSP_B:
- i2sctl |= SGTL5000_I2S_MODE_PCM;
+ i2sctl |= SGTL5000_I2S_MODE_PCM SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRALIGN;
break;
case SND_SOC_DAIFMT_I2S:
- i2sctl |= SGTL5000_I2S_MODE_I2S_LJ;
+ i2sctl |= SGTL5000_I2S_MODE_I2S_LJ SGTL5000_I2S_MODE_SHIFT;
break;
case SND_SOC_DAIFMT_RIGHT_J:
- i2sctl |= SGTL5000_I2S_MODE_RJ;
+ i2sctl |= SGTL5000_I2S_MODE_RJ SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRPOL;
break;
case SND_SOC_DAIFMT_LEFT_J:
- i2sctl |= SGTL5000_I2S_MODE_I2S_LJ;
+ i2sctl |= SGTL5000_I2S_MODE_I2S_LJ SGTL5000_I2S_MODE_SHIFT;
i2sctl |= SGTL5000_I2S_LRALIGN;
break;
default:
--
2.1.4
--
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/
Areca driver 2.6.19 on x86_64
We have a problem with the new areca driver included in kernel tree 2.6.19. During the boot sequence we get this output: Loading arcmsr.ko module ACPI: PCI Interrupt :05:0e.0[A] -> Link [LINKC] -> GSI 5 (level, low) -> IRQ 5 ARECA RAID ADAPTER0: FIRMWARE VERSION V1.41 2006-5-24 scsi0: Areca SATA Host Adapter RAID Controller Driver Version 1.20.00.13 arcmsr0: abort device command of scsi id=0 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully arcmsr0: abort device command of scsi id=1 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully arcmsr0: abort device command of scsi id=2 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully scsi 0:0:0:0: scsi: Device offlined - not ready after error recovery and keeps looping through all luns with different values of ccd at each loop. Till kernel panics because of the lack of root device. Here is our specs: HW used: ARC1220 ARC1110 on Supermicro X6DH8-XG2, Dual Xeon 3.0GHz OS distro used: CentOS 4.4 x86_64 Kernel 2.6.19 with hand-crafted config, that we are able to use successfully with kernel 2.6.16.20. Have you any suggestions to resolve this issue ? Thanks in advance, Filip Majewski - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Areca driver 2.6.19 on x86_64
We have a problem with the new areca driver included in kernel tree 2.6.19. During the boot sequence we get this output: Loading arcmsr.ko module ACPI: PCI Interrupt :05:0e.0[A] - Link [LINKC] - GSI 5 (level, low) - IRQ 5 ARECA RAID ADAPTER0: FIRMWARE VERSION V1.41 2006-5-24 scsi0: Areca SATA Host Adapter RAID Controller Driver Version 1.20.00.13 arcmsr0: abort device command of scsi id=0 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully arcmsr0: abort device command of scsi id=1 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully arcmsr0: abort device command of scsi id=2 lun=0 arcmsr0: scsi id=0 lun=0 ccb='0x810037f0' poll command abort successfully scsi 0:0:0:0: scsi: Device offlined - not ready after error recovery and keeps looping through all luns with different values of ccd at each loop. Till kernel panics because of the lack of root device. Here is our specs: HW used: ARC1220 ARC1110 on Supermicro X6DH8-XG2, Dual Xeon 3.0GHz OS distro used: CentOS 4.4 x86_64 Kernel 2.6.19 with hand-crafted config, that we are able to use successfully with kernel 2.6.16.20. Have you any suggestions to resolve this issue ? Thanks in advance, Filip Majewski - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
2.4.4 + gcc 3 compile error
Hi, I tried compiling the 2.4.4 release with a gcc 3 snapshot and it failed with following error: gcc -D__KERNEL__ -I/usr/src/linux/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-strict-aliasing -pipe -mpreferred-stack-boundary=2 -march=i686-DEXPORT_SYMTAB -c sys.c sys.c: In function `sys_gethostname': /usr/src/linux/include/asm/rwsem.h:152: inconsistent operand constraints in an `asm' make[3]: *** [sys.o] Error 1 make[3]: Leaving directory `/usr/src/linux/kernel' make[2]: *** [first_rule] Error 2 make[2]: Leaving directory `/usr/src/linux/kernel' make[1]: *** [_dir_kernel] Error 2 make[1]: Leaving directory `/usr/src/linux' make: *** [stamp-build] Error 2 lucretia:/usr/src/linux$ Regards, Filip -- My opinions may have changed, but not the fact that I am right -- Daniel Vogel PGP signature
2.4.4 + gcc 3 compile error
Hi, I tried compiling the 2.4.4 release with a gcc 3 snapshot and it failed with following error: gcc -D__KERNEL__ -I/usr/src/linux/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-strict-aliasing -pipe -mpreferred-stack-boundary=2 -march=i686-DEXPORT_SYMTAB -c sys.c sys.c: In function `sys_gethostname': /usr/src/linux/include/asm/rwsem.h:152: inconsistent operand constraints in an `asm' make[3]: *** [sys.o] Error 1 make[3]: Leaving directory `/usr/src/linux/kernel' make[2]: *** [first_rule] Error 2 make[2]: Leaving directory `/usr/src/linux/kernel' make[1]: *** [_dir_kernel] Error 2 make[1]: Leaving directory `/usr/src/linux' make: *** [stamp-build] Error 2 lucretia:/usr/src/linux$ Regards, Filip -- My opinions may have changed, but not the fact that I am right -- Daniel Vogel PGP signature
vfat read problem with 2.4.x
Hi, I encountered a problem with 2.4 kernels today. Decompressing a 60+ Mb file with bzip2, residing on a vfat partition, gave errors reporting that the archive was corrupt. When I rebooted into windows and ran scandisk it couldn't find any problem with the partition. That made me suspicious... I rebooted into an leftover 2.2.18, ran "bunzip -t" on the file, and all was fine. Rebooted back into 2.4.3, bunzip gave errors. Tried 2.4.2 (had an image of that left around as well), errors too. I copied the file (in 2.2.18) to my ext2 partition, tested it with bzip2 in the 2.4 kernels and all was fine - so it is most likely a vfat related problem. Also, the file was written to the vfat partition in 2.4.2, so writing may not even be affected by this possible bug. Regards, Filip -- Sometimes it pays to stay in bed on Monday, rather than spending the rest of the week debugging Monday's code. -- Dan Salomon - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
vfat read problem with 2.4.x
Hi, I encountered a problem with 2.4 kernels today. Decompressing a 60+ Mb file with bzip2, residing on a vfat partition, gave errors reporting that the archive was corrupt. When I rebooted into windows and ran scandisk it couldn't find any problem with the partition. That made me suspicious... I rebooted into an leftover 2.2.18, ran "bunzip -t" on the file, and all was fine. Rebooted back into 2.4.3, bunzip gave errors. Tried 2.4.2 (had an image of that left around as well), errors too. I copied the file (in 2.2.18) to my ext2 partition, tested it with bzip2 in the 2.4 kernels and all was fine - so it is most likely a vfat related problem. Also, the file was written to the vfat partition in 2.4.2, so writing may not even be affected by this possible bug. Regards, Filip -- Sometimes it pays to stay in bed on Monday, rather than spending the rest of the week debugging Monday's code. -- Dan Salomon - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
