All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 5/5] sandbox: led: use new function to configure default state
From: Simon Glass @ 2018-07-23 23:47 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <1532338891-6848-6-git-send-email-patrick.delaunay@st.com>

Hi Patrick,

On 23 July 2018 at 03:41, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> Initialize the led with the default state defined in device tree
> in board_init and solve issue with test for led default state.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> Led default-state is correctly handle in Sandbox, tested with:
>   ./u-boot -d ./arch/sandbox/dts/test.dtb
>   => led list
>   sandbox:red     <inactive>
>   sandbox:green   <inactive>
>   sandbox:default_on on
>   sandbox:default_off off
>
> This patch solve "make tests" issue introduced by
> http://patchwork.ozlabs.org/patch/943651/
>
> Changes in v2:
>   - add sandbox impact and test update
>
>  board/sandbox/sandbox.c | 9 +++++++++
>  common/board_r.c        | 3 ++-
>  test/dm/led.c           | 3 +++
>  3 files changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Please see below.

>
> diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
> index 195f620..66b5f24 100644
> --- a/board/sandbox/sandbox.c
> +++ b/board/sandbox/sandbox.c
> @@ -6,6 +6,7 @@
>  #include <common.h>
>  #include <cros_ec.h>
>  #include <dm.h>
> +#include <led.h>
>  #include <os.h>
>  #include <asm/test.h>
>  #include <asm/u-boot-sandbox.h>
> @@ -47,6 +48,14 @@ int dram_init(void)
>         return 0;
>  }
>
> +int board_init(void)
> +{
> +#ifdef CONFIG_LED
> +       led_default_state();

if (IS_ENABLED(CONFIG_LED))
   led_default_state();

> +#endif /* CONFIG_LED */

blank line here

> +       return 0;
> +}
> +
>  #ifdef CONFIG_BOARD_LATE_INIT
>  int board_late_init(void)
>  {
> diff --git a/common/board_r.c b/common/board_r.c
> index 64f2574..9402c0e 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -690,7 +690,8 @@ static init_fnc_t init_sequence_r[] = {
>  #ifdef CONFIG_DM
>         initr_dm,
>  #endif
> -#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV)
> +#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
> +       defined(CONFIG_SANDBOX)
>         board_init,     /* Setup chipselects */
>  #endif
>         /*
> diff --git a/test/dm/led.c b/test/dm/led.c
> index 0071f21..00de7b3 100644
> --- a/test/dm/led.c
> +++ b/test/dm/led.c
> @@ -32,6 +32,9 @@ static int dm_test_led_default_state(struct unit_test_state *uts)
>  {
>         struct udevice *dev;
>
> +       /* configure the default state (auto-probe) */
> +       led_default_state();
> +
>         /* Check that we handle the default-state property correctly. */
>         ut_assertok(led_get_by_label("sandbox:default_on", &dev));
>         ut_asserteq(LEDST_ON, led_get_state(dev));
> --
> 2.7.4
>

^ permalink raw reply

* [U-Boot] [PATCH v2 4/5] stm32mp1: use new function led default state
From: Simon Glass @ 2018-07-23 23:47 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <1532338891-6848-5-git-send-email-patrick.delaunay@st.com>

On 23 July 2018 at 03:41, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> Initialize the led with the default state defined in device tree.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2: None
>
>  board/st/stm32mp1/stm32mp1.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

But I wonder if you can use if (IS_ENABLED(CONFIG_LED)) instead?
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index cc39fa6..db8c805 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -22,5 +22,9 @@ int board_init(void)
>         /* address of boot parameters */
>         gd->bd->bi_boot_params = STM32_DDR_BASE + 0x100;
>
> +#ifdef CONFIG_LED
> +       led_default_state();
> +#endif /* CONFIG_LED */
> +
>         return 0;
>  }
> --
> 2.7.4
>

^ permalink raw reply

* [U-Boot] [PATCH v2 2/5] Revert "dm: led: auto probe() LEDs with "default-state""
From: Simon Glass @ 2018-07-23 23:47 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <1532338891-6848-3-git-send-email-patrick.delaunay@st.com>

Hi Patrick,

On 23 July 2018 at 03:41, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> This reverts commit bc882f5d5c7b4d6ed5e927bf838863af43c786e7.

A revert should have a motivation and a discussion of the purpose,
just like any other patch. Can you add it please?

>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2: None
>
>  drivers/led/led_gpio.c | 9 ---------
>  1 file changed, 9 deletions(-)

Regards,
Simon

^ permalink raw reply

* HI
From: Mrs Suzara Maling Wan @ 2018-07-23 22:42 UTC (permalink / raw)





--
I am Mrs Suzara  i have a pending project of fulfillment to put in your 
hand, i will need your support to make this ream come through, could 
you le me know your interest to enable me give you further information, 
and I hereby advice that you send the below mentioned information I 
decided to will/donate the sum of  $4.5 Million US to you for the good 
work of god, and also to help the motherless and less privilege and 
also forth assistance of the widows.

At the moment I cannot take an telephone calls right now due to the 
fact that my relatives (that have squandered the funds agave them for 
this purpose before) are around me and my health status also. I have 
adjusted my will and my lawyer is aware. I have willed those properties 
to you by quoting my personal file routing and account information. And 
I have also notified the bank that I am willing that properties to you 
for a good, effective and prudent\work.

 I know I don't know you but I have been directed to do this by god.ok 
Please contact this woman for more details you might not get me on line 
in time contact this email; mrs.suzaramalingwan1962@gmail.com

Your full name..........
Your private telephone number..........
Your passport or identity card........
Your country....................... ...
Your occupation..............
Thank you as i wait your reply.

Yoursfaithful friand,

Mrs Suzara Maling Wan

^ permalink raw reply

* [PATCH] tpm: add support for partial reads
From: Tadeusz Struk @ 2018-07-23 23:42 UTC (permalink / raw)
  To: linux-security-module
In-Reply-To: <20180723220822.GJ532@ziepe.ca>

On 07/23/2018 03:08 PM, Jason Gunthorpe wrote:
> On Mon, Jul 23, 2018 at 03:00:20PM -0700, Tadeusz Struk wrote:
>> On 07/23/2018 02:56 PM, Jason Gunthorpe wrote:
>>> The proposed patch doesn't clear the data_pending if the entire buffer
>>> is not consumed, so of course it is ABI breaking, that really isn't OK.
>> The data_pending will be cleared by the timeout handler if the user doesn't
>> read the response fully before the timeout expires. The is the same situation
>> if the user would not read the response at all.
> That causes write() to fail with EBUSY
> 
> NAK from me on breaking the ABI like this

What if we introduce this new behavior only for the non-blocking mode
as James suggested? Or do you have some other suggestions?

Thanks,
-- 
Tadeusz
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [Fuego] [PATCH] busybox: add support for command head/hexdump/hostname/id/ifconfig/install
From: Tim.Bird @ 2018-07-23 23:34 UTC (permalink / raw)
  To: wangmy, fuego
In-Reply-To: <1532324114-87301-2-git-send-email-wangmy@cn.fujitsu.com>

See comments inline below.

> -----Original Message-----
> From: Wang Mingyu
> 
> Signed-off-by: Wang Mingyu <wangmy@cn.fujitsu.com>
> ---
>  .../tests/Functional.busybox/tests/busybox_head.sh | 22
> ++++++++++++++++++++++
>  .../Functional.busybox/tests/busybox_hexdump.sh    | 19
> +++++++++++++++++++
>  .../Functional.busybox/tests/busybox_hostname.sh   | 13 +++++++++++++
>  .../tests/Functional.busybox/tests/busybox_id.sh   | 13 +++++++++++++
>  .../Functional.busybox/tests/busybox_ifconfig.sh   | 13 +++++++++++++
>  .../Functional.busybox/tests/busybox_install.sh    | 17 +++++++++++++++++
>  6 files changed, 97 insertions(+)
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_head.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_hexdump.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_hostname.sh
>  create mode 100644 engine/tests/Functional.busybox/tests/busybox_id.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_ifconfig.sh
>  create mode 100644
> engine/tests/Functional.busybox/tests/busybox_install.sh
> 
> diff --git a/engine/tests/Functional.busybox/tests/busybox_head.sh
> b/engine/tests/Functional.busybox/tests/busybox_head.sh
> new file mode 100644
> index 0000000..be0269e
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_head.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command head
> +#  1) Option -n
> +
> +test="head"
> +
> +mkdir test_dir
> +touch test_dir/test1
This is not needed (the 'touch')

> +echo "This is the 1st line.">test_dir/test1
> +echo "This is the 2nd line.">>test_dir/test1
> +echo "This is the 3rd line.">>test_dir/test1
> +busybox head -n 2 test_dir/test1 > log
Please use a space before the redirection operator.

> +
> +if [ "$(busybox head -n 1 log)" = "This is the 1st line." ] && [ "$(tail -n 1 log)"
> = "This is the 2nd line." ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm log
> +rm -rf test_dir
> diff --git a/engine/tests/Functional.busybox/tests/busybox_hexdump.sh
> b/engine/tests/Functional.busybox/tests/busybox_hexdump.sh
> new file mode 100644
> index 0000000..d598c72
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_hexdump.sh
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command hexdump
> +#  1) Option: -c
> +
> +test="hexdump"
> +
> +mkdir test_dir
> +touch test_dir/test1
> +echo "HELLO WORLD">test_dir/test1
> +busybox hexdump -c test_dir/test1 > log
> +if head -n 1 log | grep "0000000   H   E   L   L   O       W   O   R   L   D  \\\n" && [
> "$(tail -n 1 log)" = "000000c" ]
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm log
> +rm -rf test_dir
> diff --git a/engine/tests/Functional.busybox/tests/busybox_hostname.sh
> b/engine/tests/Functional.busybox/tests/busybox_hostname.sh
> new file mode 100644
> index 0000000..f854515
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_hostname.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command hostname
> +#  1) Option none
> +
> +test="hostname"
> +
> +if [ $(busybox hostname) = $(hostname) ]
On a busybox-based system, this just tests the program against itself.
Maybe compare with $(cat /etc/hostname)?

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_id.sh
> b/engine/tests/Functional.busybox/tests/busybox_id.sh
> new file mode 100644
> index 0000000..9f53fcc
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_id.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command id
> +#  1) Option none
> +
> +test="id"
> +
> +if [ "$(busybox id)" = "$(id)" ]
Same issue here.  If busybox is the main tool for a board, then
'id' will just be a symlink to busybox, and this test just verifies
that the same command produces the same output.

This one I'm not sure what to do with this one.  Maybe
see if there's a shell variable that has the UID?

Or, check that 'id' is not 'busybox id'?

Let me know what you think.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_ifconfig.sh
> b/engine/tests/Functional.busybox/tests/busybox_ifconfig.sh
> new file mode 100644
> index 0000000..31b07d2
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_ifconfig.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command ifconfig
> +#  1) Option none
> +
> +test="ifconfig"
> +
> +if busybox ifconfig
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> diff --git a/engine/tests/Functional.busybox/tests/busybox_install.sh
> b/engine/tests/Functional.busybox/tests/busybox_install.sh
> new file mode 100644
> index 0000000..a8cba56
> --- /dev/null
> +++ b/engine/tests/Functional.busybox/tests/busybox_install.sh
> @@ -0,0 +1,17 @@
> +#!/bin/sh
> +
> +#  The testscript checks the following options of the command install
> +#  1) Option none
> +
> +test="install"
> +
> +mkdir test_dir test_install_dir
> +echo "ls test_install_dir">test_dir/test1
I'm not sure what's going on here.
test1 should have the contents: 'ls test_install_dir'.
It's not clear to me that this is executable.

> +busybox install test_dir/test1 test_install_dir
> +if [ $(./test_install_dir/test1) = test1 ]
Are you trying to execute the script?

I'm confused.

> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +rm -rf test_dir test_install_dir;
> --
> 1.8.3.1

Thanks for the patch.
Please address my feedback and re-submit.

(Note: Sorry - but I will be out-of-the-office the rest of this week,
so my responses will be delayed until next week.)

Regards,
 -- Tim


^ permalink raw reply

* USB: option: add support for DW5821e
From: Aleksander Morgado @ 2018-07-23 23:34 UTC (permalink / raw)
  To: johan; +Cc: gregkh, bjorn, linux-usb, Aleksander Morgado, stable

The device exposes AT, NMEA and DIAG ports in both USB configurations.

The patch explicitly ignores interfaces 0 and 1, as they're bound to
other drivers already; and also interface 6, which is a GNSS interface
for which we don't have a driver yet.

T:  Bus=01 Lev=03 Prnt=04 Port=00 Cnt=01 Dev#= 18 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  2
P:  Vendor=413c ProdID=81d7 Rev=03.18
S:  Manufacturer=DELL
S:  Product=DW5821e Snapdragon X20 LTE
S:  SerialNumber=0123456789ABCDEF
C:  #Ifs= 7 Cfg#= 2 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
I:  If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
I:  If#= 6 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)

T:  Bus=01 Lev=03 Prnt=04 Port=00 Cnt=01 Dev#= 16 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  2
P:  Vendor=413c ProdID=81d7 Rev=03.18
S:  Manufacturer=DELL
S:  Product=DW5821e Snapdragon X20 LTE
S:  SerialNumber=0123456789ABCDEF
C:  #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
I:  If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
Cc: stable <stable@vger.kernel.org>
---
 drivers/usb/serial/option.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 664e61f16b6a..0215b70c4efc 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -196,6 +196,8 @@ static void option_instat_callback(struct urb *urb);
 #define DELL_PRODUCT_5800_V2_MINICARD_VZW	0x8196  /* Novatel E362 */
 #define DELL_PRODUCT_5804_MINICARD_ATT		0x819b  /* Novatel E371 */
 
+#define DELL_PRODUCT_5821E			0x81d7
+
 #define KYOCERA_VENDOR_ID			0x0c88
 #define KYOCERA_PRODUCT_KPC650			0x17da
 #define KYOCERA_PRODUCT_KPC680			0x180a
@@ -1030,6 +1032,8 @@ static const struct usb_device_id option_ids[] = {
 	{ USB_DEVICE_AND_INTERFACE_INFO(DELL_VENDOR_ID, DELL_PRODUCT_5800_MINICARD_VZW, 0xff, 0xff, 0xff) },
 	{ USB_DEVICE_AND_INTERFACE_INFO(DELL_VENDOR_ID, DELL_PRODUCT_5800_V2_MINICARD_VZW, 0xff, 0xff, 0xff) },
 	{ USB_DEVICE_AND_INTERFACE_INFO(DELL_VENDOR_ID, DELL_PRODUCT_5804_MINICARD_ATT, 0xff, 0xff, 0xff) },
+	{ USB_DEVICE(DELL_VENDOR_ID, DELL_PRODUCT_5821E),
+	  .driver_info = RSVD(0) | RSVD(1) | RSVD(6) },
 	{ USB_DEVICE(ANYDATA_VENDOR_ID, ANYDATA_PRODUCT_ADU_E100A) },	/* ADU-E100, ADU-310 */
 	{ USB_DEVICE(ANYDATA_VENDOR_ID, ANYDATA_PRODUCT_ADU_500A) },
 	{ USB_DEVICE(ANYDATA_VENDOR_ID, ANYDATA_PRODUCT_ADU_620UW) },

^ permalink raw reply related

* Re: [PATCH] phy: qcom-qmp: Fix dts bindings to reflect reality
From: Rob Herring @ 2018-07-23 23:33 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Kishon Vijay Abraham I, Mark Rutland, devicetree,
	linux-kernel@vger.kernel.org, Vivek Gautam, Manu Gautam,
	Varadarajan Narayanan
In-Reply-To: <CAD=FV=UDq8oq+nKsa2mZWvAUueUFxEUSYz2JoZ=E7e131SOR=g@mail.gmail.com>

On Mon, Jul 23, 2018 at 11:37 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 23, 2018 at 7:04 AM, Rob Herring <robh@kernel.org> wrote:
> > On Fri, Jul 20, 2018 at 11:54 AM Doug Anderson <dianders@chromium.org> wrote:
> >>
> >> Hi,
> >>
> >> On Fri, Jul 20, 2018 at 10:26 AM, Rob Herring <robh@kernel.org> wrote:
> >> > On Fri, Jul 20, 2018 at 9:13 AM Doug Anderson <dianders@chromium.org> wrote:
> >> >>
> >> >> Rob,
> >> >>
> >> >> On Fri, Jul 20, 2018 at 7:10 AM, Rob Herring <robh@kernel.org> wrote:
> >> >> > On Fri, Jul 06, 2018 at 04:31:42PM -0700, Douglas Anderson wrote:
> >> >> >> A few patches have landed for the qcom-qmp PHY that affect how you
> >> >> >> would write a device tree node.  ...yet the bindings weren't updated.
> >> >> >> Let's remedy the situation and make the bindings refelect reality.
> >> >> >
> >> >> > "dt-bindings: phy: ..." for the subject.
> >> >>
> >> >> Sorry.  Every subsystem has different conventions for this so I
> >> >> usually just do a "git log" on the file and make my best guess.  I'll
> >> >> try to remember this for next time though.
> >> >
> >> > NP. I'd like to add this info to MAINTAINERS or maybe a git commit
> >> > hook could figure this out automagically.
> >> >
> >> >> In this case, though, it looks like this already landed.  I see this
> >> >> patch in Kishon's next branch.
> >> >>
> >> >>
> >> >> >> Fixes: efb05a50c956 ("phy: qcom-qmp: Add support for QMP V3 USB3 PHY")
> >> >> >> Fixes: ac0d239936bd ("phy: qcom-qmp: Add support for runtime PM")
> >> >> >> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >> >> >> ---
> >> >> >>
> >> >> >>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 14 ++++++++++++--
> >> >> >>  1 file changed, 12 insertions(+), 2 deletions(-)
> >> >> >>
> >> >> >> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> >> index 266a1bb8bb6e..0c7629e88bf3 100644
> >> >> >> --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> >> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> >> >> >> @@ -12,7 +12,14 @@ Required properties:
> >> >> >>              "qcom,sdm845-qmp-usb3-phy" for USB3 QMP V3 phy on sdm845,
> >> >> >>              "qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845.
> >> >> >>
> >> >> >> - - reg: offset and length of register set for PHY's common serdes block.
> >> >> >> + - reg:
> >> >> >> +   - For "qcom,sdm845-qmp-usb3-phy":
> >> >> >> +     - index 0: address and length of register set for PHY's common serdes
> >> >> >> +       block.
> >> >> >> +     - named register "dp_com" (using reg-names): address and length of the
> >> >> >> +       DP_COM control block.
> >> >> >
> >> >> > You need to list reg-names and what are the names for the other 2
> >> >> > regions?
> >> >>
> >> >> Here's the code works.  You can tell me how you want this expressed in
> >> >> the bindings:
> >> >>
> >> >> * In all cases the driver maps its main memory range (for the common
> >> >> serdes block) without specifying any name.  This is equivalent to
> >> >> asking for index 0.
> >> >>
> >> >> * For "qcom,sdm845-qmp-usb3-phy" the driver requests a second memory
> >> >> range by name using the name "dp_com".
> >> >>
> >> >> ...basically the driver is inconsistent between using names and
> >> >> indices and I was trying to document that fact.
> >> >
> >> > That's fine as long as the indices are fixed.
> >> >
> >> >>
> >> >> I guess options:
> >> >>
> >> >> 1. I could reword this so it's clearer (open to suggestions)
> >> >>
> >> >> 2. I could add something to the bindings saying that the first reg
> >> >> name should be "reg-base" or something.  Then the question is whether
> >> >> we should go to the code and start enforcing that.  If we do choose to
> >> >> enforce it then it's technically breaking compatibility (though I
> >> >> doubt there is any real dts in the wild).  If we don't choose to
> >> >> enforce it then why did we bother saying what it should be named?
> >> >
> >> > I think you need to state index 1 is dp_com (and only for
> >> > "qcom,sdm845-qmp-usb3-phy") and a name for index 0. 'reg-base' doesn't
> >> > seem great, but I don't have another suggestion.
> >>
> >> ...but why do we bother giving "dp_com" a name if we're saying it has
> >> to be index 1?  It feels like the author of the driver was trying to
> >> transition from specifying to specifying registers by index to
> >> specifying them by name, but left the first register specified by
> >> index for compatibility (or code simplicity?).  It seems like the
> >> whole point of referring to things by name is to _not_ force the index
> >> number.
> >
> > No. Specifying the order and indexes is how bindings are done.
> > "-names" is extra information, not a license to change the rules.
>
> OK.
>
> Just for context: I'm not trying to be argumentative or anything--I
> just seem to be lacking a fundamental understanding of why reg-names
> exists and when it should be used.  I'm trying to ask questions so I
> can have a better intuition here and submit better patches / do better
> reviews.  I'm sorry if I'm coming off as a jerk.  :(  I'm not trying
> to be.
>
> Do you happen to know if there's anything written up that explains all
> the conventions around reg-names and I can just read that?

Probably not. We should add it to the DT spec I guess.

> For context, it seems to me that clocks and registers have different
> conventions, but I certainly could be wrong.  From what I've seen
> clocks are often specified in bindings by name and referred to in code
> by name.  Conversely reg-names seems to be highly discouraged and
> people are told to just refer to them by index.  The difference
> between the two always seemed strange to me, though I always assumed
> it was because it was more common to just have one or two register
> ranges but a whole chunk of clocks (and more get added over time as
> the driver matures).

That probably is because the clock binding has had clock-names from
the start (it may have been the first one). That was probably partly
due to the clock API also was mainly by name already if we want to
admit Linux influence on bindings. IIRC, reg-names came along for the
OMAP folks and was related to the hwmods stuff. Part of it was also
people wanting platform_get_resource_by_name to work when converting
to DT. If you haven't figured it out, I'm still not really a fan. It
is definitely more useful for some bindings than others.

There's a few cases where I think *-names is useful. The first is what
I mentioned where you could have holes in the list. The 2nd is things
like the standardized "wakeup" name for wakeup interrupts.

I really don't like seeing things like reset-names with a single entry
like "rst".

> >> I'm imagining a future compatible string that adds yet another address
> >> range.  Presumably we'd refer to this by name too.  In that case both
> >> of these would be fine:
> >>
> >> reg = <0xa x>, <0xb x>, <0xc x>;
> >> reg-names = "reg-base", "dp_com", "new_range"
> >>
> >> reg = <0xa x>, <0xc x>, <0xb x>;
> >> reg-names = "reg-base", "new_range", "dp_com"
> >
> > But why. There is absolutely no need to support both of these.
>
> I guess I'm trying to understand the motivation for reg-names.  If you
> say that something must be index 1 and also have reg-name of "dp_com"
> then the reg-name isn't buying us anything and maybe the more proper
> fix is to change the driver to work by index and drop the whole name
> thing here?  Should I do that?

That would be fine by me.

> > What you could need to support is this:
> >
> > reg-names = "reg-base", "new_range";
> >
> > The order is still fixed, but we have optional entries in the middle.
> > And that is the case when using -names is helpful. Otherwise, "-names"
> > is just unnecessary bloat to DT.
>
> From what you're saying the _only_ reason you'd ever want to use
> reg-names is if there is an optional register range.  Is that right?

IMO, yes. But I'm not going to tell people to remove them unless there
is only a single entry.

> I think for me intuitively I expect that drivers will change and
> expand over time and I'd expect more / different (and maybe optional)
> ranges to be added.  In general I feel like named registers scales
> better with less complexity.

They should only really change for new h/w and that should imply a new
compatible which can also define what is at each index.

> >> > For the driver, it's not the driver's job to enforce any of this.
> >>
> >> Sure, but anything that the driver doesn't enforce people will get
> >> wrong.  In other words if we say that the name of the first register
> >> ought to be "reg-base" but don't enforce it then someone will later
> >> come in and say it's stupid that one of the names uses a dash and the
> >> other an underscore.  They'll change "reg-base" and it will work.
> >> Someone else will decide that "reg-base" is a dumb name and will
> >> change it to "serdes".  It will still work.  Now we have a bindings
> >> that claims "reg-base" and lots of different device trees in practice.
> >
> > You are arguing against making the binding stricter and then worrying
> > about the driver getting things wrong. Stricter bindings leave less to
> > interpretation by the driver and less chance to get things wrong.
> > Unless "anything goes" and then the binding can never be "wrong".
>
> I guess I'm saying that my intuition says that over-specifying things
> seems bad to me.  The driver will either ask for a register by name or
> by index and not both.  In my mind either is a fine way to do it so as
> long as the driver and the binding is consistent then we have a
> functioning system.  Specifying that people have to do get both the
> index and name right basically gives people twice as many places to do
> things wrong.

Validation should solve that problem mostly. The only thing we can't
validate is if the names are the right order, but the addresses are
not.

> > Again, it is not the kernel's job to validate bindings even though we
> > have little else right now. That will change soon hopefully.
> >
> >> If the whole "keep things compatible" is important then what matters
> >> more is what's happening in practice, not what's should happen in
> >> theory.
> >>
> >> IMO if the driver isn't enforcing that the first field be called
> >> "reg-base" then it shouldn't be in the bindings doc.
> >>
> >>
> >> BTW: I use the name "reg-base" in my example because that's what the
> >> register was named in downstream device trees that I found.  Even if
> >> the name isn't terribly appealing, if it's not terribly objectionable
> >> I'd rather pick something that's closer to what people used in
> >> practice.
> >
> > I don't really care. People just make shit up anyway.
> >
> >>
> >> ---
> >>
> >> How about this?
> >
> > No.
> >
> >>
> >> - reg:
> >>   - For "qcom,sdm845-qmp-usb3-phy" (names mapped by reg-names):
> >>     - "reg-base": address and length of register set for PHY's common serdes
> >>       block.  MUST BE THE FIRST RANGE LISTED IN THE LIST (AKA index 0).  Note
> >>       that the name of "reg-base" is suggested but not enforced.
> >>     - "dp-com": address and length of the DP_COM control block.
> >>   - For all others (only one range--don't use reg-names)
> >>     - offset and length of register set for PHY's common serdes block.
>
> OK, I guess I will try again then and you can tell me when I get it
> right (unless you tell me I should just change the driver to not use
> named registers at all).  How about:
>
> - reg:
>   - For "qcom,sdm845-qmp-usb3-phy":
>     - index 0: address and length of register set for PHY's common serdes
>                block.
>     - index 1: address and length of the DP_COM control block.

Instead of repeating index 0 below, just say "for
'qcom,sdm845-qmp-usb3-phy' only" here.

>   - For all others:
>     - index 0: address and length of register set for PHY's common serdes
>                block.
> - reg-names:
>   - For "qcom,sdm845-qmp-usb3-phy":
>     - Should be: "reg-base", "dp_com"
>   - For all others:
>     - The reg-names property shouldn't be defined.
>
>
> -Doug

^ permalink raw reply

* Re: [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Vladimir Zapolskiy @ 2018-07-23 23:31 UTC (permalink / raw)
  To: Oleksij Rempel, Shawn Guo, Fabio Estevam, Rob Herring,
	Mark Rutland, A.s. Dong, Vladimir Zapolskiy
  Cc: devicetree, linux-arm-kernel, kernel, dl-linux-imx
In-Reply-To: <20180722063923.30222-6-o.rempel@pengutronix.de>

Hi Oleksij,

On 07/22/2018 09:39 AM, Oleksij Rempel wrote:
> The Mailbox controller is able to send messages (up to 4 32 bit words)
> between the endpoints.
> 
> This driver was tested using the mailbox-test driver sending messages
> between the Cortex-A7 and the Cortex-M4.
> 
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

[snip]

> +static int imx_mu_startup(struct mbox_chan *chan)
> +{
> +	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +	struct imx_mu_con_priv *cp = chan->con_priv;
> +	int ret;
> +
> +	cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> +				      cp->idx);
> +	if (!cp->irq_desc)
> +		return -ENOMEM;
> +

Again I would suggest to move this allocation to the loop in .probe function.

[snip]

> +
> +	for (i = 0; i < IMX_MU_CHANS; i++) {
> +		struct imx_mu_con_priv *cp = &priv->con_priv[i];
> +
> +		cp->idx = i;
> +		cp->irq = irq;

^^^^ right over here.

> +		priv->mbox_chans[i].con_priv = cp;
> +	}
> +

please feel free to add my technical side review tag to the next version:

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

FWIW I find that review comments from Lucas are pretty valid, I would
recommend to incorporate them.

--
Best wishes,
Vladimir

^ permalink raw reply

* [PATCH v6 5/5] mailbox: Add support for i.MX7D messaging unit
From: Vladimir Zapolskiy @ 2018-07-23 23:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180722063923.30222-6-o.rempel@pengutronix.de>

Hi Oleksij,

On 07/22/2018 09:39 AM, Oleksij Rempel wrote:
> The Mailbox controller is able to send messages (up to 4 32 bit words)
> between the endpoints.
> 
> This driver was tested using the mailbox-test driver sending messages
> between the Cortex-A7 and the Cortex-M4.
> 
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

[snip]

> +static int imx_mu_startup(struct mbox_chan *chan)
> +{
> +	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
> +	struct imx_mu_con_priv *cp = chan->con_priv;
> +	int ret;
> +
> +	cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]",
> +				      cp->idx);
> +	if (!cp->irq_desc)
> +		return -ENOMEM;
> +

Again I would suggest to move this allocation to the loop in .probe function.

[snip]

> +
> +	for (i = 0; i < IMX_MU_CHANS; i++) {
> +		struct imx_mu_con_priv *cp = &priv->con_priv[i];
> +
> +		cp->idx = i;
> +		cp->irq = irq;

^^^^ right over here.

> +		priv->mbox_chans[i].con_priv = cp;
> +	}
> +

please feel free to add my technical side review tag to the next version:

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

FWIW I find that review comments from Lucas are pretty valid, I would
recommend to incorporate them.

--
Best wishes,
Vladimir

^ permalink raw reply

* qmi_wwan: fix interface number for DW5821e production firmware
From: Aleksander Morgado @ 2018-07-23 23:31 UTC (permalink / raw)
  To: bjorn; +Cc: davem, netdev, linux-usb, Aleksander Morgado, stable

The original mapping for the DW5821e was done using a development
version of the firmware. Confirmed with the vendor that the final
USB layout ends up exposing the QMI control/data ports in USB
config #1, interface #0, not in interface #1 (which is now a HID
interface).

T:  Bus=01 Lev=03 Prnt=04 Port=00 Cnt=01 Dev#= 16 Spd=480 MxCh= 0
D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  2
P:  Vendor=413c ProdID=81d7 Rev=03.18
S:  Manufacturer=DELL
S:  Product=DW5821e Snapdragon X20 LTE
S:  SerialNumber=0123456789ABCDEF
C:  #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=qmi_wwan
I:  If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=option
I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option

Fixes: e7e197edd09c25 ("qmi_wwan: add support for the Dell Wireless 5821e module")
Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
Cc: stable <stable@vger.kernel.org>
---
 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 8fac8e132c5b..0ed06d670a5f 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1246,7 +1246,7 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x413c, 0x81b3, 8)},	/* Dell Wireless 5809e Gobi(TM) 4G LTE Mobile Broadband Card (rev3) */
 	{QMI_FIXED_INTF(0x413c, 0x81b6, 8)},	/* Dell Wireless 5811e */
 	{QMI_FIXED_INTF(0x413c, 0x81b6, 10)},	/* Dell Wireless 5811e */
-	{QMI_FIXED_INTF(0x413c, 0x81d7, 1)},	/* Dell Wireless 5821e */
+	{QMI_FIXED_INTF(0x413c, 0x81d7, 0)},	/* Dell Wireless 5821e */
 	{QMI_FIXED_INTF(0x03f0, 0x4e1d, 8)},	/* HP lt4111 LTE/EV-DO/HSPA+ Gobi 4G Module */
 	{QMI_FIXED_INTF(0x03f0, 0x9d1d, 1)},	/* HP lt4120 Snapdragon X5 LTE */
 	{QMI_FIXED_INTF(0x22de, 0x9061, 3)},	/* WeTelecom WPD-600N */

^ permalink raw reply related

* Re: [PATCH v2] hexagon: modify ffs() and fls() to return int
From: Richard Kuo @ 2018-07-23 23:31 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: LKML, linux-hexagon, Geert Uytterhoeven
In-Reply-To: <39ac5a3f-b5fa-a928-6caf-a7a01251f5ea@infradead.org>

On Mon, Jul 23, 2018 at 04:27:47PM -0700, Randy Dunlap wrote:
> On 07/23/2018 03:50 PM, Richard Kuo wrote:
> > On Sun, Jul 22, 2018 at 04:03:58PM -0700, Randy Dunlap wrote:
> >> From: Randy Dunlap <rdunlap@infradead.org>
> >>
> >> Building drivers/mtd/nand/raw/nandsim.c on arch/hexagon/ produces a
> >> printk format build warning.  This is due to hexagon's ffs() being
> >> coded as returning long instead of int.
> >>
> >> Fix the printk format warning by changing all of hexagon's ffs() and
> >> fls() functions to return int instead of long.  The variables that
> >> they return are already int instead of long.  This return type
> >> matches the return type in <asm-generic/bitops/>.
> >>
> >> ../drivers/mtd/nand/raw/nandsim.c: In function 'init_nandsim':
> >> ../drivers/mtd/nand/raw/nandsim.c:760:2: warning: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'long int' [-Wformat]
> >>
> >> There are no ffs() or fls() allmodconfig build errors after making this
> >> change.
> >>
> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> >> Cc: Richard Kuo <rkuo@codeaurora.org>
> >> Cc: linux-hexagon@vger.kernel.org
> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> >> ---
> >> v2:
> >> add hexagon contacts, drop erroneous sh contacts; [thanks, Geert]
> >> only change return type for ffs() and fls() [thanks, Geert]
> >>   [drop the changes for ffz(), __ffs(), and __fls()]
> >>
> >>  arch/hexagon/include/asm/bitops.h |    4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> > 
> > 
> > Acked-by: Richard Kuo <rkuo@codeaurora.org>
> > 
> 
> Hi Richard,
> 
> You are listed as the arch/hexagon/ maintainer.  Can you please merge these
> patches?
> 
> thanks,
> -- 
> ~Randy

Yes, I can queue it up and take it through my tree.


Thanks,
Richard Kuo


-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, 
a Linux Foundation Collaborative Project

^ permalink raw reply

* [PATCH] remoteproc/davinci: Mark error recovery as disabled
From: Suman Anna @ 2018-07-23 23:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, Sekhar Nori, linux-arm-kernel, linux-kernel,
	Suman Anna

The Davinci remoteproc driver does not support error recovery at
present, so mark the corresponding remoteproc flag appropriately
so that the debugfs flag shows the value as 'disabled' by default.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/da8xx_remoteproc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index b668e32996e2..53160d5ed543 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -284,6 +284,9 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 		goto free_mem;
 	}
 
