All of lore.kernel.org
 help / color / mirror / Atom feed
* 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
  Cc: linux-hwmon, devicetree, linux-aspeed, linux-doc, Vernon Mauery,
	openbmc, linux-kernel, James Feist, Jason M Biils,
	linux-arm-kernel
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

* [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: linux-arm-kernel
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

* [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: linux-aspeed
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: [PATCH v4 16/21] range-diff --dual-color: fix bogus white-space warning
From: Stefan Beller @ 2018-07-23 22:20 UTC (permalink / raw)
  To: gitgitgadget; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <f4252f2b2198cf13d5b0a21c54098e2a1d8158dd.1532210683.git.gitgitgadget@gmail.com>

On Sat, Jul 21, 2018 at 3:05 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When displaying a diff of diffs, it is possible that there is an outer
> `+` before a context line. That happens when the context changed between
> old and new commit. When that context line starts with a tab (after the
> space that marks it as context line), our diff machinery spits out a
> white-space error (space before tab), but in this case, that is
> incorrect.
>
> Fix this by adding a specific whitespace flag that simply ignores the
> first space in the output.

That sounds like a simple (not easy) solution, which sounds acceptable
to me here.

I guess you dropped all ideas that I originally proposed for the cleanup
regarding ws. that is fine, I can roll the cleanup on top of your patches
here.

> Of course, this flag is *really* specific to the "diff of diffs" use
> case. The original idea was to simply skip the space from the output,
> but that workaround was rejected by the Git maintainer as causing
> headaches.

By that you mean
https://public-inbox.org/git/xmqqr2kb9jk2.fsf@gitster-ct.c.googlers.com/
?

> Note: as the original code did not leave any space in the bit mask
> before the WSEH_* bits, the diff of this commit looks unnecessarily
> involved: the diff is dominated by making room for one more bit to be
> used by the whitespace rules.

It took me some minutes, but I am reasonably convinced this patch
is correct (and doesn't collide with other series in flight, sb/diff-color-more
adds another flag to move detection in another bit field at (1<<23))

Thanks for writing this patch instead of the other, though I'll leave
it to Junio to weigh in if this approach is the best design.

Stefan

>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  cache.h |  3 ++-
>  diff.c  | 15 ++++++++-------
>  diff.h  |  6 +++---
>  ws.c    | 11 ++++++++++-
>  4 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 8b447652a..8abfbeb73 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1681,11 +1681,12 @@ void shift_tree_by(const struct object_id *, const struct object_id *, struct ob
>  #define WS_CR_AT_EOL           01000
>  #define WS_BLANK_AT_EOF        02000
>  #define WS_TAB_IN_INDENT       04000
> +#define WS_IGNORE_FIRST_SPACE 010000
>  #define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)
>  #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)
>  #define WS_TAB_WIDTH_MASK        077
>  /* All WS_* -- when extended, adapt diff.c emit_symbol */
> -#define WS_RULE_MASK           07777
> +#define WS_RULE_MASK           017777
>  extern unsigned whitespace_rule_cfg;
>  extern unsigned whitespace_rule(const char *);
>  extern unsigned parse_whitespace_rule(const char *);
> diff --git a/diff.c b/diff.c
> index e163bc8a3..03ed235c7 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -650,14 +650,14 @@ enum diff_symbol {
>  };
>  /*
>   * Flags for content lines:
> - * 0..12 are whitespace rules
> - * 13-15 are WSEH_NEW | WSEH_OLD | WSEH_CONTEXT
> - * 16 is marking if the line is blank at EOF
> + * 0..14 are whitespace rules
> + * 14-16 are WSEH_NEW | WSEH_OLD | WSEH_CONTEXT
> + * 17 is marking if the line is blank at EOF
>   */
> -#define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF     (1<<16)
> -#define DIFF_SYMBOL_MOVED_LINE                 (1<<17)
> -#define DIFF_SYMBOL_MOVED_LINE_ALT             (1<<18)
> -#define DIFF_SYMBOL_MOVED_LINE_UNINTERESTING   (1<<19)
> +#define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF     (1<<17)
> +#define DIFF_SYMBOL_MOVED_LINE                 (1<<18)
> +#define DIFF_SYMBOL_MOVED_LINE_ALT             (1<<19)
> +#define DIFF_SYMBOL_MOVED_LINE_UNINTERESTING   (1<<20)
>  #define DIFF_SYMBOL_CONTENT_WS_MASK (WSEH_NEW | WSEH_OLD | WSEH_CONTEXT | WS_RULE_MASK)
>
>  /*
> @@ -1094,6 +1094,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
>                                 set = diff_get_color_opt(o, DIFF_FRAGINFO);
>                         else if (c != '+')
>                                 set = diff_get_color_opt(o, DIFF_CONTEXT);
> +                       flags |= WS_IGNORE_FIRST_SPACE;
>                 }
>                 emit_line_ws_markup(o, set, reset, line, len, set_sign, '+',
>                                     flags & DIFF_SYMBOL_CONTENT_WS_MASK,
> diff --git a/diff.h b/diff.h
> index 79beb6eea..892416a14 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -160,9 +160,9 @@ struct diff_options {
>         int abbrev;
>         int ita_invisible_in_index;
>  /* white-space error highlighting */
> -#define WSEH_NEW (1<<12)
> -#define WSEH_CONTEXT (1<<13)
> -#define WSEH_OLD (1<<14)
> +#define WSEH_NEW (1<<13)
> +#define WSEH_CONTEXT (1<<14)
> +#define WSEH_OLD (1<<15)
>         unsigned ws_error_highlight;
>         const char *prefix;
>         int prefix_length;
> diff --git a/ws.c b/ws.c
> index a07caedd5..e02365a6a 100644
> --- a/ws.c
> +++ b/ws.c
> @@ -20,6 +20,7 @@ static struct whitespace_rule {
>         { "blank-at-eol", WS_BLANK_AT_EOL, 0 },
>         { "blank-at-eof", WS_BLANK_AT_EOF, 0 },
>         { "tab-in-indent", WS_TAB_IN_INDENT, 0, 1 },
> +       { "ignore-first-space", WS_IGNORE_FIRST_SPACE, 0, 1 },
>  };
>
>  unsigned parse_whitespace_rule(const char *string)
> @@ -177,8 +178,16 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
>         if (trailing_whitespace == -1)
>                 trailing_whitespace = len;
>
> +       if ((ws_rule & WS_IGNORE_FIRST_SPACE) && len && line[0] == ' ') {
> +               if (stream)
> +                       fwrite(line, 1, 1, stream);
> +               written++;
> +               if (!trailing_whitespace)
> +                       trailing_whitespace++;
> +       }
> +
>         /* Check indentation */
> -       for (i = 0; i < trailing_whitespace; i++) {
> +       for (i = written; i < trailing_whitespace; i++) {
>                 if (line[i] == ' ')
>                         continue;
>                 if (line[i] != '\t')
> --
> gitgitgadget
>

^ permalink raw reply

* Re: [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
From: Rodrigo Vivi @ 2018-07-23 22:19 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, dri-devel
In-Reply-To: <20180723212735.23893-2-matthew.s.atwood@intel.com>

On Mon, Jul 23, 2018 at 02:27:35PM -0700, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> According to DP spec (2.9.3.1 of DP 1.4) if
> EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> 02200h through 0220Fh shall contain the DPRX's true capability. These
> values will match 00000h through 0000Fh, except for DPCD_REV,
> MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> 
> Read from DPCD once for all 3 values as this is an expensive operation.
> Spec mentions that all of address space 02200h through 0220Fh should
> contain the right information however currently only 3 values can
> differ.
> 
> There is no address space in the intel_dp->dpcd struct for addresses
> 02200h through 0220Fh, and since so much of the data is a identical,
> simply overwrite the values stored in 00000h through 0000Fh with the
> values that can be overwritten from addresses 02200h through 0220Fh.
> 
> This patch helps with backward compatibility for devices pre DP1.3.
> 
> v2: read only dpcd values which can be affected, remove incorrect check,
> split into drm include changes into separate patch, commit message,
> verbose debugging statements during overwrite.
> v3: white space fixes
> v4: make path dependent on DPCD revision > 1.2
> v5: split into function, removed DPCD rev check
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 54 +++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dde92e4af5d3..ed16b93bfe40 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3731,6 +3731,58 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	/*
> +	 * Prior to DP1.3 the bit represented by
> +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> +	 * the true capability of the panel. The only way to check is to
> +	 * then compare 0000h and 2200h.
> +	 */
> +	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> +	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> +		uint8_t dpcd_ext[6];
> +
> +		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");
> +
> +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +				     &dpcd_ext, sizeof(dpcd_ext)) < 0)

DRM_ERROR

> +			return;
> +
> +		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV])

DRM_DEBUG_KMS
> +			return;

The silent skip will be hard to debug and we will never know where it went
wrong.

> +
> +		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
> +			   sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DPCD_REV],
> +				      dpcd_ext[DP_DPCD_REV]);
> +			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
> +			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
> +		}
> +		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_MAX_LINK_RATE],
> +				      dpcd_ext[DP_MAX_LINK_RATE]);
> +			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));
> +		}
> +		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
> +			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
> +			       sizeof(u8));
> +		}
> +	}
> +}
> +
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3738,6 +3790,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  			     sizeof(intel_dp->dpcd)) < 0)
>  		return false; /* aux transfer failed */
>  
> +	intel_dp_extended_receiver_capabilities(intel_dp);
> +
>  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
>  
>  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: In-Band Firmware Update
From: Vernon Mauery @ 2018-07-23 22:18 UTC (permalink / raw)
  To: Patrick Venture; +Cc: OpenBMC Maillist