+	/* error recovery is not supported at present */
+	rproc->recovery_disabled = true;
+
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
 	drproc->dsp_clk = dsp_clk;
-- 
2.18.0


^ permalink raw reply related

* [PATCH] remoteproc/davinci: Mark error recovery as disabled
From: Suman Anna @ 2018-07-23 23:27 UTC (permalink / raw)
  To: linux-arm-kernel

The Davinci remoteproc driver does not support error recovery at
present, so mark the corresponding remoteproc flag appropriately
so that the debugfs flag shows the value as 'disabled' by default.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/da8xx_remoteproc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index b668e32996e2..53160d5ed543 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -284,6 +284,9 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 		goto free_mem;
 	}
 
+	/* error recovery is not supported at present */
+	rproc->recovery_disabled = true;
+
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
 	drproc->dsp_clk = dsp_clk;
-- 
2.18.0

^ permalink raw reply related

* [PATCH] remoteproc/davinci: Mark error recovery as disabled
From: Suman Anna @ 2018-07-23 23:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-remoteproc, Sekhar Nori, linux-arm-kernel, linux-kernel,
	Suman Anna

The Davinci remoteproc driver does not support error recovery at
present, so mark the corresponding remoteproc flag appropriately
so that the debugfs flag shows the value as 'disabled' by default.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/da8xx_remoteproc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c
index b668e32996e2..53160d5ed543 100644
--- a/drivers/remoteproc/da8xx_remoteproc.c
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -284,6 +284,9 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 		goto free_mem;
 	}
 
+	/* error recovery is not supported at present */
+	rproc->recovery_disabled = true;
+
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
 	drproc->dsp_clk = dsp_clk;
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH v2] hexagon: modify ffs() and fls() to return int
From: Randy Dunlap @ 2018-07-23 23:27 UTC (permalink / raw)
  To: Richard Kuo; +Cc: LKML, linux-hexagon, Geert Uytterhoeven
In-Reply-To: <20180723225025.GC12771@codeaurora.org>

On 07/23/2018 03:50 PM, Richard Kuo wrote:
> On Sun, Jul 22, 2018 at 04:03:58PM -0700, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Building drivers/mtd/nand/raw/nandsim.c on arch/hexagon/ produces a
>> printk format build warning.  This is due to hexagon's ffs() being
>> coded as returning long instead of int.
>>
>> Fix the printk format warning by changing all of hexagon's ffs() and
>> fls() functions to return int instead of long.  The variables that
>> they return are already int instead of long.  This return type
>> matches the return type in <asm-generic/bitops/>.
>>
>> ../drivers/mtd/nand/raw/nandsim.c: In function 'init_nandsim':
>> ../drivers/mtd/nand/raw/nandsim.c:760:2: warning: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'long int' [-Wformat]
>>
>> There are no ffs() or fls() allmodconfig build errors after making this
>> change.
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Richard Kuo <rkuo@codeaurora.org>
>> Cc: linux-hexagon@vger.kernel.org
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> v2:
>> add hexagon contacts, drop erroneous sh contacts; [thanks, Geert]
>> only change return type for ffs() and fls() [thanks, Geert]
>>   [drop the changes for ffz(), __ffs(), and __fls()]
>>
>>  arch/hexagon/include/asm/bitops.h |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> 
> Acked-by: Richard Kuo <rkuo@codeaurora.org>
> 