In-Reply-To: <CAO=notyCMVrJ9Ya+PaukB1k1M7KE07ZHnD5prt4CsA0gdfe+qw@mail.gmail.com>

On 23-Jul-2018 11:13 AM, Patrick Venture wrote:
>I've started to implement the host-tool outside of google3, and
>started splitting up the OEM handler that corresponds with it.
>However, firstly, I've submitted the design for the protocol for
>review, please let me know if you're interested and I'll add you to
>the review.  IIRC, there was at least one interested party outside of
>us.

I am interested in coming up with a common (OpenBMC OEM level) set of 
Firmware NetFn commands that will allow all users of OpenBMC to be able 
to use common, open-source utilities to do firmware updates. If they are 
IPMI commands, this would include in-band (with KCS/BT for 
command/control and USB for transfer) or out-of-band (over RMCP+).

Rather than use the OEM NetFn, for firmware updates, we should be using 
the Firmware NetFn.  The entire Firmware NetFn is considered to be OEM 
per the IPMI spec. I would propose that we simply provide a common 
implementation for OpenBMC (as a provider library, of course, so it 
could be replaced if a downstream OEM doesn't want it).

--Vernon

^ permalink raw reply

* Re: [PATCH] mtd: spi-nor: clear Extended Address Reg on switch to 3-byte addressing.
From: Brian Norris @ 2018-07-23 22:17 UTC (permalink / raw)
  To: NeilBrown
  Cc: Marek Vasut, Cyrille Pitchen, David Woodhouse, Boris Brezillon,
	Richard Weinberger, linux-mtd, Linux Kernel, Hou Zhiqiang
In-Reply-To: <87tvoppqwa.fsf@notabene.neil.brown.name>

Hi Neil,

On Mon, Jul 23, 2018 at 2:45 PM, NeilBrown <neil@brown.name> wrote:
> On Mon, Jul 23 2018, Brian Norris wrote:
>> On Mon, Apr 9, 2018 at 6:05 PM, NeilBrown <neil@brown.name> wrote:
>>> On Mon, Apr 09 2018, Marek Vasut wrote:
>>>> On 04/08/2018 11:56 PM, NeilBrown wrote:
>>>>> were added to Linux.  They appear to be designed to address a very
>>>>> similar situation to mine.  Unfortunately they aren't complete as the
>>>>> code to disable 4-byte addressing doesn't follow documented requirements
>>>>> (at least for winbond) and doesn't work as intended (at least in one
>>>>> case - mine). This code should either be fixed (e.g. with my patch), or removed.
>>
>> I would (and already did) vote for removal. The shutdown() hook just
>> papers over bugs and leads people to think that it is a good solution.
>> There's a reason we rejected such patches repeatedly in the past. This
>> one slipped through.
>
> Hi Brian,
>  thanks for your thoughts.
>  Could you just clarify what you see as the end-game.
>  Do you have an alternate approach which can provide reliability for the
>  various hardware which currently seems to need these patches?
>  Or do you propose that people with this hardware should suffer
>  a measurably lower level of reliability than they currently enjoy?

I'd suggest following the original thread, which I resurrected:

[PATCHv3 2/2] mtd: m25p80: restore the status of SPI flash when exiting
https://lkml.org/lkml/2018/7/23/1207
https://patchwork.ozlabs.org/patch/845022/

I suppose I could CC you on future replies...

My current summary: I'd prefer the hack be much more narrowly applied,
with a big warning, if we apply it at all. But if we don't merge
something to narrow the use of the hack, then yes, I'd prefer a
degraded experience for crappy products over today's status quo.

Brian

^ permalink raw reply

* Re: [Fuego] [PATCH] Functional.glib: support for multic versions glib
From: Tim.Bird @ 2018-07-23 22:16 UTC (permalink / raw)
  To: lixm.fnst, fuego