Hi Richard,

You are listed as the arch/hexagon/ maintainer.  Can you please merge these
patches?

thanks,
-- 
~Randy

^ permalink raw reply

* [PATCH 0/2] PCI: NVMe reset quirk
From: Alex Williamson @ 2018-07-23 22:24 UTC (permalink / raw)
  To: linux-pci; +Cc: linux-kernel, linux-nvme

As discussed in the 2nd patch, at least one NVMe controller sometimes
doesn't like being reset while enabled and another will timeout during
a subsequent re-enable if it happens too quickly after reset.
Introduce a device specific reset quirk for all NVMe class devices so
that we can try to get reliable behavior from them for device
assignment and any other users of the PCI subsystem reset interface.

Patches against current PCI next branch.  Thanks,

Alex

---

Alex Williamson (2):
      PCI: Export pcie_has_flr()
      PCI: NVMe device specific reset quirk


 drivers/pci/pci.c    |    3 +
 drivers/pci/quirks.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  |    1 
 3 files changed, 115 insertions(+), 1 deletion(-)

^ permalink raw reply

* [Drbd-dev] BuildRequires for drbd-utils
From: Digimer @ 2018-07-23 23:19 UTC (permalink / raw)
  To: drbd-dev

Hi all,

  I'm working on RPMs for Fedora, and noticed two missing build
dependencies in drbd-utils-9.5.0;

BuildRequires: automake
BuildRequires: libxslt

Cheers

-- 
Digimer
Papers and Projects: https://alteeve.com/w/
"I am, somehow, less interested in the weight and convolutions of
Einstein’s brain than in the near certainty that people of equal talent
have lived and died in cotton fields and sweatshops." - Stephen Jay Gould

^ permalink raw reply

* Re: [PATCH v7 08/12] mfd: intel-peci-client: Add PECI client MFD driver
From: Randy Dunlap @ 2018-07-23 22:21 UTC (permalink / raw)
  To: Jae Hyun Yoo, Jean Delvare, Guenter Roeck, Rob Herring,
	Mark Rutland, Lee Jones, Joel Stanley, Andrew Jeffery,
	Jonathan Corbet, Greg Kroah-Hartman, Gustavo Pimentel,
	Kishon Vijay Abraham I, Lorenzo Pieralisi, Darrick J . Wong,
	Eric Sandeen, Arnd Bergmann, Wu Hao, Tomohiro Kusumi,
	Bryant G . Ly, Frederic Barrat, David S . Miller,
	Mauro Carvalho Chehab, Andrew Morton, Philippe Ombredanne,
	Vinod Koul, Stephen Boyd, David Kershner, Uwe Kleine-Konig,
	Sagar Dharia, Johan Hovold, Thomas Gleixner, Juergen Gross,
	Cyrille Pitchen
  Cc: linux-hwmon, devicetree, linux-kernel, linux-arm-kernel,
	linux-aspeed, linux-doc, openbmc, James Feist, Jason M Biils,
	Vernon Mauery