In-Reply-To: <62079D2F712F7747B0BDCC6821B8DF0D013D26C3@G08CNEXMBPEKD02.g08.fujitsu.local>



> -----Original Message-----
> From: Li, Xiaoming 
> Hi Tim
> 
> Sorry for my late reply.
> 
> Other versions of tarball will be submitted by git merge request, as my mail
> accout does not support a mail of 5M+ size.
> 
> I have investigated on glib2.46 and glib2.48, and make it work.

OK. Sounds good.  I will look for a pull request shortly.
(Please note I'll be out of the office for the rest of this week, so
I won't respond until Monday at the earliest.)


> The reasons of why I keep 3 versions of glib are below:
> 1). Different board  may have different versions of glib, and I think it looks
> good to make the board and fuego have the same version of glib.(maybe
> dont need so, i have not thoutht much about it).
> 
> 2). Some fuego user may dont want to upgrade their board's glib to a higher
> version.

I don't think Fuego should impose a version of glib to test, so this sounds
OK.  We should be able to test whatever version of glib is on the target
board.

> 3). As you have seen, different versions of glib test suite need different *.c
> files copied to board,  and i have investigated to make 2.46 and 2.48 work
> well. It will cost someone of 2) a lot of time of fix this problem if we only
> keep a latest version.

OK.  I think it's worth checking to see if the tests are backwards-compatible.
I'm not sure how the glib tests work.  Do they compile programs against the
library on the target?  In this case, if the API has changed, then there could
be issues.

> 
> And i agree with you on the rest review points.

OK Thanks.
 -- Tim