In-Reply-To: <20180723214751.1733-9-jae.hyun.yoo@linux.intel.com>

On 07/23/2018 02:47 PM, Jae Hyun Yoo wrote:
> This commit adds PECI client MFD driver.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Andrew Jeffery <andrew@aj.id.au>
> Cc: James Feist <james.feist@linux.intel.com>
> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
> Cc: Joel Stanley <joel@jms.id.au>
> Cc: Vernon Mauery <vernon.mauery@linux.intel.com>
> ---
>  drivers/mfd/Kconfig                   |  14 ++
>  drivers/mfd/Makefile                  |   1 +
>  drivers/mfd/intel-peci-client.c       | 182 ++++++++++++++++++++++++++
>  include/linux/mfd/intel-peci-client.h |  81 ++++++++++++
>  4 files changed, 278 insertions(+)
>  create mode 100644 drivers/mfd/intel-peci-client.c
>  create mode 100644 include/linux/mfd/intel-peci-client.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index f3fa516011ec..e38b591479d4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -595,6 +595,20 @@ config MFD_INTEL_MSIC
>  	  Passage) chip. This chip embeds audio, battery, GPIO, etc.
>  	  devices used in Intel Medfield platforms.
>  
> +config MFD_INTEL_PECI_CLIENT
> +	bool "Intel PECI client"
> +	depends on (PECI || COMPILE_TEST)
> +	select MFD_CORE
> +	help
> +	  If you say yes to this option, support will be included for the
> +	  multi-funtional Intel PECI (Platform Environment Control Interface)

	  multi-functional

> +	  client. PECI is a one-wire bus interface that provides a communication
> +	  channel from PECI clients in Intel processors and chipset components
> +	  to external monitoring or control devices.
> +
> +	  Additional drivers must be enabled in order to use the functionality
> +	  of the device.
> +
>  config MFD_IPAQ_MICRO
>  	bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support"
>  	depends on SA1100_H3100 || SA1100_H3600


-- 
~Randy

^ permalink raw reply

* Re: [Fuego] [PATCH] busybox: modify the code of option g+x in chmod1 test
From: Tim.Bird @ 2018-07-23 23:24 UTC (permalink / raw)
  To: wangmy, fuego
In-Reply-To: <1532324114-87301-1-git-send-email-wangmy@cn.fujitsu.com>

OK - this is applied and pushed.  But it was a bit harder to apply than anticipated
because your mailer decided to put the message body in base64 encoding.

You other patches don't have this issue.  I'm not sure what happened on this 
one.  The only thing I can think of is maybe there was a non-8-bit-ASCII
character somewhere in the patch?  The only "strange" characters are
the quotes (maybe in the patch message) - maybe one of those got turned into
a Unicode char or some other extended character-set character.

If you can, please check this.
 -- Tim


> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Wang Mingyu
> Sent: Sunday, July 22, 2018 10:35 PM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH] busybox: modify the code of option g+x in chmod1
> test
> 
> Change the cut to retrieve the group permission bytes, to check that they are
> 'execute' ('x').
> 
> Signed-off-by: Wang Mingyu <wangmy@cn.fujitsu.com>
> ---
>  engine/tests/Functional.busybox/tests/busybox_chmod1.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> b/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> index cdc7d18..2eef95a 100644
> --- a/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> +++ b/engine/tests/Functional.busybox/tests/busybox_chmod1.sh
> @@ -21,7 +21,7 @@ else
>  fi;
> 
>  busybox chmod g+x ./test_dir/test1
> -if [ "$(busybox ls -l ./test_dir | grep -v "total" | cut -b 1-3)" = "-rw" ]
> +if [ "$(busybox ls -l ./test_dir | grep -v "total" | cut -b 5-7)" = "r-x" ]
>  then
>      echo " -> $test: Changed file permissions verification#2 succeeded."
>  else
> --
> 1.8.3.1
> 
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

^ permalink raw reply

* [PATCH net-next] tls: Fix improper revert in zerocopy_from_iter
From: Doron Roberts-Kedes @ 2018-07-23 22:20 UTC (permalink / raw)
  To: David S . Miller
  Cc: Dave Watson, Vakul Garg, Matt Mullins, netdev,
	Doron Roberts-Kedes

The current code is problematic because the iov_iter is reverted and
never advanced in the non-error case. This patch skips the revert in the
non-error case. This patch also fixes the amount by which the iov_iter
is reverted. Currently, iov_iter is reverted by size, which can be
greater than the amount by which the iter was actually advanced.
Instead, mimic the tx path which reverts by the difference before and
after zerocopy_from_iter. 