> -----Original Message-----
> From: Tim.Bird@sony.com [mailto:Tim.Bird@sony.com]
> Sent: Saturday, July 21, 2018 7:53 AM
> To: Li, Xiaoming/李 霄鸣; fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] Functional.glib: support for multic versions glib
> 
> See comments inline below.
> 
> > -----Original Message-----
> > From: Li Xiaoming
> 
> Some more details in the commit message would be nice.
> 
> >
> > Signed-off-by: Li Xiaoming <lixm.fnst@cn.fujitsu.com>
> > ---
> >  engine/tests/Functional.glib/fuego_test.sh | 59
> > +++++++++++++++++++++++++-----
> >  engine/tests/Functional.glib/spec.json     | 13 ++++++-
> >  2 files changed, 62 insertions(+), 10 deletions(-)
> >
> > diff --git a/engine/tests/Functional.glib/fuego_test.sh
> > b/engine/tests/Functional.glib/fuego_test.sh
> > index b69c05c..dbcc468 100755
> > --- a/engine/tests/Functional.glib/fuego_test.sh
> > +++ b/engine/tests/Functional.glib/fuego_test.sh
> > @@ -1,22 +1,55 @@
> > -tarball=glib-2.22.1.tar.bz2
> > +tarball=$FUNCTIONAL_GLIB_PACKAGE
> Interesting use of a dynamic variable here.
> 
> The other tarball packages are not available in the Fuego repository.
> Will you be adding a patch to provide them?
> 
> I think I'd like to eliminate the variable PACKAGE, and just use VERSION, so
> this would become:
> tarball=glib-$VERSION.tar.bz2
> 
> Is there a reason not to move to the latest version of the tarball, and just use
> that?  Are there differences in the tests between 2.22 and 2.48, such that the
> tests are not backwards compatible?
> 
> >
> >  function test_build {
> > -    patch -p0 -N -s < $TEST_HOME/glib-strfuncs.patch
> > +    if [[ $tarball =~ "2.22" ]]
> > +    then
> > +	patch -p0 -N -s < $TEST_HOME/glib-strfuncs.patch
> > +    fi
> > +
> Looks good.  Nice to not need a patch anymore.  If we just migrate to a later
> version of the library, we can get rid of the patch, and this conditional.
> 
> >      # Don't run tests on the build host
> >      find . -name 'Makefile*' | xargs sed -i '/@test -z
> > "${TEST_PROGS}" || ${GTESTER} --verbose ${TEST_PROGS}/d'
> >      echo -e "glib_cv_stack_grows=no \nglib_cv_uscore=no
> > \nac_cv_func_posix_getpwuid_r=no
> \nac_cv_func_nonposix_getpwuid_r=no
> > \nac_cv_func_posix_getgrgid_r=no" > config.cross
> >      # get updated config.sub and config.guess files, so configure
> >      # doesn't reject new toolchains
> >      cp /usr/share/misc/config.{sub,guess} .
> > -    ./configure --prefix=`pwd`/build --cache-file=config.cross --host=$HOST
> --
> > target=$HOST --build=`uname -m`-linux-gnu CC="$CC" AR="$AR"
> > RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP" CXXCPP="$CXXCPP"
> > CXXFLAGS="$CXXFLAGS"
> > -    make 2>&1
> > -    cd tests && make test; cd -
> > -    cd glib/tests && make test; cd -
> > +
> > +    if [[ $tarball =~ "2.22" ]]
> > +    then
> > +        ./configure --prefix=`pwd`/build --cache-file=config.cross
> > + --host=$HOST
> > --target=$HOST --build=`uname -m`-linux-gnu CC="$CC" AR="$AR"
> > RANLIB="$RANLIB" CXX="$CXX" CPP="$CPP" CXXCPP="$CXXCPP"
> > CXXFLAGS="$CXXFLAGS"
> > +        make 2>&1
> > +	cd tests && make test; cd -
> > +        cd glib/tests && make test; cd -
> > +    fi
> > +
> > +    if [[ $tarball =~ "2.46" ]] || [[ $tarball =~ "2.48" ]]
> > +    then
> > +	./configure --prefix=`pwd`/build  --enable-always-build-tests --
> > cache-file=config.cross --host=$HOST --target=$HOST --build=`uname
> > -m`- linux-gnu CC="$CC" AR="$AR" RANLIB="$RANLIB" CXX="$CXX"
> CPP="$CPP"
> > CXXCPP="$CXXCPP" CXXFLAGS="$CXXFLAGS"
> > +        make 2>&1
> > +        cd tests && make; cd -
> > +        cd glib/tests && make; cd -
> > +    fi
> >  }
> >
> This is OK to make these conditional like this, but why have all three?
> 
> >  function test_deploy {
> > -	# Makefile.am is used by 'contexts' test. XXX - why?
> > -	put gio/tests/Makefile.am `find . -name .libs | xargs -IDIRS find DIRS -
> > not -type d -not -name '*.so*' -not -name '*.la*' -perm /u+x`
> > $BOARD_TESTDIR/fuego.$TESTDIR/
> > +    put `find . -name .libs | xargs -IDIRS find DIRS -not -type d
> > + -not -name
> > '*.so*' -not -name '*.la*' -perm /u+x`  $BOARD_TESTDIR/fuego.$TESTDIR/
> > +
> > +    if [[ $tarball =~ "2.22" ]]
> > +    then
> > +	#glib2.22 contexts nedd gio/tests/Makefile.am
> 'nedd' should be 'needs'?
> 
> > +	put gio/tests/Makefile.am $BOARD_TESTDIR/fuego.$TESTDIR/
> > +    fi
> > +
> > +    if [[ $tarball =~ "2.46" ]] || [[ $tarball =~ "2.48" ]]
> > +    then
> > +        #glib(2.46 2.48) contexts need gio/tests/contexts.c
> > +        put gio/tests/contexts.c $BOARD_TESTDIR/fuego.$TESTDIR/
> > +
> > +        #glib(2.46 2.48) keyfile need
> > glib/tests/keyfiletest.ini,glib/tests/keyfile.c,glib/tests/pages.ini
> > +        put glib/tests/keyfiletest.ini glib/tests/keyfile.c
> > + glib/tests/pages.ini
> > $BOARD_TESTDIR/fuego.$TESTDIR/
> > +
> > +        #glib(2.46 2.48) g-icon need gio/tests/g-icon.c
> > +        put gio/tests/g-icon.c $BOARD_TESTDIR/fuego.$TESTDIR/
> > +    fi
> >  }
> >
> >  # Excluded tests
> > @@ -40,7 +73,15 @@ function test_deploy {  # filter-streams
> >
> >  function test_run {
> > -	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./gtester  ./objects
> > ./readwrite ./g-file ./fileutils ./data-output-stream ./signal3
> > ./properties ./rand ./memory-input-stream ./sleepy-stream  ./g-icon
> > ./array-test ./strfuncs ./testgdateparser ./srvtarget ./threadtests
> > ./keyfile ./string ./hostutils ./memory-output-stream ./objects2
> > ./closures ./scannerapi ./testgdate ./data-input-stream ./signal2
> > ./signal1 ./testgobject ./g-file-info ./simple-async-result
> > ./buffered-input-stream ./contexts ./printf
> > ./properties2 ./unix-streams "
> > +    if [[ $tarball =~ "2.22" ]]
> > +    then
> > +	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./gtester  ./objects
> > ./readwrite ./g-file ./fileutils ./data-output-stream ./signal3
> > ./properties ./rand ./memory-input-stream ./sleepy-stream  ./g-icon
> > ./array-test ./strfuncs ./testgdateparser ./srvtarget ./threadtests
> > ./keyfile ./string ./hostutils ./memory-output-stream ./objects2
> > ./closures ./scannerapi ./testgdate ./data-input-stream ./signal2
> > ./signal1 ./testgobject ./g-file-info ./simple-async-result
> > ./buffered-input-stream ./contexts ./printf
> > ./properties2 ./unix-streams "
> > +    fi
> > +
> > +    if [[ $tarball =~ "2.46" ]] || [[ $tarball =~ "2.48" ]]
> > +    then
> > +	report "cd $BOARD_TESTDIR/fuego.$TESTDIR; ./gtester ./readwrite
> > ./g-file  ./data-output-stream  ./properties ./rand
> > ./memory-input-stream ./sleepy-stream  ./array-test ./strfuncs  ./srvtarget
> ./string ./hostutils
> > ./memory-output-stream   ./scannerapi ./data-input-stream  ./g-file-info
> > ./simple-async-result ./buffered-input-stream   ./test-printf  ./unix-
> streams
> > ./contexts ./keyfile ./g-icon "
> > +    fi
> >  }
> >
> >  function test_processing {
> > diff --git a/engine/tests/Functional.glib/spec.json
> > b/engine/tests/Functional.glib/spec.json
> > index 8bc2da5..5b06923 100644
> > --- a/engine/tests/Functional.glib/spec.json
> > +++ b/engine/tests/Functional.glib/spec.json
> > @@ -1,6 +1,17 @@
> >  {
> >      "testName": "Functional.glib",
> >      "specs": {
> > -        "default": {}
> > +	 "default": {
> > +	      "VERSION":"2.22.1",
> > +	      "PACKAGE":"glib-2.22.1.tar.bz2"
> > +	 },
> > +	 "2-46": {
> > +	      "VERSION":"2.46.2",
> > +	      "PACKAGE":"glib-2.46.2.tar.bz2"
> > +	 },
> > +         "2-48":{
> > +              "VERSION":"2.48.2",
> > +              "PACKAGE":"glib-2.48.2.tar.bz2"
> > +	}
> >      }
> >  }
> > --
> > 2.7.4
> 
> 
> The biggest question here is why keep the older tests around?  Otherwise, it
> looks good.
> I haven't applied it yet, though, pending some more discussion.
>  -- Tim
> 
> 
> 
> 


^ permalink raw reply

* [PATCH v2 7/7] OF: Don't set default coherent DMA mask
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Now that we can track upstream DMA constraints properly with
bus_dma_mask instead of trying (and failing) to maintain it in
coherent_dma_mask, it doesn't make much sense for the firmware code to
be touching the latter at all. It's merely papering over bugs wherein a
driver has failed to call dma_set_coherent_mask() *and* the bus code has
not initialised any default value.

We don't really want to encourage more drivers coercing dma_mask so
we'll continue to fix that up if necessary, but add a warning to help
flush out any such buggy bus code that remains.

CC: Rob Herring <robh+dt@kernel.org>
CC: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/of/device.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 0d39633e8545..5957cd4fa262 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -127,20 +127,20 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
 	}
 
 	/*
-	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
-	 * setup the correct supported mask.
+	 * If @dev is expected to be DMA-capable then the bus code that created
+	 * it should have initialised its dma_mask pointer by this point. For
+	 * now, we'll continue the legacy behaviour of coercing it to the
+	 * coherent mask if not, but we'll no longer do so quietly.
 	 */
-	if (!dev->coherent_dma_mask)
-		dev->coherent_dma_mask = DMA_BIT_MASK(32);
-	/*
-	 * Set it to coherent_dma_mask by default if the architecture
-	 * code has not set it.
-	 */
-	if (!dev->dma_mask)
+	if (!dev->dma_mask) {
+		dev_warn(dev, "DMA mask not set\n");
 		dev->dma_mask = &dev->coherent_dma_mask;
+	}
 
-	if (!size)
+	if (!size && dev->coherent_dma_mask)
 		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	else if (!size)
+		size = 1ULL << 32;
 
 	dev->dma_pfn_offset = offset;
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 7/7] OF: Don't set default coherent DMA mask
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Now that we can track upstream DMA constraints properly with
bus_dma_mask instead of trying (and failing) to maintain it in
coherent_dma_mask, it doesn't make much sense for the firmware code to
be touching the latter at all. It's merely papering over bugs wherein a
driver has failed to call dma_set_coherent_mask() *and* the bus code has
not initialised any default value.

We don't really want to encourage more drivers coercing dma_mask so
we'll continue to fix that up if necessary, but add a warning to help
flush out any such buggy bus code that remains.

CC: Rob Herring <robh+dt@kernel.org>
CC: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/of/device.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 0d39633e8545..5957cd4fa262 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -127,20 +127,20 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
 	}
 
 	/*
-	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
-	 * setup the correct supported mask.
+	 * If @dev is expected to be DMA-capable then the bus code that created
+	 * it should have initialised its dma_mask pointer by this point. For
+	 * now, we'll continue the legacy behaviour of coercing it to the
+	 * coherent mask if not, but we'll no longer do so quietly.
 	 */
-	if (!dev->coherent_dma_mask)
-		dev->coherent_dma_mask = DMA_BIT_MASK(32);
-	/*
-	 * Set it to coherent_dma_mask by default if the architecture
-	 * code has not set it.
-	 */
-	if (!dev->dma_mask)
+	if (!dev->dma_mask) {
+		dev_warn(dev, "DMA mask not set\n");
 		dev->dma_mask = &dev->coherent_dma_mask;
+	}
 
-	if (!size)
+	if (!size && dev->coherent_dma_mask)
 		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	else if (!size)
+		size = 1ULL << 32;
 
 	dev->dma_pfn_offset = offset;
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 6/7] ACPI/IORT: Don't set default coherent DMA mask
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Now that we can track upstream DMA constraints properly with
bus_dma_mask instead of trying (and failing) to maintain it in
coherent_dma_mask, it doesn't make much sense for the firmware code to
be touching the latter at all. It's merely papering over bugs wherein a
driver has failed to call dma_set_coherent_mask() *and* the bus code has
not initialised any default value.

We don't really want to encourage more drivers coercing dma_mask so
we'll continue to fix that up if necessary, but add a warning to help
flush out any such buggy bus code that remains.

CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Hanjun Guo <hanjun.guo@linaro.org>
CC: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index bc51cff5505e..08f26db2da7e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -978,20 +978,20 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 	int ret, msb;
 
 	/*
-	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
-	 * setup the correct supported mask.
+	 * If @dev is expected to be DMA-capable then the bus code that created
+	 * it should have initialised its dma_mask pointer by this point. For
+	 * now, we'll continue the legacy behaviour of coercing it to the
+	 * coherent mask if not, but we'll no longer do so quietly.
 	 */
-	if (!dev->coherent_dma_mask)
-		dev->coherent_dma_mask = DMA_BIT_MASK(32);
-
-	/*
-	 * Set it to coherent_dma_mask by default if the architecture
-	 * code has not set it.
-	 */
-	if (!dev->dma_mask)
+	if (!dev->dma_mask) {
+		dev_warn(dev, "DMA mask not set\n");
 		dev->dma_mask = &dev->coherent_dma_mask;
+	}
 
-	size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	if (dev->coherent_dma_mask)
+		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	else
+		size = 1ULL << 32;
 
 	if (dev_is_pci(dev)) {
 		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 6/7] ACPI/IORT: Don't set default coherent DMA mask
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Now that we can track upstream DMA constraints properly with
bus_dma_mask instead of trying (and failing) to maintain it in
coherent_dma_mask, it doesn't make much sense for the firmware code to
be touching the latter at all. It's merely papering over bugs wherein a
driver has failed to call dma_set_coherent_mask() *and* the bus code has
not initialised any default value.

We don't really want to encourage more drivers coercing dma_mask so
we'll continue to fix that up if necessary, but add a warning to help
flush out any such buggy bus code that remains.

CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Hanjun Guo <hanjun.guo@linaro.org>
CC: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index bc51cff5505e..08f26db2da7e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -978,20 +978,20 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 	int ret, msb;
 
 	/*
-	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
-	 * setup the correct supported mask.
+	 * If @dev is expected to be DMA-capable then the bus code that created
+	 * it should have initialised its dma_mask pointer by this point. For
+	 * now, we'll continue the legacy behaviour of coercing it to the
+	 * coherent mask if not, but we'll no longer do so quietly.
 	 */
-	if (!dev->coherent_dma_mask)
-		dev->coherent_dma_mask = DMA_BIT_MASK(32);
-
-	/*
-	 * Set it to coherent_dma_mask by default if the architecture
-	 * code has not set it.
-	 */
-	if (!dev->dma_mask)
+	if (!dev->dma_mask) {
+		dev_warn(dev, "DMA mask not set\n");
 		dev->dma_mask = &dev->coherent_dma_mask;
+	}
 
-	size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	if (dev->coherent_dma_mask)
+		size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
+	else
+		size = 1ULL << 32;
 
 	if (dev_is_pci(dev)) {
 		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 5/7] iommu/dma: Respect bus DMA limit for IOVAs
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Take the new bus limit into account (when present) for IOVA allocations,
to accommodate those SoCs which integrate off-the-shelf IP blocks with
narrower interconnects such that the link between a device output and an
IOMMU input can truncate DMA addresses to even fewer bits than the
native size of either block's interface would imply.

Eventually it might make sense for the DMA core to apply this constraint
up-front in dma_set_mask() and friends, but for now this seems like the
least risky approach.

CC: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---

Bonus question: Now that we're collecing DMA API code in kernel/dma/
do we want to reevaluate dma-iommu? On the one hand it's the bulk of a
dma_ops implementation so should perhaps move, but on the other it's
entirely IOMMU-specific code so should perhaps stay where it is... :/

 drivers/iommu/dma-iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ddcbbdb5d658..511ff9a1d6d9 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -367,6 +367,9 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
 	if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
 		iova_len = roundup_pow_of_two(iova_len);
 
+	if (dev->bus_dma_mask)
+		dma_limit &= dev->bus_dma_mask;
+
 	if (domain->geometry.force_aperture)
 		dma_limit = min(dma_limit, domain->geometry.aperture_end);
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 5/7] iommu/dma: Respect bus DMA limit for IOVAs
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Take the new bus limit into account (when present) for IOVA allocations,
to accommodate those SoCs which integrate off-the-shelf IP blocks with
narrower interconnects such that the link between a device output and an
IOMMU input can truncate DMA addresses to even fewer bits than the
native size of either block's interface would imply.

Eventually it might make sense for the DMA core to apply this constraint
up-front in dma_set_mask() and friends, but for now this seems like the
least risky approach.

CC: Joerg Roedel <joro@8bytes.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---