Fixes: 4718799817c5 ("tls: Fix zerocopy_from_iter iov handling")
Signed-off-by: Doron Roberts-Kedes <doronrk@fb.com>
---
 net/tls/tls_sw.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 490f2bcc6313..2ea000baebf8 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -276,7 +276,7 @@ static int zerocopy_from_iter(struct sock *sk, struct iov_iter *from,
 			      int length, int *pages_used,
 			      unsigned int *size_used,
 			      struct scatterlist *to, int to_max_pages,
-			      bool charge, bool revert)
+			      bool charge)
 {
 	struct page *pages[MAX_SKB_FRAGS];
 
@@ -327,8 +327,6 @@ static int zerocopy_from_iter(struct sock *sk, struct iov_iter *from,
 out:
 	*size_used = size;
 	*pages_used = num_elem;
-	if (revert)
-		iov_iter_revert(from, size);
 
 	return rc;
 }
@@ -431,7 +429,7 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 				&ctx->sg_plaintext_size,
 				ctx->sg_plaintext_data,
 				ARRAY_SIZE(ctx->sg_plaintext_data),
-				true, false);
+				true);
 			if (ret)
 				goto fallback_to_reg_send;
 
@@ -811,6 +809,7 @@ int tls_sw_recvmsg(struct sock *sk,
 			    likely(!(flags & MSG_PEEK)))  {
 				struct scatterlist sgin[MAX_SKB_FRAGS + 1];
 				int pages = 0;
+				int orig_chunk = chunk;
 
 				zc = true;
 				sg_init_table(sgin, MAX_SKB_FRAGS + 1);
@@ -820,9 +819,11 @@ int tls_sw_recvmsg(struct sock *sk,
 				err = zerocopy_from_iter(sk, &msg->msg_iter,
 							 to_copy, &pages,
 							 &chunk, &sgin[1],
-							 MAX_SKB_FRAGS,	false, true);
-				if (err < 0)
+							 MAX_SKB_FRAGS,	false);
+				if (err < 0) {
+					iov_iter_revert(&msg->msg_iter, chunk - orig_chunk);
 					goto fallback_to_reg_recv;
+				}
 
 				err = decrypt_skb(sk, skb, sgin);
 				for (; pages > 0; pages--)
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCHv3 2/2] mtd: m25p80: restore the status of SPI flash when exiting
From: Boris Brezillon @ 2018-07-23 23:22 UTC (permalink / raw)
  To: NeilBrown
  Cc: Brian Norris, Zhiqiang Hou, linux-mtd, Linux Kernel,
	David Woodhouse, Boris BREZILLON, Marek Vasut, Richard Weinberger,
	Cyrille Pitchen
In-Reply-To: <87in55po3a.fsf@notabene.neil.brown.name>

On Tue, 24 Jul 2018 08:46:33 +1000
NeilBrown <neilb@suse.com> wrote:

> On Mon, Jul 23 2018, Brian Norris wrote:
> 
> > Hi Boris,
> >
> > On Mon, Jul 23, 2018 at 1:10 PM, Boris Brezillon
> > <boris.brezillon@bootlin.com> wrote:  
> >> On Mon, 23 Jul 2018 11:13:50 -0700
> >> Brian Norris <computersforpeace@gmail.com> wrote:  
> >>> I noticed this got merged, but I wanted to put my 2 cents in here:  
> >>
> >> I wish you had replied to this thread when it was posted (more than
> >> 6 months ago). Reverting the patch now implies making some people
> >> unhappy because they'll have to resort to their old out-of-tree
> >> hacks :-(.  
> >
> > I'd say I'm sorry for not following things closely these days, but I'm
> > not really that sorry. There are plenty of other capable hands. And if
> > y'all shoot yourselves in the foot, so be it. This patch isn't going
> > to blow things up, but now that I did finally notice it (because it
> > happened to show up in a list of backports I was looking at), I
> > thought better late than never to remind you.
> >
> > For way of notification: Marek already noticed that we've started down
> > a slippery slope months ago:
> >
> > https://lkml.org/lkml/2018/4/8/141
> > Re: [PATCH] mtd: spi-nor: clear Extended Address Reg on switch to
> > 3-byte addressing.
> >
> > I'm not quite sure why that wasn't taken to its logical conclusion --
> > that the hack should be reverted.
> >
> > This problem has been noted many times already, and we've always
> > stayed on the side of *avoiding* this hack. A few references from a
> > search of my email:
> >
> > http://lists.infradead.org/pipermail/linux-mtd/2013-March/046343.html
> > [PATCH 1/3] mtd: m25p80: utilize dedicated 4-byte addressing commands
> >
> > http://lists.infradead.org/pipermail/barebox/2014-September/020682.html
> > [RFC] MTD m25p80 3-byte addressing and boot problem
> >
> > http://lists.infradead.org/pipermail/linux-mtd/2015-February/057683.html
> > [PATCH 2/2] m25p80: if supported put chip to deep power down if not used
> >  
> >>> On Wed, Dec 06, 2017 at 10:53:42AM +0800, Zhiqiang Hou wrote:  
> >>> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>> >
> >>> > Restore the status to be compatible with legacy devices.
> >>> > Take Freescale eSPI boot for example, it copies (in 3 Byte
> >>> > addressing mode) the RCW and bootloader images from SPI flash
> >>> > without firing a reset signal previously, so the reboot command
> >>> > will fail without reseting the addressing mode of SPI flash.
> >>> > This patch implement .shutdown function to restore the status
> >>> > in reboot process, and add the same operation to the .remove
> >>> > function.  
> >>>
> >>> We have previously rejected this patch multiple times, because the above
> >>> comment demonstrates a broken product.  
> >>
> >> If we were to only support working HW parts, I fear Linux would not
> >> support a lot of HW (that's even more true when it comes to flashes :P).  
> >
> > You stopped allowing UBI to attach to MLC NAND recently, no? That
> > sounds like almost the same boat -- you've probably killed quite a few
> > shitty products, if they were to use mainline directly.
> >
> > Anyway, that's derailing the issue. Supporting broken hardware isn't
> > something you try to do by applying the same hack to all systems. You
> > normally try to apply your hack as narrowly as possible. You seem to
> > imply that below. So maybe that's a solution to move forward with. But
> > I'd personally be just as happy to see the patch reverted.
> >  
> >>> You cannot guarantee that all
> >>> reboots will invoke the .shutdown() method -- what about crashes? What
> >>> about watchdog resets? IIUC, those will hit the same broken behavior,
> >>> and have unexepcted behavior in your bootloader.  
> >>
> >> Yes, there are corner cases that are not addressed with this approach,  
> >
> > Is a system crash really a corner case? :D
> >  
> >> but it still seems to improve things. Of course, that means the
> >> user should try to re-route all HW reset sources to SW ones (RESET input
> >> pin muxed to the GPIO controller, watchdog generating an interrupt
> >> instead of directly asserting the RESET output pin), which is not always
> >> possible, but even when it's not, isn't it better to have a setup that
> >> works fine 99% of the time instead of 50% of the time?  
> >
> > Perhaps, but not at the expense of future development. And
> > realistically, no one is doing that if they have this hack. Most
> > people won't even know that this hack is protecting them at all (so
> > again, they won't try to mitigate the problem any further).
> >  
> >>> I suppose one could argue for doing this in remove(), but AIUI you're
> >>> just papering over system bugs by introducing the shutdown() function
> >>> here. Thus, I'd prefer we drop the shutdown() method to avoid misleading
> >>> other users of this driver.  
> >>
> >> I understand your point. But if the problem is about making sure people
> >> designing new boards get that right, why not complaining at probe time
> >> when things are wrong?
> >>
> >> I mean, spi_nor_restore() seems to only do something on very specific
> >> NORs (those on which a SW RESET does not resets the addressing
> >> mode).  
> >
> > The point isn't that SW RESET doesn't reset the addressing mode -- it
> > does on any flash I've seen. The point is that most systems are built
> > around a stateless assumption in these flash. IIRC, there wasn't even
> > a SW RESET command at all until these "huge" flash came around and
> > stateful addressing modes came about. So boot ROMs and bootloaders
> > would have to be updated to start figuring out when/how to do this SW
> > RESET. And once two vendors start doing it differently (I'm not sure:
> > have they done this already? I think so) it's no longer something a
> > boot ROM will get right.
> >
> > The only way to get this stuff right is to have a hardware reset, or
> > else to avoid all of the stateful modes in software.
> >  
> >> So, how about adding a flag that says "my board has the NOR HW
> >> RESET pin wired" (there would be a DT props to set that flag). Then you
> >> add a WARN_ON() when this flag is not set and a NOR chip impacted by
> >> this bug is detected.  
> >
> > I'd kinda prefer the reverse. There really isn't a need to document
> > anything for a working system (software usually can't control this
> > RESET pin). The burden should be on the b0rked system to document
> > where it needs unsound hacks to survive.
> >  
> >> This way you make sure people are informed that
> >> they're doing something wrong, and for those who can't change their HW
> >> (because it's already widely deployed), you have a fix that improve
> >> things.  
> >
> > Or even better: put this hack behind a DT flag, so that one has to
> > admit that their board design is broken before it will even do
> > anything. Proposal: "linux,badly-designed-flash-reset".
> >
> > But, I'd prefer just (partially?) reverting this, and let the authors
> > submit something that works. We're not obligated to keep bad hacks in
> > the kernel.
> >
> > Brian  
> 
> One possibility that occurred to me when I was exploring this issue is
> to revert to 3-byte mode whenever 4-byte was not actively in use.
> So any access beyond 16Meg is:
>  switch-to-4-byte ; perform IO ; switch to 3-byte
> or similar.  On my hardware it would be more efficient to
> use the 4-byte opcode to perform the IO, then reset the cached
> 4th address byte that the NOR chip transparently remembered.
> 
> This adds a little overhead, but should be fairly robust.
> It doesn't help if something goes terribly wrong while IO is happening,
> but I don't think any other software solution does either.
> 
> How would you see that approach?

I think the problem stands: people that have proper HW mitigation for
this problem (NOR chip is reset when the Processor is reset) don't want
to pay the overhead. So, even if we go for this approach, we probably
want to only do that for broken HW.

^ permalink raw reply

* Re: [patch v3 -mm 3/6] mm, memcg: add hierarchical usage oom policy
From: David Rientjes @ 2018-07-23 23:22 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Andrew Morton, Michal Hocko, Vladimir Davydov, Johannes Weiner,
	Tejun Heo, cgroups, linux-kernel, linux-mm
In-Reply-To: <20180723212855.GA25062@castle>

On Mon, 23 Jul 2018, Roman Gushchin wrote:

> > Roman, I'm trying to make progress so that the cgroup aware oom killer is 
> > in a state that it can be merged.  Would you prefer a second tunable here 
> > to specify a cgroup's points includes memory from its subtree?
> 
> Hi, David!
> 
> It's hard to tell, because I don't have a clear picture of what you're
> suggesting now.

Each patch specifies what it does rather elaborately.  If there's 
confusion on what this patch, or any of the patches in this patchset, is 
motivated by or addresses, please call it out specifically.

> My biggest concern about your last version was that it's hard
> to tell what oom_policy really defines. Each value has it's own application
> rules, which is a bit messy (some values are meaningful for OOMing cgroup only,
> other are reading on hierarchy traversal).
> If you know how to make it clear and non-contradictory,
> please, describe the proposed interface.
> 

As my initial response stated, "tree" has cgroup aware properties but it 
considers the subtree usage as its own.  I do not know of any usecase, 
today or in the future, that would want subtree usage accounted to its own 
when being considered as a single indivisible memory consumer yet still 
want per-process oom kill selection.

If you do not prefer that overloading, I can break the two out from one 
another such that one tunable defines cgroup vs process, and another 
defines subtree usage being considered or not.  That's a perfectly fine 
suggestion and I have no problem implementing it.  The only reason I did 
not was because I do not know of any user that would want process && 
subtree and that would reduce the number of files for mem cgroup by one.

If you'd like me to separate these out by adding another tunable, please 
let me know.  We will already have another tunable later, but is not 
required for this to be merged as the cover letter states, to allow the 
user to adjust the calculation for a subtree such that it may protect 
important cgroups that are allowed to use more memory than others.

> > It would be helpful if you would also review the rest of the patchset.
> 
> I think, that we should focus on interface semantics right now.
> If we can't agree on how the things should work, it makes no sense
> to discuss the implementation.
> 

Yes, I have urged that we consider the interface in both the 
memory.oom_group discussion as well as the discussion here, which is why 
this patchset removes the mount option, does not lock down the entire 
hierarchy into a single policy, and is extensible to be generally useful 
outside of very special usecases.

^ permalink raw reply

* Re: [PATCH] dt-bindings: remove 'interrupt-parent' from bindings
From: Vladimir Zapolskiy @ 2018-07-23 23:20 UTC (permalink / raw)
  To: Rob Herring, linux-kernel, devicetree; +Cc: Mark Rutland
In-Reply-To: <20180723221317.16394-1-robh@kernel.org>

Hi Rob,

On 07/24/2018 01:13 AM, Rob Herring wrote:
> 'interrupt-parent' is often documented as part of define bindings, but
> it is really outside the scope of a device binding. It's never required
> in a given node as it is often inherited from a parent node. Or it can
> be implicit if a parent node is an 'interrupt-controller' node. So
> remove it from all the binding files.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---

[snip]

> diff --git a/Documentation/devicetree/bindings/interrupt-controller/nxp,lpc3220-mic.txt b/Documentation/devicetree/bindings/interrupt-controller/nxp,lpc3220-mic.txt
> index 38211f344dc8..0bfb3ba55f4c 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/nxp,lpc3220-mic.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/nxp,lpc3220-mic.txt
> @@ -14,8 +14,6 @@ Required properties:
>    Reset value is IRQ_TYPE_LEVEL_LOW.
>  
>  Optional properties:
> -- interrupt-parent: empty for MIC interrupt controller, link to parent
> -  MIC interrupt controller for SIC1 and SIC2
>  - interrupts: empty for MIC interrupt controller, cascaded MIC
>    hardware interrupts for SIC1 and SIC2
>  

I would rather ask you to keep the property here, it is optional in sense that
its presence distinguishes MIC and SIC types of interrupt controllers.

--
Best wishes,
Vladimir

^ permalink raw reply

* Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'
From: Eric Sunshine @ 2018-07-23 23:20 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Git List, Johannes Schindelin,
	Nguyễn Thái Ngọc Duy,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <CAGZ79kZ0OMrU_O5_rPdG7bNHRWQxVV1RguEg3g-pOa+kkgPCaA@mail.gmail.com>

On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > The actual diffs output by range-diff respect diff_option.file, which
> > range-diff passes down the call-chain, thus are destination-agnostic.
> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix
> > this by making output_pair_header() respect diff_option.file, as well.
> >
> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
>
> Depending how much the range diff series has progressed
> already, it might make sense to squash it there?

It could be done that way, though it would be nice for Dscho's
range-diff series to land without another re-roll, and it looks like
I'll probably be re-rolling this series, anyhow, to move
show_interdiff() to diff-lib.c and to fix its signature. Junio could
manage it as a "squash", but that's probably more work for him than
for me to retain it in this series. *And*, this change _is_ required
for this series, whereas it doesn't really matter for Dscho's series,
even though it's an obvious "fix".

Anyhow, whichever way Junio and Dscho would like to play it is fine with me.

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.