Bonus question: Now that we're collecing DMA API code in kernel/dma/
do we want to reevaluate dma-iommu? On the one hand it's the bulk of a
dma_ops implementation so should perhaps move, but on the other it's
entirely IOMMU-specific code so should perhaps stay where it is... :/

 drivers/iommu/dma-iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ddcbbdb5d658..511ff9a1d6d9 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -367,6 +367,9 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain,
 	if (iova_len < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
 		iova_len = roundup_pow_of_two(iova_len);
 
+	if (dev->bus_dma_mask)
+		dma_limit &= dev->bus_dma_mask;
+
 	if (domain->geometry.force_aperture)
 		dma_limit = min(dma_limit, domain->geometry.aperture_end);
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 4/7] of/device: Set bus DMA mask as appropriate
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

When an explicit DMA limit is described by firmware, we need to remember
it regardless of how drivers might subsequently update their devices'
masks. The new bus_dma_mask field does that.

CC: Rob Herring <robh+dt@kernel.org>
CC: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/of/device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 33d85511d790..0d39633e8545 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -149,6 +149,7 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
 	 * set by the driver.
 	 */
 	mask = DMA_BIT_MASK(ilog2(dma_addr + size - 1) + 1);
+	dev->bus_dma_mask = mask;
 	dev->coherent_dma_mask &= mask;
 	*dev->dma_mask &= mask;
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 4/7] of/device: Set bus DMA mask as appropriate
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

When an explicit DMA limit is described by firmware, we need to remember
it regardless of how drivers might subsequently update their devices'
masks. The new bus_dma_mask field does that.

CC: Rob Herring <robh+dt@kernel.org>
CC: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/of/device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 33d85511d790..0d39633e8545 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -149,6 +149,7 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
 	 * set by the driver.
 	 */
 	mask = DMA_BIT_MASK(ilog2(dma_addr + size - 1) + 1);
+	dev->bus_dma_mask = mask;
 	dev->coherent_dma_mask &= mask;
 	*dev->dma_mask &= mask;
 
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 3/7] ACPI/IORT: Set bus DMA mask as appropriate
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

When an explicit DMA limit is described by firmware, we need to remember
it regardless of how drivers might subsequently update their devices'
masks. The new bus_dma_mask field does that.

CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Hanjun Guo <hanjun.guo@linaro.org>
CC: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4a66896e2aa3..bc51cff5505e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1014,6 +1014,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 		 * Limit coherent and dma mask based on size
 		 * retrieved from firmware.
 		 */
+		dev->bus_dma_mask = mask;
 		dev->coherent_dma_mask = mask;
 		*dev->dma_mask = mask;
 	}
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 3/7] ACPI/IORT: Set bus DMA mask as appropriate
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

When an explicit DMA limit is described by firmware, we need to remember
it regardless of how drivers might subsequently update their devices'
masks. The new bus_dma_mask field does that.

CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
CC: Hanjun Guo <hanjun.guo@linaro.org>
CC: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4a66896e2aa3..bc51cff5505e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1014,6 +1014,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 		 * Limit coherent and dma mask based on size
 		 * retrieved from firmware.
 		 */
+		dev->bus_dma_mask = mask;
 		dev->coherent_dma_mask = mask;
 		*dev->dma_mask = mask;
 	}
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 2/7] dma-mapping: Generalise dma_32bit_limit flag
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Whilst the notion of an upstream DMA restriction is most commonly seen
in PCI host bridges saddled with a 32-bit native interface, a more
general version of the same issue can exist on complex SoCs where a bus
or point-to-point interconnect link from a device's DMA master interface
to another component along the path to memory (often an IOMMU) may carry
fewer address bits than the interfaces at both ends nominally support.
In order to properly deal with this, the first step is to expand the
dma_32bit_limit flag into an arbitrary mask.

To minimise the impact on existing code, we'll make sure to only
consider this new mask valid if set. That makes sense anyway, since a
mask of zero would represent DMA not being wired up at all, and that
would be better handled by not providing valid ops in the first place.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/x86/kernel/pci-dma.c | 2 +-
 include/linux/device.h    | 6 +++---
 kernel/dma/direct.c       | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index ab5d9dd668d2..c29b0d453db3 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -175,7 +175,7 @@ rootfs_initcall(pci_iommu_init);
 
 static int via_no_dac_cb(struct pci_dev *pdev, void *data)
 {
-	pdev->dev.dma_32bit_limit = true;
+	pdev->bus_dma_mask = DMA_BIT_MASK(32);
 	return 0;
 }
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 055a69dbcd18..6d3b000be57e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -886,6 +886,8 @@ struct dev_links_info {
  * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all
  * 		hardware supports 64-bit addresses for consistent allocations
  * 		such descriptors.
+ * @bus_dma_mask: Mask of an upstream bridge or bus which imposes a smaller DMA
+ *		limit than the device itself supports.
  * @dma_pfn_offset: offset of DMA memory range relatively of RAM
  * @dma_parms:	A low level driver may set these to teach IOMMU code about
  * 		segment limitations.
@@ -912,8 +914,6 @@ struct dev_links_info {
  * @offline:	Set after successful invocation of bus type's .offline().
  * @of_node_reused: Set if the device-tree node is shared with an ancestor
  *              device.
- * @dma_32bit_limit: bridge limited to 32bit DMA even if the device itself
- *		indicates support for a higher limit in the dma_mask field.
  *
  * At the lowest level, every device in a Linux system is represented by an
  * instance of struct device. The device structure contains the information
@@ -967,6 +967,7 @@ struct device {
 					     not all hardware supports
 					     64 bit addresses for consistent
 					     allocations such descriptors. */
+	u64		bus_dma_mask;	/* upstream dma_mask constraint */
 	unsigned long	dma_pfn_offset;
 
 	struct device_dma_parameters *dma_parms;
@@ -1002,7 +1003,6 @@ struct device {
 	bool			offline_disabled:1;
 	bool			offline:1;
 	bool			of_node_reused:1;
-	bool			dma_32bit_limit:1;
 };
 
 static inline struct device *kobj_to_dev(struct kobject *kobj)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 8be8106270c2..c2860c5a9e96 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -180,10 +180,10 @@ int dma_direct_supported(struct device *dev, u64 mask)
 		return 0;
 #endif
 	/*
-	 * Various PCI/PCIe bridges have broken support for > 32bit DMA even
-	 * if the device itself might support it.
+	 * Upstream PCI/PCIe bridges or SoC interconnects may not carry
+	 * as many DMA address bits as the device itself supports.
 	 */
-	if (dev->dma_32bit_limit && mask > DMA_BIT_MASK(32))
+	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
 		return 0;
 	return 1;
 }
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 2/7] dma-mapping: Generalise dma_32bit_limit flag
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

Whilst the notion of an upstream DMA restriction is most commonly seen
in PCI host bridges saddled with a 32-bit native interface, a more
general version of the same issue can exist on complex SoCs where a bus
or point-to-point interconnect link from a device's DMA master interface
to another component along the path to memory (often an IOMMU) may carry
fewer address bits than the interfaces at both ends nominally support.
In order to properly deal with this, the first step is to expand the
dma_32bit_limit flag into an arbitrary mask.

To minimise the impact on existing code, we'll make sure to only
consider this new mask valid if set. That makes sense anyway, since a
mask of zero would represent DMA not being wired up at all, and that
would be better handled by not providing valid ops in the first place.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/x86/kernel/pci-dma.c | 2 +-
 include/linux/device.h    | 6 +++---
 kernel/dma/direct.c       | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index ab5d9dd668d2..c29b0d453db3 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -175,7 +175,7 @@ rootfs_initcall(pci_iommu_init);
 
 static int via_no_dac_cb(struct pci_dev *pdev, void *data)
 {
-	pdev->dev.dma_32bit_limit = true;
+	pdev->bus_dma_mask = DMA_BIT_MASK(32);
 	return 0;
 }
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 055a69dbcd18..6d3b000be57e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -886,6 +886,8 @@ struct dev_links_info {
  * @coherent_dma_mask: Like dma_mask, but for alloc_coherent mapping as not all
  * 		hardware supports 64-bit addresses for consistent allocations
  * 		such descriptors.
+ * @bus_dma_mask: Mask of an upstream bridge or bus which imposes a smaller DMA
+ *		limit than the device itself supports.
  * @dma_pfn_offset: offset of DMA memory range relatively of RAM
  * @dma_parms:	A low level driver may set these to teach IOMMU code about
  * 		segment limitations.
@@ -912,8 +914,6 @@ struct dev_links_info {
  * @offline:	Set after successful invocation of bus type's .offline().
  * @of_node_reused: Set if the device-tree node is shared with an ancestor
  *              device.
- * @dma_32bit_limit: bridge limited to 32bit DMA even if the device itself
- *		indicates support for a higher limit in the dma_mask field.
  *
  * At the lowest level, every device in a Linux system is represented by an
  * instance of struct device. The device structure contains the information
@@ -967,6 +967,7 @@ struct device {
 					     not all hardware supports
 					     64 bit addresses for consistent
 					     allocations such descriptors. */
+	u64		bus_dma_mask;	/* upstream dma_mask constraint */
 	unsigned long	dma_pfn_offset;
 
 	struct device_dma_parameters *dma_parms;
@@ -1002,7 +1003,6 @@ struct device {
 	bool			offline_disabled:1;
 	bool			offline:1;
 	bool			of_node_reused:1;
-	bool			dma_32bit_limit:1;
 };
 
 static inline struct device *kobj_to_dev(struct kobject *kobj)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 8be8106270c2..c2860c5a9e96 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -180,10 +180,10 @@ int dma_direct_supported(struct device *dev, u64 mask)
 		return 0;
 #endif
 	/*
-	 * Various PCI/PCIe bridges have broken support for > 32bit DMA even
-	 * if the device itself might support it.
+	 * Upstream PCI/PCIe bridges or SoC interconnects may not carry
+	 * as many DMA address bits as the device itself supports.
 	 */
-	if (dev->dma_32bit_limit && mask > DMA_BIT_MASK(32))
+	if (dev->bus_dma_mask && mask > dev->bus_dma_mask)
 		return 0;
 	return 1;
 }
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 1/7] ACPI/IORT: Support address size limit for root complexes
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

IORT revision D allows PCI root complex nodes to specify a memory
address size limit equivalently to named components, to help describe
straightforward integrations which don't really warrant a full-blown
_DMA method. Now that our headers are up-to-date, plumb it in.

If both _DMA and an address size limit are present, we would always
expect the former to be a more specific subset of the latter (since it
makes little sense for a _DMA range to involve bits which IORT says
aren't wired up), thus we can save calculating an explicit intersection
of the two effective masks and simply use short-circuit logic instead.

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7a3a541046ed..4a66896e2aa3 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -947,6 +947,24 @@ static int nc_dma_get_range(struct device *dev, u64 *size)
 	return 0;
 }
 
+static int rc_dma_get_range(struct device *dev, u64 *size)
+{
+	struct acpi_iort_node *node;
+	struct acpi_iort_root_complex *rc;
+
+	node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_node_callback, dev);
+	if (!node || node->revision < 1)
+		return -ENODEV;
+
+	rc = (struct acpi_iort_root_complex *)node->node_data;
+
+	*size = rc->memory_address_limit >= 64 ? U64_MAX :
+			1ULL<<rc->memory_address_limit;
+
+	return 0;
+}
+
 /**
  * iort_dma_setup() - Set-up device DMA parameters.
  *
@@ -975,10 +993,13 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 
 	size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
 
-	if (dev_is_pci(dev))
+	if (dev_is_pci(dev)) {
 		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
-	else
+		if (ret == -ENODEV)
+			ret = rc_dma_get_range(dev, &size);
+	} else {
 		ret = nc_dma_get_range(dev, &size);
+	}
 
 	if (!ret) {
 		msb = fls64(dmaaddr + size - 1);
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 1/7] ACPI/IORT: Support address size limit for root complexes
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch, m.szyprowski
  Cc: lorenzo.pieralisi, gregkh, joro, x86, linux-acpi, iommu, robh+dt,
	hanjun.guo, sudeep.holla, frowand.list, linux-arm-kernel
In-Reply-To: <cover.1532382222.git.robin.murphy@arm.com>

IORT revision D allows PCI root complex nodes to specify a memory
address size limit equivalently to named components, to help describe
straightforward integrations which don't really warrant a full-blown
_DMA method. Now that our headers are up-to-date, plumb it in.

If both _DMA and an address size limit are present, we would always
expect the former to be a more specific subset of the latter (since it
makes little sense for a _DMA range to involve bits which IORT says
aren't wired up), thus we can save calculating an explicit intersection
of the two effective masks and simply use short-circuit logic instead.

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/acpi/arm64/iort.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7a3a541046ed..4a66896e2aa3 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -947,6 +947,24 @@ static int nc_dma_get_range(struct device *dev, u64 *size)
 	return 0;
 }
 
+static int rc_dma_get_range(struct device *dev, u64 *size)
+{
+	struct acpi_iort_node *node;
+	struct acpi_iort_root_complex *rc;
+
+	node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_node_callback, dev);
+	if (!node || node->revision < 1)
+		return -ENODEV;
+
+	rc = (struct acpi_iort_root_complex *)node->node_data;
+
+	*size = rc->memory_address_limit >= 64 ? U64_MAX :
+			1ULL<<rc->memory_address_limit;
+
+	return 0;
+}
+
 /**
  * iort_dma_setup() - Set-up device DMA parameters.
  *
@@ -975,10 +993,13 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 
 	size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
 
-	if (dev_is_pci(dev))
+	if (dev_is_pci(dev)) {
 		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
-	else
+		if (ret == -ENODEV)
+			ret = rc_dma_get_range(dev, &size);
+	} else {
 		ret = nc_dma_get_range(dev, &size);
+	}
 
 	if (!ret) {
 		msb = fls64(dmaaddr + size - 1);
-- 
2.17.1.dirty

^ permalink raw reply related

* [PATCH v2 0/7] Stop losing firmware-set DMA masks
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: linux-arm-kernel

Whilst the common firmware code invoked by dma_configure() initialises
devices' DMA masks according to limitations described by the respective
properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of
the dma_set_mask() API leads to that information getting lost when
well-behaved drivers probe and set a 64-bit mask, since in general
there's no way to tell the difference between a firmware-described mask
(which should be respected) and whatever default may have come from the
bus code (which should be replaced outright). This can break DMA on
systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver
only knows its maximum supported address size, not how many of those
address bits might actually be wired up between any of its input
interfaces and the associated DMA master devices. Similarly, some PCIe
root complexes only have a 32-bit native interface on their host bridge,
which leads to the same DMA-address-truncation problem in systems with a
larger physical memory map and RAM above 4GB (e.g. [2]).

These patches attempt to deal with this in the simplest way possible by
generalising the specific quirk for 32-bit bridges into an arbitrary
mask which can then also be plumbed into the firmware code. In the
interest of being minimally invasive, I've only included a point fix
for the IOMMU issue as seen on arm64 - there may be further tweaks
needed in DMA ops (e.g. in arch/arm/ and other OF users) to catch all
possible incarnations of this problem, but at least any that I'm not
fixing here have always been broken. It is also noteworthy that
of_dma_get_range() has never worked properly for the way PCI host
bridges are passed into of_dma_configure() - I'll be working on
further patches to sort that out once this part is done.

Changes since v1 (RFC):
- Pull in patch #1 (previously sent separately) to avoid conflicts
- Fix up comment and silly build-breaking typo in patch #2
- Add patches #6 and #7 since fiddling with coherent masks no longer
  serves a reasonable purpose

Robin.


[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/580804.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474443.html

Robin Murphy (7):
  ACPI/IORT: Support address size limit for root complexes
  dma-mapping: Generalise dma_32bit_limit flag
  ACPI/IORT: Set bus DMA mask as appropriate
  of/device: Set bus DMA mask as appropriate
  iommu/dma: Respect bus DMA limit for IOVAs
  ACPI/IORT: Don't set default coherent DMA mask
  OF: Don't set default coherent DMA mask

 arch/x86/kernel/pci-dma.c |  2 +-
 drivers/acpi/arm64/iort.c | 50 ++++++++++++++++++++++++++++-----------
 drivers/iommu/dma-iommu.c |  3 +++
 drivers/of/device.c       | 21 ++++++++--------
 include/linux/device.h    |  6 ++---
 kernel/dma/direct.c       |  6 ++---
 6 files changed, 57 insertions(+), 31 deletions(-)

-- 
2.17.1.dirty

^ permalink raw reply

* [PATCH v2 0/7] Stop losing firmware-set DMA masks
From: Robin Murphy @ 2018-07-23 22:16 UTC (permalink / raw)
  To: hch-jcswGhMUV9g, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	x86-DgEjT+Ai2ygdnm+yROfE0A, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, sudeep.holla-5wv7dgnIgG8,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Whilst the common firmware code invoked by dma_configure() initialises
devices' DMA masks according to limitations described by the respective
properties ("dma-ranges" for OF and _DMA/IORT for ACPI), the nature of
the dma_set_mask() API leads to that information getting lost when
well-behaved drivers probe and set a 64-bit mask, since in general
there's no way to tell the difference between a firmware-described mask
(which should be respected) and whatever default may have come from the
bus code (which should be replaced outright). This can break DMA on
systems with certain IOMMU topologies (e.g. [1]) where the IOMMU driver
only knows its maximum supported address size, not how many of those
address bits might actually be wired up between any of its input
interfaces and the associated DMA master devices. Similarly, some PCIe
root complexes only have a 32-bit native interface on their host bridge,
which leads to the same DMA-address-truncation problem in systems with a
larger physical memory map and RAM above 4GB (e.g. [2]).

These patches attempt to deal with this in the simplest way possible by
generalising the specific quirk for 32-bit bridges into an arbitrary
mask which can then also be plumbed into the firmware code. In the
interest of being minimally invasive, I've only included a point fix
for the IOMMU issue as seen on arm64 - there may be further tweaks
needed in DMA ops (e.g. in arch/arm/ and other OF users) to catch all
possible incarnations of this problem, but at least any that I'm not
fixing here have always been broken. It is also noteworthy that
of_dma_get_range() has never worked properly for the way PCI host
bridges are passed into of_dma_configure() - I'll be working on
further patches to sort that out once this part is done.

Changes since v1 (RFC):
- Pull in patch #1 (previously sent separately) to avoid conflicts
- Fix up comment and silly build-breaking typo in patch #2
- Add patches #6 and #7 since fiddling with coherent masks no longer
  serves a reasonable purpose

Robin.


[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/580804.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474443.html

Robin Murphy (7):
  ACPI/IORT: Support address size limit for root complexes
  dma-mapping: Generalise dma_32bit_limit flag
  ACPI/IORT: Set bus DMA mask as appropriate
  of/device: Set bus DMA mask as appropriate
  iommu/dma: Respect bus DMA limit for IOVAs
  ACPI/IORT: Don't set default coherent DMA mask
  OF: Don't set default coherent DMA mask

 arch/x86/kernel/pci-dma.c |  2 +-
 drivers/acpi/arm64/iort.c | 50 ++++++++++++++++++++++++++++-----------
 drivers/iommu/dma-iommu.c |  3 +++
 drivers/of/device.c       | 21 ++++++++--------
 include/linux/device.h    |  6 ++---
 kernel/dma/direct.c       |  6 ++---
 6 files changed, 57 insertions(+), 31 deletions(-)

-- 
2.17.1.dirty

^ permalink raw reply

* [PATCH v5] media: dvb-frontends: add Socionext MN88443x ISDB-S/T demodulator driver
From: kbuild test robot @ 2018-07-23 22:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180723085150.24266-1-suzuki.katsuhiro@socionext.com>

Hi Katsuhiro,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.18-rc6 next-20180723]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Katsuhiro-Suzuki/media-dvb-frontends-add-Socionext-MN88443x-ISDB-S-T-demodulator-driver/20180724-050011
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-i1-201829 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/media/dvb-frontends/mn88443x.c:15:10: fatal error: sc1501a.h: No such file or directory
    #include "sc1501a.h"
             ^~~~~~~~~~~
   compilation terminated.

vim +15 drivers/media/dvb-frontends/mn88443x.c

    14	
  > 15	#include "sc1501a.h"
    16	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 27693 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180724/6fbfffe0/attachment-0001.gz>

^ 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.