All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC 1/1] image: Add TEE loading to FIT loadable processing
From: Andrew F. Davis @ 2016-11-14 19:49 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20161114194925.17117-1-afd@ti.com>

To help automate the loading of a TEE image during the boot we add a new
FIT section type 'tee', when we see this type while loading the loadable
sections we automatically call the platforms TEE processing function on
this image section.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 Kconfig         | 10 ++++++++++
 common/image.c  | 18 ++++++++++++++++++
 include/image.h | 15 +++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/Kconfig b/Kconfig
index 1263d0b..97cf7c8 100644
--- a/Kconfig
+++ b/Kconfig
@@ -291,6 +291,16 @@ config FIT_IMAGE_POST_PROCESS
 	  injected into the FIT creation (i.e. the blobs would have been pre-
 	  processed before being added to the FIT image).
 
+config FIT_IMAGE_TEE_PROCESS
+	bool "Enable processing of TEE images during FIT loading by U-Boot"
+	depends on FIT && TI_SECURE_DEVICE
+	help
+	  Allows platforms to perform processing, such as authentication and
+	  installation, on TEE images extracted from FIT images in a platform
+	  or board specific way. In order to use this feature a platform or
+	  board-specific implementation of board_tee_image_process() must be
+	  provided.
+
 config SPL_DFU_SUPPORT
 	bool "Enable SPL with DFU to load binaries to memory device"
 	depends on USB
diff --git a/common/image.c b/common/image.c
index 7604494..4552ca5 100644
--- a/common/image.c
+++ b/common/image.c
@@ -165,6 +165,7 @@ static const table_entry_t uimage_type[] = {
 	{	IH_TYPE_ZYNQIMAGE,  "zynqimage",  "Xilinx Zynq Boot Image" },
 	{	IH_TYPE_ZYNQMPIMAGE, "zynqmpimage", "Xilinx ZynqMP Boot Image" },
 	{	IH_TYPE_FPGA,       "fpga",       "FPGA Image" },
+	{	IH_TYPE_TEE,        "tee",        "TEE OS Image",},
 	{	-1,		    "",		  "",			},
 };
 
@@ -1408,6 +1409,8 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
 	int fit_img_result;
 	const char *uname;
 
+	uint8_t img_type;
+
 	/* Check to see if the images struct has a FIT configuration */
 	if (!genimg_has_config(images)) {
 		debug("## FIT configuration was not specified\n");
@@ -1447,6 +1450,21 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
 				/* Something went wrong! */
 				return fit_img_result;
 			}
+
+			fit_img_result = fit_image_get_node(buf, uname);
+			if (fit_img_result < 0) {
+				/* Something went wrong! */
+				return fit_img_result;
+			}
+			fit_img_result = fit_image_get_type(buf, fit_img_result, &img_type);
+			if (fit_img_result < 0) {
+				/* Something went wrong! */
+				return fit_img_result;
+			}
+#if defined(CONFIG_FIT_IMAGE_TEE_PROCESS)
+			if (img_type == IH_TYPE_TEE)
+				board_tee_image_process(img_data, img_len);
+#endif
 		}
 		break;
 	default:
diff --git a/include/image.h b/include/image.h
index 2b1296c..57084c8 100644
--- a/include/image.h
+++ b/include/image.h
@@ -279,6 +279,7 @@ enum {
 	IH_TYPE_ZYNQMPIMAGE,		/* Xilinx ZynqMP Boot Image */
 	IH_TYPE_FPGA,			/* FPGA Image */
 	IH_TYPE_VYBRIDIMAGE,	/* VYBRID .vyb Image */
+	IH_TYPE_TEE,		/* Trusted Execution Environment OS Image */
 
 	IH_TYPE_COUNT,			/* Number of image types */
 };
@@ -1263,4 +1264,18 @@ int board_fit_config_name_match(const char *name);
 void board_fit_image_post_process(void **p_image, size_t *p_size);
 #endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
 
+#ifdef CONFIG_FIT_IMAGE_TEE_PROCESS
+/**
+ * board_fit_tee_process() - Do any needed processing on a loaded TEE image
+ *
+ * This is used to verify, decrypt, and/or install a TEE in a platform or
+ * board specific way.
+ *
+ * @tee_image: pointer to the image
+ * @tee_size: the image size
+ * @return no return value (failure should be handled internally)
+ */
+void board_tee_image_process(void *tee_image, size_t tee_size);
+#endif /* CONFIG_FIT_IMAGE_TEE_PROCESS */
+
 #endif	/* __IMAGE_H__ */
-- 
2.10.1

^ permalink raw reply related

* [U-Boot]  [RFC 0/1] Add TEE loading support to FIT image
From: Andrew F. Davis @ 2016-11-14 19:49 UTC (permalink / raw)
  To: u-boot

Hello all,

Internally[0] we use the a hook in the FIT loadable section to install
our trusted execution environment solution (OPTEE) which is packaged
in the FIT image with the rest of the platform's images. I would like
to get some feedback if this will be an acceptable solution?

This patch is the first step where we allow a platform to add a
custom hook for the "tee" type loadable. The rest of the work
is in the platform specific call and so not relevant to this RFC.

The resulting FIT .its looks something like this:

...
am57xx-evm.optee {
	description = "OPTEE OS Image";
	data = /incbin/("am57xx-evm.optee");
	type = "tee";
	arch = "arm";
        compression = "none";
};
...
configurations {
	am57xx-evm {
		description = "Linux kernel, FDT blob, and OPTEE OS for the AM57xx EVM";
		kernel = "kernel at 1";
		fdt = "am57xx-evm.dtb";
		loadables = "am57xx-evm.optee";
	};
...

Thanks,
Andrew

[0] git://git.ti.com/ti-u-boot/ti-u-boot.git

Andrew F. Davis (1):
  image: Add TEE loading to FIT loadable processing

 Kconfig         | 10 ++++++++++
 common/image.c  | 18 ++++++++++++++++++
 include/image.h | 15 +++++++++++++++
 3 files changed, 43 insertions(+)

-- 
2.10.1

^ permalink raw reply

* Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active
From: Alex Williamson @ 2016-11-14 19:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: cornelia.huck, famz, qemu-devel, stefanha, mst
In-Reply-To: <635f3da4-ce9f-5fe8-bbc7-e2fcf97bed63@redhat.com>

On Mon, 14 Nov 2016 20:33:32 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 14/11/2016 19:49, Alex Williamson wrote:
> > On Mon, 14 Nov 2016 19:10:54 +0100
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >   
> >> On 14/11/2016 18:09, Alex Williamson wrote:  
> >>> Hmm, fixed yet not fixed.  I get a nice shutdown and it even eliminates
> >>> a cpu spike shown in virt-manager at the end of shutdown that was
> >>> typical previously, but then I noticed dmesg showing me segfaults, so I
> >>> hooked up gdb and:    
> >>
> >> Well, that I don't get the segfault already says something...  Though 
> >> it's not even clear from the backtrace what is causing the segfault.  
> >> Let's try the same printf without the change.  
> > 
> > Log starting from virsh shutdown.  The final line occurs just as the VM
> > starts spinning on all vcpus.  Thanks,  
> 
> Ok, this could be a start.  Just for information what is your virtio-win
> version?

Device manager shows:

Driver Date: 9/22/2015
Driver Version: 62.72.104.11000

^ permalink raw reply

* Re: [PATCH] fetch/push: document that private data can be leaked
From: Junio C Hamano @ 2016-11-14 19:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Matt McCutchen, git
In-Reply-To: <20161114190725.fxjymvztc2eiomv6@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So I think the in-between answer is "it is OK to push to an
> untrustworthy place, but do not do it from a repo that may contain
> secret contents".

Yes, that sounds like a sensible piece of advice to give to the
readers.


^ permalink raw reply

* Re: [PATCH 0/2] Salvator-X: Add GPIO keys support
From: Geert Uytterhoeven @ 2016-11-14 19:47 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Laurent Pinchart, Linux-Renesas, Kieran Bingham
In-Reply-To: <3190171.WPEDSgjiKH@avalon>

Hi Laurent,

On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
>> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
>> > On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
>> >> The main reason I haven't sent out a similar series yet is because the
>> >> GPIOs used for the 3 push buttons are shared with the 3 user LEDs. For
>> >> each of them, you have to choose at DT time if you want to use them as
>> >> buttons or as LEDs.
>> >>
>> >> On ULCB, the same issue is present. For those, we settled on 1 key and 2
>> >> LEDs...
>> >>
>> >> Looking forward to more comments...
>> >
>> > In theory the GPIOs could be shared by the gpio-keys and LED drivers in
>> > open- drain mode. I'm not sure the GPIO subsystem supports that though.
>>
>> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
>> output (LEDs)". The result wasn't pretty...
>
> Wasn't it ? Linus basically told you to use open-drain GPIOs and fix the GPIO
> driver in case it can't read the value of GPIOs configured as output in open-
> drain mode. If didn't shoot the idea down.

If I'm not mistaken, the R-Car GPIO block does not support open-drain GPIO.
Even if it would support it, you cannot read the GPIO while actively
pulling it down.
Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
through switching between pulling down and not pulling down (or between
output and input, which is what I did).

Apart from that, there's also the discrepancy between hardware description
(the GPIO is connected to both buttons and LEDs, hence it should be described
that way in DT) and user policy (the user wants to use e.g. the first GPIO as a
button, and the second GPIO as an LED).

If you have ideas to solve these 2  issues, I'm happy  to hear your thoughts!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Ramsay Jones @ 2016-11-14 19:45 UTC (permalink / raw)
  To: Jeff King, Torsten Bögershausen
  Cc: Lars Schneider, Junio C Hamano, Johannes Schindelin,
	Johannes Sixt, git, pranit.bauva
In-Reply-To: <20161114170105.btnohk2777ddaiul@sigill.intra.peff.net>



On 14/11/16 17:01, Jeff King wrote:
> On Mon, Nov 14, 2016 at 05:35:56PM +0100, Torsten Bögershausen wrote:
> 
>>> Git 'pu' does not compile on macOS right now:
>>> builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized 
>>> whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> 
> The next step is to make sure that the topic author is aware (in this
> case, one assumes it's pb/bisect).

[+cc Pranit]

Yep, I had a quick squint, and it looks like the compiler is correct.
It should be complaining about the 'bad_syn' variable for exactly the
same reason: namely, whenever the if condition is true, the only exit
from that block is via 'goto finish' which bypasses the initialisation
of 'good_syn' and 'bad_syn'.

> Better still is to make a patch that can either be applied on top, or
> squashed as appropriate. 

No patch this time, but it simply requires those variables to be
initialised to NULL in their declarations. :-D

>                            I know that Ramsay Jones does this, for
> example, with some of his sparse-related checks, and I'm pretty sure
> from the turnaround-time that he runs it against "pu".

Yep, the idea being to catch these simple problems before the topic
reaches 'next'.

ATB,
Ramsay Jones



^ permalink raw reply

* Re: [Qemu-devel] [RFC 1/2] qom/cpu: move tlb_flush to cpu_common_reset
From: Richard Henderson @ 2016-11-14 19:25 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Eduardo Habkost, Michael Walle, Laurent Vivier, Aurelien Jarno,
	Yongbok Kim, Anthony Green, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC
In-Reply-To: <20161114174010.31040-2-alex.bennee@linaro.org>

On 11/14/2016 06:40 PM, Alex Bennée wrote:
> It is a common thing amongst the various cpu reset functions want to
> flush the SoftMMU's TLB entries. This is done either by calling
> tlb_flush directly or by way of a general memset of the CPU
> structure (sometimes both).
>
> This moves the tlb_flush call to the common reset function and
> additionally ensures it is only done for the CONFIG_SOFTMMU case and
> when tcg is enabled.
>
> In some target cases we add an empty end_of_reset_fields structure to the
> target vCPU structure so have a clear end point for any memset which
> is resetting value in the structure before CPU_COMMON (where the TLB
> structures are).
>
> While this is a nice clean-up in general it is also a precursor for
> changes coming to cputlb for MTTCG where the clearing of entries
> can't be done arbitrarily across vCPUs. Currently the cpu_reset
> function is usually called from the context of another vCPU as the
> architectural power up sequence is run. By using the cputlb API
> functions we can ensure the right behaviour in the future.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

^ permalink raw reply

* Re: [RFC 1/2] qom/cpu: move tlb_flush to cpu_common_reset
From: Richard Henderson @ 2016-11-14 19:25 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Eduardo Habkost, Michael Walle, Laurent Vivier, Aurelien Jarno,
	Yongbok Kim, Anthony Green, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC
In-Reply-To: <20161114174010.31040-2-alex.bennee@linaro.org>

On 11/14/2016 06:40 PM, Alex Bennée wrote:
> It is a common thing amongst the various cpu reset functions want to
> flush the SoftMMU's TLB entries. This is done either by calling
> tlb_flush directly or by way of a general memset of the CPU
> structure (sometimes both).
>
> This moves the tlb_flush call to the common reset function and
> additionally ensures it is only done for the CONFIG_SOFTMMU case and
> when tcg is enabled.
>
> In some target cases we add an empty end_of_reset_fields structure to the
> target vCPU structure so have a clear end point for any memset which
> is resetting value in the structure before CPU_COMMON (where the TLB
> structures are).
>
> While this is a nice clean-up in general it is also a precursor for
> changes coming to cputlb for MTTCG where the clearing of entries
> can't be done arbitrarily across vCPUs. Currently the cpu_reset
> function is usually called from the context of another vCPU as the
> architectural power up sequence is run. By using the cputlb API
> functions we can ensure the right behaviour in the future.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2016-11-14 19:20 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring
  Cc: Kevin Hilman, Jon Hunter, Rafael J . Wysocki,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	devicetree@vger.kernel.org, Nishanth Menon, Keerthy, Russell King,
	Tero Kristo, Sudeep Holla, Santosh Shilimkar
In-Reply-To: <CAPDyKFofZdFfGA8gbS9FJxAX5Ub-s8yZWDcNVOgwiEBpho4oyw@mail.gmail.com>

Hi,
On 11/11/2016 06:34 AM, Ulf Hansson wrote:
> On 10 November 2016 at 20:56, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Rob, Ulf, Jon,
>>
>> On 10/27/2016 08:15 AM, Dave Gerlach wrote:
>>>
>>> +Jon
>>> On 10/26/2016 04:59 PM, Rob Herring wrote:
>>>>
>>>> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman <khilman@baylibre.com>
>>>> wrote:
>>>>>
>>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>>
>>>>>> Hi,
>>>>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>>>>>>>
>>>>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>>>>
>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>>> control device power states.
>>>>>>>>
>>>>>>>> Also, provide macros representing each device index as understood
>>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>>
>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>>> ---
>>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 54
>>>>>>>> +++++++++++++
>>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>>> ++++++++++++++++++++++
>>>>>>>>  3 files changed, 146 insertions(+)
>>>>>>>>  create mode 100644
>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..32f38a349656
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> @@ -0,0 +1,54 @@
>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>>> +---------------------------------------------
>>>>>>>> +
>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>>> that is
>>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>>> +Communication between the host processor running an OS and the
>>>>>>>> system
>>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>>> domain
>>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>>> use of
>>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>>> +
>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>>> +
>>>>>>>> +PM Domain Node
>>>>>>>> +==============
>>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>>> PMMC,
>>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>>> generic
>>>>>>>> +PM domain bindings in
>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>>> +
>>>>>>>> +Required Properties:
>>>>>>>> +--------------------
>>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the
>>>>>>>> devices.
>>>>>>>>
>>>>>>>> +Example:
>>>>>>>> +--------------------
>>>>>>>> +k2g_pds: k2g_pds {
>>>>>>>
>>>>>>>
>>>>>>> should use generic name like "power-contoller", e.g. k2g_pds:
>>>>>>> power-controller
>>>>>>
>>>>>>
>>>>>> Ok, that makes more sense.
>>>>>>
>>>>>>>
>>>>>>>> +        compatible = "ti,sci-pm-domain";
>>>>>>>> +        #power-domain-cells = <0>;
>>>>>>>> +        ti,sci = <&pmmc>;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +PM Domain Consumers
>>>>>>>> +===================
>>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>>> provide
>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>>> device
>>>>>>>> +specific ID that identifies the device.
>>>>>>>> +
>>>>>>>> +Required Properties:
>>>>>>>> +--------------------
>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>>> node.
>>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>>> to
>>>>>>>> +        be used for device control.
>>>>>>>
>>>>>>>
>>>>>>> This ID doesn't look right.
>>>>>>>
>>>>>>> Why not use #power-domain-cells = <1> and pass the index in the DT?
>>>>>>> ...
>>>>
>>>>
>>>> Exactly. ti,sci-id is a NAK for me.
>>>
>>>
>>> I was told not to use the onecell during v1 discussion. I agree this would
>>> be
>>> ideal but I cannot due to what the bindings represent, the phandle
>>> parameter is
>>> an index into a list of genpds, whereas we need an actual ID number we can
>>> use
>>> and I do not have the ability to get that from the phandle.
>>>
>>> @Ulf/Jon, is there any hope of bringing back custom xlate functions for
>>> genpd
>>> providers? I don't have a good background on why it was even removed. I
>>> can
>>> maintain a single genpd for all devices but I need a way to parse this ID,
>>> whether it's from a separate property or a phandle. It is locked now to
>>> indexing
>>> into a list of genpds but I need additional per device information for
>>> devices
>>> bound to a genpd and I need either a custom parameter or the ability to
>>> parse
>>> the phandle myself.
>>>
>>
>> Any comments here? The meaning of the phandle onecell is fixed in the genpd
>> framework so I'm not sure how we want to move forward with this, I need to
>> pass a power domain ID to the genpd driver, and if this shouldn't be a new
>> property I'm not sure what direction we should take.
>>
>> Regards,
>> Dave
>>
>>
>>>>
>>>>>>>
>>>>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for
>>>>>>>> k2g.
>>>>>>>> +
>>>>>>>> +Example:
>>>>>>>> +--------------------
>>>>>>>> +uart0: serial@02530c00 {
>>>>>>>> +   compatible = "ns16550a";
>>>>>>>> +   ...
>>>>>>>> +   power-domains = <&k2g_pds>;
>>>>>>>> +   ti,sci-id = <K2G_DEV_UART0>;
>>>>>>>
>>>>>>>
>>>>>>> ... like this:
>>>>>>>
>>>>>>>      power-domains = <&k2g_pds K2G_DEV_UART0>;
>>>>>>
>>>>>>
>>>>>> That's how I did it in version one actually. I was able to define my
>>>>>> own xlate function to parse the phandle and get that index, but Ulf
>>>>>> pointed me to this series by Jon Hunter [1] that simplified genpd
>>>>>> providers and dropped the concept of adding your own xlate. This locks
>>>>>> the onecell approach to using a fixed static array of genpds that get
>>>>>> indexed into (without passing the index to the provider, just the
>>>>>> genpd that's looked up), which doesn't fit our usecase, as we don't
>>>>>> want a 1 to 1 genpd to device mapping based on the comments provided
>>>>>> in v1. Now we just use the genpd device attach/detach hooks to parse
>>>>>> the sci-id and then use it in the genpd device start/stop hooks.
>>>>
>>>>
>>>> I have no idea what any of this means. All sounds like driver
>>>> architecture, not anything to do with bindings.
>>>
>>>
>>> This was a response to Kevin, not part of binding description.
>>>
>>>>
>>>>>
>>>>> Ah, right.  I remember now.  This approach allows you to use a single
>>>>> genpd as discussed earlier.
>>>>>
>>>>> Makes sense now, suggestion retracted.
>>>>
>>>>
>>>> IIRC, the bindings in Jon's case had a node for each domain and didn't
>>>> need any additional property.
>>>
>>>
>>> Yes but we only have one domain and index into it, not into a list of
>>> domains,
>
> Exactly. And this my main point as well. We are not talking about a
> domain property but a device property.
>
>>> so the additional property is solving a different problem.
>
> Yes.
>
> Perhaps you could try to elaborate about what the TI SCI ID really
> represents for the device, as to help Rob understand the bigger
> picture?
>
> To me, the TI SCI ID, is similar to a "conid" for any another "device
> resource" (like clock, pinctrl, regulator etc) which we can describe
> in DT and assign to a device node. The only difference here, is that
> we don't have common API to fetch the resource (like clk_get(),
> regulator_get()), but instead we fetches the device's resource from
> SoC specific code, via genpd's device ->attach() callback.

Thanks for the response. Yes, you've pretty much hit it on the head. It 
is not an index into a list of genpds but rather identifies the device 
*within* a single genpd. It is a property specific to each device that 
resides in a ti-sci-genpd, not a mapping describing which genpd the 
device belongs to. The generic power domain binding is concerned with 
mapping the device to a specific genpd, which is does fine for us, but 
we have a sub mapping for devices that exist inside a genpd which, we 
must describe as well, hence the ti,sci-id.

Regards,
Dave

>
> Hope that helps.
>
> Kind regards
> Uffe
>

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v12 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops
From: Alex Williamson @ 2016-11-14 19:43 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
	bjsdjshi, linux-kernel
In-Reply-To: <1479138156-28905-6-git-send-email-kwankhede@nvidia.com>

On Mon, 14 Nov 2016 21:12:19 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Added APIs for pining and unpining set of pages. These call back into
> backend iommu module to actually pin and unpin pages.
> Added two new callback functions to struct vfio_iommu_driver_ops. Backend
> IOMMU module that supports pining and unpinning pages for mdev devices
> should provide these functions.
> 
> Renamed static functions in vfio_type1_iommu.c to resolve conflicts
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: Ia7417723aaae86bec2959ad9ae6c2915ddd340e0
> ---
>  drivers/vfio/vfio.c             | 103 ++++++++++++++++++++++++++++++++++++++++
>  drivers/vfio/vfio_iommu_type1.c |  20 ++++----
>  include/linux/vfio.h            |  12 ++++-
>  3 files changed, 124 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2e83bdf007fe..7dcfbca2016a 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1799,6 +1799,109 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
>  }
>  EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>  
> +
> +/*
> + * Pin a set of guest PFNs and return their associated host PFNs for local
> + * domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of user/guest
> + *		  PFNs should not be greater than PAGE_SIZE.
> + * @npage [in] :count of elements in array.  This count should not be greater
> + *		than PAGE_SIZE.
> + * @prot [in] : protection flags
> + * @phys_pfn[out] : array of host PFNs
> + * Return error or number of pages pinned.
> + */
> +int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, int npage,
> +		   int prot, unsigned long *phys_pfn)
> +{
> +	struct vfio_container *container;
> +	struct vfio_group *group;
> +	struct vfio_iommu_driver *driver;
> +	int ret;
> +
> +	if (!dev || !user_pfn || !phys_pfn || !npage)
> +		return -EINVAL;
> +
> +	if (npage >= PAGE_SIZE)
> +		return -E2BIG;

This misses the point of using PAGE_SIZE.  The concern is that
previously we were allowing (nearly) arbitrarily large arrays to be
passed around.  The agreement as I understood it would be that the
array itself would be sized up to a maximum of PAGE_SIZE, which means
the number of entries cannot exceed PAGE_SIZE/sizeof(*user_pfn) (ie.
512 of x86).  I also suggested that we should have a #define for this so
that vendor drivers can actually chunk their calls into allowable sizes
if they need to and not need to guess the limit, ex.

include/linux/vfio.h
#define VFIO_PAGE_PINNING_MAX_ENTRIES (PAGE_SIZE / sizeof(unsigned
long))

If we wanted a simple limit to the number of entries per call, there
would be no reason to have it based on PAGE_SIZE.  Thanks,

Alex

> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		goto err_pin_pages;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	driver = container->iommu_driver;
> +	if (likely(driver && driver->ops->pin_pages))
> +		ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
> +					     npage, prot, phys_pfn);
> +	else
> +		ret = -ENOTTY;
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +err_pin_pages:
> +	vfio_group_put(group);
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_pin_pages);
> +
> +/*
> + * Unpin set of host PFNs for local domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of user/guest
> + *		  PFNs should not be greater than PAGE_SIZE.
> + * @npage [in] :count of elements in array.  This count should not be greater
> + *		than PAGE_SIZE.
> + * Return error or number of pages unpinned.
> + */
> +int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage)
> +{
> +	struct vfio_container *container;
> +	struct vfio_group *group;
> +	struct vfio_iommu_driver *driver;
> +	int ret;
> +
> +	if (!dev || !user_pfn || !npage)
> +		return -EINVAL;
> +
> +	if (npage >= PAGE_SIZE)
> +		return -E2BIG;
> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		goto err_unpin_pages;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	driver = container->iommu_driver;
> +	if (likely(driver && driver->ops->unpin_pages))
> +		ret = driver->ops->unpin_pages(container->iommu_data, user_pfn,
> +					       npage);
> +	else
> +		ret = -ENOTTY;
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +err_unpin_pages:
> +	vfio_group_put(group);
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_unpin_pages);
> +
>  /**
>   * Module/class support
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 2ba19424e4a1..9f3d58d3dfaf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -259,8 +259,8 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>   * the iommu can only map chunks of consecutive pfns anyway, so get the
>   * first page and all consecutive pages with the same locking.
>   */
> -static long vfio_pin_pages(unsigned long vaddr, long npage,
> -			   int prot, unsigned long *pfn_base)
> +static long vfio_pin_pages_remote(unsigned long vaddr, long npage,
> +				  int prot, unsigned long *pfn_base)
>  {
>  	unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  	bool lock_cap = capable(CAP_IPC_LOCK);
> @@ -318,8 +318,8 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>  	return i;
>  }
>  
> -static long vfio_unpin_pages(unsigned long pfn, long npage,
> -			     int prot, bool do_accounting)
> +static long vfio_unpin_pages_remote(unsigned long pfn, long npage,
> +				    int prot, bool do_accounting)
>  {
>  	unsigned long unlocked = 0;
>  	long i;
> @@ -382,9 +382,9 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>  		if (WARN_ON(!unmapped))
>  			break;
>  
> -		unlocked += vfio_unpin_pages(phys >> PAGE_SHIFT,
> -					     unmapped >> PAGE_SHIFT,
> -					     dma->prot, false);
> +		unlocked += vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
> +						    unmapped >> PAGE_SHIFT,
> +						    dma->prot, false);
>  		iova += unmapped;
>  
>  		cond_resched();
> @@ -613,8 +613,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  
>  	while (size) {
>  		/* Pin a contiguous chunk of memory */
> -		npage = vfio_pin_pages(vaddr + dma->size,
> -				       size >> PAGE_SHIFT, prot, &pfn);
> +		npage = vfio_pin_pages_remote(vaddr + dma->size,
> +					      size >> PAGE_SHIFT, prot, &pfn);
>  		if (npage <= 0) {
>  			WARN_ON(!npage);
>  			ret = (int)npage;
> @@ -624,7 +624,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  		/* Map it! */
>  		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
>  		if (ret) {
> -			vfio_unpin_pages(pfn, npage, prot, true);
> +			vfio_unpin_pages_remote(pfn, npage, prot, true);
>  			break;
>  		}
>  
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 0ecae0b1cd34..86f507d0f585 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -75,7 +75,11 @@ struct vfio_iommu_driver_ops {
>  					struct iommu_group *group);
>  	void		(*detach_group)(void *iommu_data,
>  					struct iommu_group *group);
> -
> +	int		(*pin_pages)(void *iommu_data, unsigned long *user_pfn,
> +				     int npage, int prot,
> +				     unsigned long *phys_pfn);
> +	int		(*unpin_pages)(void *iommu_data,
> +				       unsigned long *user_pfn, int npage);
>  };
>  
>  extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
> @@ -127,6 +131,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>  }
>  #endif /* CONFIG_EEH */
>  
> +extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> +			  int npage, int prot, unsigned long *phys_pfn);
> +
> +extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
> +			    int npage);
> +
>  /*
>   * IRQfd - generic
>   */

^ permalink raw reply

* Re: [PATCH v12 05/22] vfio iommu: Added pin and unpin callback functions to vfio_iommu_driver_ops
From: Alex Williamson @ 2016-11-14 19:43 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
	bjsdjshi, linux-kernel
In-Reply-To: <1479138156-28905-6-git-send-email-kwankhede@nvidia.com>

On Mon, 14 Nov 2016 21:12:19 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> Added APIs for pining and unpining set of pages. These call back into
> backend iommu module to actually pin and unpin pages.
> Added two new callback functions to struct vfio_iommu_driver_ops. Backend
> IOMMU module that supports pining and unpinning pages for mdev devices
> should provide these functions.
> 
> Renamed static functions in vfio_type1_iommu.c to resolve conflicts
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: Ia7417723aaae86bec2959ad9ae6c2915ddd340e0
> ---
>  drivers/vfio/vfio.c             | 103 ++++++++++++++++++++++++++++++++++++++++
>  drivers/vfio/vfio_iommu_type1.c |  20 ++++----
>  include/linux/vfio.h            |  12 ++++-
>  3 files changed, 124 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2e83bdf007fe..7dcfbca2016a 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1799,6 +1799,109 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
>  }
>  EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
>  
> +
> +/*
> + * Pin a set of guest PFNs and return their associated host PFNs for local
> + * domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of user/guest
> + *		  PFNs should not be greater than PAGE_SIZE.
> + * @npage [in] :count of elements in array.  This count should not be greater
> + *		than PAGE_SIZE.
> + * @prot [in] : protection flags
> + * @phys_pfn[out] : array of host PFNs
> + * Return error or number of pages pinned.
> + */
> +int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, int npage,
> +		   int prot, unsigned long *phys_pfn)
> +{
> +	struct vfio_container *container;
> +	struct vfio_group *group;
> +	struct vfio_iommu_driver *driver;
> +	int ret;
> +
> +	if (!dev || !user_pfn || !phys_pfn || !npage)
> +		return -EINVAL;
> +
> +	if (npage >= PAGE_SIZE)
> +		return -E2BIG;

This misses the point of using PAGE_SIZE.  The concern is that
previously we were allowing (nearly) arbitrarily large arrays to be
passed around.  The agreement as I understood it would be that the
array itself would be sized up to a maximum of PAGE_SIZE, which means
the number of entries cannot exceed PAGE_SIZE/sizeof(*user_pfn) (ie.
512 of x86).  I also suggested that we should have a #define for this so
that vendor drivers can actually chunk their calls into allowable sizes
if they need to and not need to guess the limit, ex.

include/linux/vfio.h
#define VFIO_PAGE_PINNING_MAX_ENTRIES (PAGE_SIZE / sizeof(unsigned
long))

If we wanted a simple limit to the number of entries per call, there
would be no reason to have it based on PAGE_SIZE.  Thanks,

Alex

> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		goto err_pin_pages;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	driver = container->iommu_driver;
> +	if (likely(driver && driver->ops->pin_pages))
> +		ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
> +					     npage, prot, phys_pfn);
> +	else
> +		ret = -ENOTTY;
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +err_pin_pages:
> +	vfio_group_put(group);
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_pin_pages);
> +
> +/*
> + * Unpin set of host PFNs for local domain only.
> + * @dev [in] : device
> + * @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of user/guest
> + *		  PFNs should not be greater than PAGE_SIZE.
> + * @npage [in] :count of elements in array.  This count should not be greater
> + *		than PAGE_SIZE.
> + * Return error or number of pages unpinned.
> + */
> +int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage)
> +{
> +	struct vfio_container *container;
> +	struct vfio_group *group;
> +	struct vfio_iommu_driver *driver;
> +	int ret;
> +
> +	if (!dev || !user_pfn || !npage)
> +		return -EINVAL;
> +
> +	if (npage >= PAGE_SIZE)
> +		return -E2BIG;
> +
> +	group = vfio_group_get_from_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	ret = vfio_group_add_container_user(group);
> +	if (ret)
> +		goto err_unpin_pages;
> +
> +	container = group->container;
> +	down_read(&container->group_lock);
> +
> +	driver = container->iommu_driver;
> +	if (likely(driver && driver->ops->unpin_pages))
> +		ret = driver->ops->unpin_pages(container->iommu_data, user_pfn,
> +					       npage);
> +	else
> +		ret = -ENOTTY;
> +
> +	up_read(&container->group_lock);
> +	vfio_group_try_dissolve_container(group);
> +
> +err_unpin_pages:
> +	vfio_group_put(group);
> +	return ret;
> +}
> +EXPORT_SYMBOL(vfio_unpin_pages);
> +
>  /**
>   * Module/class support
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 2ba19424e4a1..9f3d58d3dfaf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -259,8 +259,8 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>   * the iommu can only map chunks of consecutive pfns anyway, so get the
>   * first page and all consecutive pages with the same locking.
>   */
> -static long vfio_pin_pages(unsigned long vaddr, long npage,
> -			   int prot, unsigned long *pfn_base)
> +static long vfio_pin_pages_remote(unsigned long vaddr, long npage,
> +				  int prot, unsigned long *pfn_base)
>  {
>  	unsigned long limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>  	bool lock_cap = capable(CAP_IPC_LOCK);
> @@ -318,8 +318,8 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>  	return i;
>  }
>  
> -static long vfio_unpin_pages(unsigned long pfn, long npage,
> -			     int prot, bool do_accounting)
> +static long vfio_unpin_pages_remote(unsigned long pfn, long npage,
> +				    int prot, bool do_accounting)
>  {
>  	unsigned long unlocked = 0;
>  	long i;
> @@ -382,9 +382,9 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
>  		if (WARN_ON(!unmapped))
>  			break;
>  
> -		unlocked += vfio_unpin_pages(phys >> PAGE_SHIFT,
> -					     unmapped >> PAGE_SHIFT,
> -					     dma->prot, false);
> +		unlocked += vfio_unpin_pages_remote(phys >> PAGE_SHIFT,
> +						    unmapped >> PAGE_SHIFT,
> +						    dma->prot, false);
>  		iova += unmapped;
>  
>  		cond_resched();
> @@ -613,8 +613,8 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  
>  	while (size) {
>  		/* Pin a contiguous chunk of memory */
> -		npage = vfio_pin_pages(vaddr + dma->size,
> -				       size >> PAGE_SHIFT, prot, &pfn);
> +		npage = vfio_pin_pages_remote(vaddr + dma->size,
> +					      size >> PAGE_SHIFT, prot, &pfn);
>  		if (npage <= 0) {
>  			WARN_ON(!npage);
>  			ret = (int)npage;
> @@ -624,7 +624,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
>  		/* Map it! */
>  		ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage, prot);
>  		if (ret) {
> -			vfio_unpin_pages(pfn, npage, prot, true);
> +			vfio_unpin_pages_remote(pfn, npage, prot, true);
>  			break;
>  		}
>  
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 0ecae0b1cd34..86f507d0f585 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -75,7 +75,11 @@ struct vfio_iommu_driver_ops {
>  					struct iommu_group *group);
>  	void		(*detach_group)(void *iommu_data,
>  					struct iommu_group *group);
> -
> +	int		(*pin_pages)(void *iommu_data, unsigned long *user_pfn,
> +				     int npage, int prot,
> +				     unsigned long *phys_pfn);
> +	int		(*unpin_pages)(void *iommu_data,
> +				       unsigned long *user_pfn, int npage);
>  };
>  
>  extern int vfio_register_iommu_driver(const struct vfio_iommu_driver_ops *ops);
> @@ -127,6 +131,12 @@ static inline long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>  }
>  #endif /* CONFIG_EEH */
>  
> +extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
> +			  int npage, int prot, unsigned long *phys_pfn);
> +
> +extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn,
> +			    int npage);
> +
>  /*
>   * IRQfd - generic
>   */


^ permalink raw reply

* [U-Boot] [PATCH] drivers: SPI: sunxi SPL: fix warning
From: André Przywara @ 2016-11-14 19:43 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20161114203250.5bfe232d@i7>

On 14/11/16 18:32, Siarhei Siamashka wrote:
> On Thu,  3 Nov 2016 00:58:12 +0000
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
>> Somehow an int returning function without a return statement sneaked
>> in. Fix it.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  drivers/mtd/spi/sunxi_spi_spl.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi/sunxi_spi_spl.c b/drivers/mtd/spi/sunxi_spi_spl.c
>> index 67c7edd..7502314 100644
>> --- a/drivers/mtd/spi/sunxi_spi_spl.c
>> +++ b/drivers/mtd/spi/sunxi_spi_spl.c
>> @@ -158,9 +158,10 @@ static void spi0_disable_clock(void)
>>  			     (1 << AHB_RESET_SPI0_SHIFT));
>>  }
>>  
>> -static int spi0_init(void)
>> +static void spi0_init(void)
>>  {
>>  	unsigned int pin_function = SUNXI_GPC_SPI0;
>> +
>>  	if (IS_ENABLED(CONFIG_MACH_SUN50I))
>>  		pin_function = SUN50I_GPC_SPI0;
>>  
> 

Hi Siarhei,

> Thanks for spotting and fixing this compilation warning.
> 
> This was a last minute change. Originally there was also a check for
> the pins state and the function returned an error code in the case if
> the pins are already configured for NAND. But I removed this code
> before sending the patch.

please, no need to apologize or explain, those things happen, especially
when shuffling around patches.

The only learning that we should take from it that at least those
compilation warnings should be spotted on a more automated base.
I think the problem here is that the feature is not enabled in any
defconfig atm, but I think I will enable it in my Opi PC 2 series.

On a related matter, I ran buildman on HEAD for armv8 today and GCC 6.2
found quite some issues (will send out the fixes ASAP).
So is there some buildbot somewhere that runs buildman? If yes, with
what compilers?

> The idea is that probing the SPI flash may be useful in the future even
> if booting from some other media. We may store some board-specific
> configuration in the on-board SPI flash (for example, the DRAM and
> CPU speed grade information). But this functionality definitely belongs
> to a separate patch and can be always contributed later. There is also
> the SPL size concern and we don't want to unnecessarily increase the
> code size.

Totally agree, and as I said: No worries. Actually thanks a lot for this
series, as booting from NOR flash is really a cool feature.

Cheers,
Andre.

^ permalink raw reply

* Re: [PATCH 3/3] blk-mq: make the polling code adaptive
From: Omar Sandoval @ 2016-11-14 19:43 UTC (permalink / raw)
  To: Jens Axboe; +Cc: axboe, linux-block, linux-fsdevel, hch
In-Reply-To: <1478927487-12998-4-git-send-email-axboe@fb.com>

On Fri, Nov 11, 2016 at 10:11:27PM -0700, Jens Axboe wrote:
> The previous commit introduced the hybrid sleep/poll mode. Take
> that one step further, and use the completion latencies to
> automatically sleep for half the mean completion time. This is
> a good approximation.
> 
> This changes the 'io_poll_delay' sysfs file a bit to expose the
> various options. Depending on the value, the polling code will
> behave differently:
> 
> -1	Never enter hybrid sleep mode
>  0	Use half of the completion mean for the sleep delay
> >0	Use this specific value as the sleep delay
> 
> Signed-off-by: Jens Axboe <axboe@fb.com>
> ---
>  block/blk-mq.c         | 74 ++++++++++++++++++++++++++++++++++++++++++++++----
>  block/blk-sysfs.c      | 26 ++++++++++++------
>  include/linux/blkdev.h |  2 +-
>  3 files changed, 88 insertions(+), 14 deletions(-)
> 

[snip]

>  static bool blk_mq_poll_hybrid_sleep(struct request_queue *q,
> +				     struct blk_mq_hw_ctx *hctx,
>  				     struct request *rq)
>  {
>  	struct hrtimer_sleeper hs;
> +	enum hrtimer_mode mode;
> +	unsigned int nsecs;
>  	ktime_t kt;
>  
> -	if (!q->poll_nsec || test_bit(REQ_ATOM_POLL_SLEPT, &rq->atomic_flags))
> +	if (test_bit(REQ_ATOM_POLL_SLEPT, &rq->atomic_flags))
> +		return false;
> +
> +	/*
> +	 * poll_nsec can be:
> +	 *
> +	 * -1:	don't ever hybrid sleep
> +	 *  0:	use half of prev avg
> +	 * >0:	use this specific value
> +	 */
> +	if (q->poll_nsec == -1)
> +		return false;
> +	else if (q->poll_nsec > 0)
> +		nsecs = q->poll_nsec;
> +	else
> +		nsecs = blk_mq_poll_nsecs(q, hctx, rq);
> +
> +	if (!nsecs)
>  		return false;
>  
>  	set_bit(REQ_ATOM_POLL_SLEPT, &rq->atomic_flags);
> @@ -2477,9 +2539,10 @@ static bool blk_mq_poll_hybrid_sleep(struct request_queue *q,
>  	 * This will be replaced with the stats tracking code, using
>  	 * 'avg_completion_time / 2' as the pre-sleep target.
>  	 */
> -	kt = ktime_set(0, q->poll_nsec);
> +	kt = ktime_set(0, nsecs);
>  
> -	hrtimer_init_on_stack(&hs.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	mode = HRTIMER_MODE_REL;
> +	hrtimer_init_on_stack(&hs.timer, CLOCK_MONOTONIC, mode);
>  	hrtimer_set_expires(&hs.timer, kt);
>  
>  	hrtimer_init_sleeper(&hs, current);
> @@ -2487,10 +2550,11 @@ static bool blk_mq_poll_hybrid_sleep(struct request_queue *q,
>  		if (test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags))
>  			break;
>  		set_current_state(TASK_UNINTERRUPTIBLE);
> -		hrtimer_start_expires(&hs.timer, HRTIMER_MODE_REL);
> +		hrtimer_start_expires(&hs.timer, mode);
>  		if (hs.task)
>  			io_schedule();
>  		hrtimer_cancel(&hs.timer);
> +		mode = HRTIMER_MODE_ABS;
>  	} while (hs.task && !signal_pending(current));

This fix should be folded into patch 2.

>  	__set_current_state(TASK_RUNNING);
> @@ -2510,7 +2574,7 @@ static bool __blk_mq_poll(struct blk_mq_hw_ctx *hctx, struct request *rq)
>  	 * the IO isn't complete, we'll get called again and will go
>  	 * straight to the busy poll loop.
>  	 */
> -	if (blk_mq_poll_hybrid_sleep(q, rq))
> +	if (blk_mq_poll_hybrid_sleep(q, hctx, rq))
>  		return true;
>  
>  	hctx->poll_considered++;

[snip]

-- 
Omar

^ permalink raw reply

* [PATCH] time: Avoid signed overflow in timekeeping_delta_to_ns()
From: Chris Metcalf @ 2016-11-14 19:42 UTC (permalink / raw)
  To: John Stultz, Thomas Gleixner, Laurent Vivier, David Gibson,
	Christopher S . Hall, linux-kernel
  Cc: Chris Metcalf

This bugfix was originally made in commit 35a4933a8959 ("time:
Avoid signed overflow in timekeeping_get_ns()").  When the code was
refactored in commit 6bd58f09e1d8 ("time: Add cycles to nanoseconds
translation") the signed overflow fix was lost.  Re-introduce it.

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
---
I happened to be looking for an unrelated fix, found this code,
realized the tip code didn't match the fixed code, and
backtracked to where it had gone away.

 kernel/time/timekeeping.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 37dec7e3db43..57926bc7b7f3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -304,8 +304,7 @@ static inline s64 timekeeping_delta_to_ns(struct tk_read_base *tkr,
 {
 	s64 nsec;
 
-	nsec = delta * tkr->mult + tkr->xtime_nsec;
-	nsec >>= tkr->shift;
+	nsec = (delta * tkr->mult + tkr->xtime_nsec) >> tkr->shift;
 
 	/* If arch requires, add in get_arch_timeoffset() */
 	return nsec + arch_gettimeoffset();
-- 
2.7.2

^ permalink raw reply related

* RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
From: Roberts, William C @ 2016-11-14 19:41 UTC (permalink / raw)
  To: Roberts, William C, Stephen Smalley, selinux@tycho.nsa.gov
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC561CD24A5C@ORSMSX103.amr.corp.intel.com>



> -----Original Message-----
> From: Selinux [mailto:selinux-bounces@tycho.nsa.gov] On Behalf Of Roberts,
> William C
> Sent: Monday, November 14, 2016 10:44 AM
> To: Stephen Smalley <sds@tycho.nsa.gov>; selinux@tycho.nsa.gov
> Subject: RE: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> 
> 
> 
> > -----Original Message-----
> > From: Selinux [mailto:selinux-bounces@tycho.nsa.gov] On Behalf Of
> > Stephen Smalley
> > Sent: Monday, November 14, 2016 9:48 AM
> > To: selinux@tycho.nsa.gov
> > Cc: Stephen Smalley <sds@tycho.nsa.gov>
> > Subject: [PATCH v2] libsepol: fix checkpolicy dontaudit compiler bug
> >
> > The combining logic for dontaudit rules was wrong, causing a dontaudit
> > A B:C *; rule to be clobbered by a dontaudit A B:C p; rule.
> >
> > Reported-by: Nick Kralevich <nnk@google.com>
> > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> > ---
> >  libsepol/src/expand.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c index
> > 004a029..d7adbf8
> > 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -1604,7 +1604,8 @@ static int expand_range_trans(expand_state_t *
> > state, static avtab_ptr_t find_avtab_node(sepol_handle_t * handle,
> >  				   avtab_t * avtab, avtab_key_t * key,
> >  				   cond_av_list_t ** cond,
> > -				   av_extended_perms_t *xperms)
> > +				   av_extended_perms_t *xperms,
> > +				   char *alloced)
> >  {
> >  	avtab_ptr_t node;
> >  	avtab_datum_t avdatum;
> > @@ -1658,6 +1659,11 @@ static avtab_ptr_t
> > find_avtab_node(sepol_handle_t * handle,
> >  			nl->next = *cond;
> >  			*cond = nl;
> >  		}
> > +		if (alloced)
> > +			*alloced = 1;
> > +	} else {
> > +		if (alloced)
> > +			*alloced = 0;
> >  	}
> >
> >  	return node;
> > @@ -1750,7 +1756,7 @@ static int expand_terule_helper(sepol_handle_t *
> > handle,
> >  			return EXPAND_RULE_CONFLICT;
> >  		}
> >
> > -		node = find_avtab_node(handle, avtab, &avkey, cond, NULL);
> > +		node = find_avtab_node(handle, avtab, &avkey, cond, NULL,
> > NULL);
> >  		if (!node)
> >  			return -1;
> >  		if (enabled) {
> > @@ -1790,6 +1796,7 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> >  	class_perm_node_t *cur;
> >  	uint32_t spec = 0;
> >  	unsigned int i;
> > +	char alloced;
> >
> >  	if (specified & AVRULE_ALLOWED) {
> >  		spec = AVTAB_ALLOWED;
> > @@ -1824,7 +1831,8 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> >  		avkey.target_class = cur->tclass;
> >  		avkey.specified = spec;
> >
> > -		node = find_avtab_node(handle, avtab, &avkey, cond,
> > extended_perms);
> > +		node = find_avtab_node(handle, avtab, &avkey, cond,
> > +				       extended_perms, &alloced);
> >  		if (!node)
> >  			return EXPAND_RULE_ERROR;
> >  		if (enabled) {
> > @@ -1850,7 +1858,7 @@ static int expand_avrule_helper(sepol_handle_t *
> > handle,
> >  			 */
> >  			avdatump->data &= cur->data;
> >  		} else if (specified & AVRULE_DONTAUDIT) {
> > -			if (avdatump->data)
> > +			if (!alloced)
> >  				avdatump->data &= ~cur->data;
> >  			else
> >  				avdatump->data = ~cur->data;
> 
> This seems awkward to me. If the insertion created a new empty node why
> wouldn't !avdump->data be true (note the addition of the not operator)?

I misstated that a bit, but the !avdump->data was the else case. I am really
saying why didn't this work before? In my mind, we don't care if its allocated
we care if it's set or not.

> 
> Or perhaps a mechanism to actual set the data on allocation, this way the logic is
> Just &=.
> 
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> 
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Rob Herring @ 2016-11-14 19:41 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram
  Cc: mark.rutland, mchehab, hverkuil, sakari.ailus, crope,
	chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-3-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

On Wed, Nov 09, 2016 at 03:44:41PM +0000, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  .../devicetree/bindings/media/i2c/max2175.txt      |   61 +

It's preferred that bindings are a separate patch.

>  drivers/media/i2c/Kconfig                          |    4 +
>  drivers/media/i2c/Makefile                         |    2 +
>  drivers/media/i2c/max2175/Kconfig                  |    8 +
>  drivers/media/i2c/max2175/Makefile                 |    4 +
>  drivers/media/i2c/max2175/max2175.c                | 1558 ++++++++++++++++++++
>  drivers/media/i2c/max2175/max2175.h                |  108 ++
>  7 files changed, 1745 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/max2175.txt
>  create mode 100644 drivers/media/i2c/max2175/Kconfig
>  create mode 100644 drivers/media/i2c/max2175/Makefile
>  create mode 100644 drivers/media/i2c/max2175/max2175.c
>  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> new file mode 100644
> index 0000000..69f0dad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> @@ -0,0 +1,61 @@
> +Maxim Integrated MAX2175 RF to Bits tuner
> +-----------------------------------------
> +
> +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver with
> +RF to Bits® front-end designed for software-defined radio solutions.
> +
> +Required properties:
> +--------------------
> +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> +- clocks: phandle to the fixed xtal clock.
> +- clock-names: name of the fixed xtal clock.
> +- port: child port node of a tuner that defines the local and remote
> +  endpoints. The remote endpoint is assumed to be an SDR device
> +  that is capable of receiving the digital samples from the tuner.
> +
> +Optional properties:
> +--------------------
> +- maxim,slave	      : empty property indicates this is a slave of
> +			another master tuner. This is used to define two
> +			tuners in diversity mode (1 master, 1 slave). By
> +			default each tuner is an individual master.
> +- maxim,refout-load-pF: load capacitance value (in pF) on reference

Please add 'pF' to property-units.txt.

> +			output drive level. The possible load values are
> +			 0pF (default - refout disabled)
> +			10pF
> +			20pF
> +			30pF
> +			40pF
> +			60pF
> +			70pF
> +- maxim,am-hiz	      : empty property indicates AM Hi-Z filter path is
> +			selected for AM antenna input. By default this
> +			filter path is not used.
> +
> +Example:
> +--------
> +
> +Board specific DTS file
> +
> +/* Fixed XTAL clock node */
> +maxim_xtal: maximextal {

clock {

> +	compatible = "fixed-clock";
> +	#clock-cells = <0>;
> +	clock-frequency = <36864000>;
> +};
> +
> +/* A tuner device instance under i2c bus */
> +max2175_0: tuner@60 {
> +	compatible = "maxim,max2175";
> +	reg = <0x60>;
> +	clocks = <&maxim_xtal>;
> +	clock-names = "xtal";
> +	maxim,refout-load-pF = <10>;
> +
> +	port {
> +		max2175_0_ep: endpoint {
> +			remote-endpoint = <&slave_rx_v4l2_sdr_device>;

'v4l2' is not something that should appear in a DT.

> +		};
> +	};
> +
> +};

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Rob Herring @ 2016-11-14 19:41 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram
  Cc: mark.rutland-5wv7dgnIgG8, mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA, crope-X3B1VOXEql0,
	chris.paterson2-zM6kxYcvzFBBDgjK7y7TUQ,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478706284-59134-3-git-send-email-ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>

On Wed, Nov 09, 2016 at 03:44:41PM +0000, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
> ---
>  .../devicetree/bindings/media/i2c/max2175.txt      |   61 +

It's preferred that bindings are a separate patch.

>  drivers/media/i2c/Kconfig                          |    4 +
>  drivers/media/i2c/Makefile                         |    2 +
>  drivers/media/i2c/max2175/Kconfig                  |    8 +
>  drivers/media/i2c/max2175/Makefile                 |    4 +
>  drivers/media/i2c/max2175/max2175.c                | 1558 ++++++++++++++++++++
>  drivers/media/i2c/max2175/max2175.h                |  108 ++
>  7 files changed, 1745 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/max2175.txt
>  create mode 100644 drivers/media/i2c/max2175/Kconfig
>  create mode 100644 drivers/media/i2c/max2175/Makefile
>  create mode 100644 drivers/media/i2c/max2175/max2175.c
>  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> new file mode 100644
> index 0000000..69f0dad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> @@ -0,0 +1,61 @@
> +Maxim Integrated MAX2175 RF to Bits tuner
> +-----------------------------------------
> +
> +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver with
> +RF to Bits® front-end designed for software-defined radio solutions.
> +
> +Required properties:
> +--------------------
> +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> +- clocks: phandle to the fixed xtal clock.
> +- clock-names: name of the fixed xtal clock.
> +- port: child port node of a tuner that defines the local and remote
> +  endpoints. The remote endpoint is assumed to be an SDR device
> +  that is capable of receiving the digital samples from the tuner.
> +
> +Optional properties:
> +--------------------
> +- maxim,slave	      : empty property indicates this is a slave of
> +			another master tuner. This is used to define two
> +			tuners in diversity mode (1 master, 1 slave). By
> +			default each tuner is an individual master.
> +- maxim,refout-load-pF: load capacitance value (in pF) on reference

Please add 'pF' to property-units.txt.

> +			output drive level. The possible load values are
> +			 0pF (default - refout disabled)
> +			10pF
> +			20pF
> +			30pF
> +			40pF
> +			60pF
> +			70pF
> +- maxim,am-hiz	      : empty property indicates AM Hi-Z filter path is
> +			selected for AM antenna input. By default this
> +			filter path is not used.
> +
> +Example:
> +--------
> +
> +Board specific DTS file
> +
> +/* Fixed XTAL clock node */
> +maxim_xtal: maximextal {

clock {

> +	compatible = "fixed-clock";
> +	#clock-cells = <0>;
> +	clock-frequency = <36864000>;
> +};
> +
> +/* A tuner device instance under i2c bus */
> +max2175_0: tuner@60 {
> +	compatible = "maxim,max2175";
> +	reg = <0x60>;
> +	clocks = <&maxim_xtal>;
> +	clock-names = "xtal";
> +	maxim,refout-load-pF = <10>;
> +
> +	port {
> +		max2175_0_ep: endpoint {
> +			remote-endpoint = <&slave_rx_v4l2_sdr_device>;

'v4l2' is not something that should appear in a DT.

> +		};
> +	};
> +
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Rob Herring @ 2016-11-14 19:41 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram
  Cc: mark.rutland, mchehab, hverkuil, sakari.ailus, crope,
	chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-3-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

On Wed, Nov 09, 2016 at 03:44:41PM +0000, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  .../devicetree/bindings/media/i2c/max2175.txt      |   61 +

It's preferred that bindings are a separate patch.

>  drivers/media/i2c/Kconfig                          |    4 +
>  drivers/media/i2c/Makefile                         |    2 +
>  drivers/media/i2c/max2175/Kconfig                  |    8 +
>  drivers/media/i2c/max2175/Makefile                 |    4 +
>  drivers/media/i2c/max2175/max2175.c                | 1558 ++++++++++++++++++++
>  drivers/media/i2c/max2175/max2175.h                |  108 ++
>  7 files changed, 1745 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/max2175.txt
>  create mode 100644 drivers/media/i2c/max2175/Kconfig
>  create mode 100644 drivers/media/i2c/max2175/Makefile
>  create mode 100644 drivers/media/i2c/max2175/max2175.c
>  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> new file mode 100644
> index 0000000..69f0dad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> @@ -0,0 +1,61 @@
> +Maxim Integrated MAX2175 RF to Bits tuner
> +-----------------------------------------
> +
> +The MAX2175 IC is an advanced analog/digital hybrid-radio receiver with
> +RF to Bits� front-end designed for software-defined radio solutions.
> +
> +Required properties:
> +--------------------
> +- compatible: "maxim,max2175" for MAX2175 RF-to-bits tuner.
> +- clocks: phandle to the fixed xtal clock.
> +- clock-names: name of the fixed xtal clock.
> +- port: child port node of a tuner that defines the local and remote
> +  endpoints. The remote endpoint is assumed to be an SDR device
> +  that is capable of receiving the digital samples from the tuner.
> +
> +Optional properties:
> +--------------------
> +- maxim,slave	      : empty property indicates this is a slave of
> +			another master tuner. This is used to define two
> +			tuners in diversity mode (1 master, 1 slave). By
> +			default each tuner is an individual master.
> +- maxim,refout-load-pF: load capacitance value (in pF) on reference

Please add 'pF' to property-units.txt.

> +			output drive level. The possible load values are
> +			 0pF (default - refout disabled)
> +			10pF
> +			20pF
> +			30pF
> +			40pF
> +			60pF
> +			70pF
> +- maxim,am-hiz	      : empty property indicates AM Hi-Z filter path is
> +			selected for AM antenna input. By default this
> +			filter path is not used.
> +
> +Example:
> +--------
> +
> +Board specific DTS file
> +
> +/* Fixed XTAL clock node */
> +maxim_xtal: maximextal {

clock {

> +	compatible = "fixed-clock";
> +	#clock-cells = <0>;
> +	clock-frequency = <36864000>;
> +};
> +
> +/* A tuner device instance under i2c bus */
> +max2175_0: tuner@60 {
> +	compatible = "maxim,max2175";
> +	reg = <0x60>;
> +	clocks = <&maxim_xtal>;
> +	clock-names = "xtal";
> +	maxim,refout-load-pF = <10>;
> +
> +	port {
> +		max2175_0_ep: endpoint {
> +			remote-endpoint = <&slave_rx_v4l2_sdr_device>;

'v4l2' is not something that should appear in a DT.

> +		};
> +	};
> +
> +};

^ permalink raw reply

* Re: [PATCH 1/6] mm: khugepaged: fix radix tree node leak in shmem collapse error path
From: Kirill A. Shutemov @ 2016-11-14 19:40 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Jan Kara, Andrew Morton, Linus Torvalds, linux-mm, linux-kernel,
	kernel-team
In-Reply-To: <20161114164822.GB5141@cmpxchg.org>

On Mon, Nov 14, 2016 at 11:48:22AM -0500, Johannes Weiner wrote:
> On Mon, Nov 14, 2016 at 10:52:50AM -0500, Johannes Weiner wrote:
> > On Mon, Nov 14, 2016 at 05:29:02PM +0300, Kirill A. Shutemov wrote:
> > > @@ -1400,7 +1400,9 @@ static void collapse_shmem(struct mm_struct *mm,
> > >  					PAGE_SIZE, 0);
> > >  
> > >  		spin_lock_irq(&mapping->tree_lock);
> > > -
> > > +		slot = radix_tree_lookup_slot(&mapping->page_tree, index);
> > > +		VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot,
> > > +					&mapping->tree_lock), page);
> > >  		VM_BUG_ON_PAGE(page_mapped(page), page);
> > 
> > That looks good to me. The slot may get relocated, but the content
> > shouldn't change with the page locked.
> > 
> > Are you going to send a full patch with changelog and sign-off? If so,
> > please add:
> > 
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> Just to clarify, this is in addition to my radix_tree_iter_next()
> change. The iterator still needs to be reloaded because the number of
> valid slots that come after the current one can change as well.

Could you just amend all these fixups into your patch?

-- 
 Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH 1/6] mm: khugepaged: fix radix tree node leak in shmem collapse error path
From: Kirill A. Shutemov @ 2016-11-14 19:40 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Jan Kara, Andrew Morton, Linus Torvalds, linux-mm, linux-kernel,
	kernel-team
In-Reply-To: <20161114164822.GB5141@cmpxchg.org>

On Mon, Nov 14, 2016 at 11:48:22AM -0500, Johannes Weiner wrote:
> On Mon, Nov 14, 2016 at 10:52:50AM -0500, Johannes Weiner wrote:
> > On Mon, Nov 14, 2016 at 05:29:02PM +0300, Kirill A. Shutemov wrote:
> > > @@ -1400,7 +1400,9 @@ static void collapse_shmem(struct mm_struct *mm,
> > >  					PAGE_SIZE, 0);
> > >  
> > >  		spin_lock_irq(&mapping->tree_lock);
> > > -
> > > +		slot = radix_tree_lookup_slot(&mapping->page_tree, index);
> > > +		VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot,
> > > +					&mapping->tree_lock), page);
> > >  		VM_BUG_ON_PAGE(page_mapped(page), page);
> > 
> > That looks good to me. The slot may get relocated, but the content
> > shouldn't change with the page locked.
> > 
> > Are you going to send a full patch with changelog and sign-off? If so,
> > please add:
> > 
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> Just to clarify, this is in addition to my radix_tree_iter_next()
> change. The iterator still needs to be reloaded because the number of
> valid slots that come after the current one can change as well.

Could you just amend all these fixups into your patch?

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] remote-curl: don't hang when a server dies before any output
From: Jeff King @ 2016-11-14 19:40 UTC (permalink / raw)
  To: David Turner; +Cc: git, spearce
In-Reply-To: <20161114182431.e7jjnq422c4xobdb@sigill.intra.peff.net>

On Mon, Nov 14, 2016 at 01:24:31PM -0500, Jeff King wrote:

>   2. Have remote-curl understand enough of the protocol that it can
>      abort rather than hang.
> 
>      I think that's effectively the approach of your patch, but for one
>      specific case. But could we, for example, make sure that everything
>      we proxy is a complete set of pktlines and ends with a flush? And
>      if not, then we hang up on fetch-pack.
> 
>      I _think_ that would work, because even the pack is always encased
>      in pktlines for smart-http.

So something like this. It turned out to be a lot uglier than I had
hoped because we get fed the data from curl in odd-sized chunks, so we
need a state machine.

But it does seem to work. At least it doesn't seem to break anything in
the test suite, and it fixes the new tests you added. I'd worry that
there's some obscure case where the response isn't packetized in the
same way.

---
diff --git a/remote-curl.c b/remote-curl.c
index f14c41f4c..605357d77 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -403,6 +403,18 @@ struct rpc_state {
 	struct strbuf result;
 	unsigned gzip_request : 1;
 	unsigned initial_buffer : 1;
+
+	enum {
+		RPC_PKTLINE_ERROR, /* bogus hex chars in length */
+		RPC_PKTLINE_INITIAL, /* no packets received yet */
+		RPC_PKTLINE_1, /* got one hex char */
+		RPC_PKTLINE_2, /* got two hex chars */
+		RPC_PKTLINE_3, /* got three hex chars */
+		RPC_PKTLINE_DATA, /* reading data; pktline_len holds remaining */
+		RPC_PKTLINE_END_OF_PACKET, /* last packet completed */
+		RPC_PKTLINE_FLUSH, /* last packet was flush */
+	} pktline_state;
+	size_t pktline_len;
 };
 
 static size_t rpc_out(void *ptr, size_t eltsize,
@@ -451,11 +463,77 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
 }
 #endif
 
+static void update_pktline_state(struct rpc_state *rpc,
+				 const char *buf, size_t len)
+{
+#define READ_ONE_HEX(shift) do { \
+	int val = hexval(buf[0]); \
+	if (val < 0) { \
+		warning("error on %d", *buf); \
+		rpc->pktline_state = RPC_PKTLINE_ERROR; \
+		return; \
+	} \
+	rpc->pktline_len |= val << shift; \
+	buf++; \
+	len--; \
+} while(0)
+
+	while (len > 0) {
+		switch (rpc->pktline_state) {
+		case RPC_PKTLINE_ERROR:
+			/* previous error; there is no recovery */
+			return;
+
+		/* We can start a new pktline at any of these states */
+		case RPC_PKTLINE_INITIAL:
+		case RPC_PKTLINE_FLUSH:
+		case RPC_PKTLINE_END_OF_PACKET:
+			rpc->pktline_len = 0;
+			READ_ONE_HEX(12);
+			rpc->pktline_state = RPC_PKTLINE_1;
+			break;
+
+		case RPC_PKTLINE_1:
+			READ_ONE_HEX(8);
+			rpc->pktline_state = RPC_PKTLINE_2;
+			break;
+
+		case RPC_PKTLINE_2:
+			READ_ONE_HEX(4);
+			rpc->pktline_state = RPC_PKTLINE_3;
+			break;
+
+		case RPC_PKTLINE_3:
+			READ_ONE_HEX(0);
+			if (rpc->pktline_len) {
+				rpc->pktline_state = RPC_PKTLINE_DATA;
+				rpc->pktline_len -= 4;
+			} else
+				rpc->pktline_state = RPC_PKTLINE_FLUSH;
+			break;
+
+		case RPC_PKTLINE_DATA:
+			if (len < rpc->pktline_len) {
+				rpc->pktline_len -= len;
+				len = 0;
+			} else {
+				buf += rpc->pktline_len;
+				len -= rpc->pktline_len;
+				rpc->pktline_len = 0;
+				rpc->pktline_state = RPC_PKTLINE_END_OF_PACKET;
+			}
+			break;
+		}
+	}
+#undef READ_ONE_HEX
+}
+
 static size_t rpc_in(char *ptr, size_t eltsize,
 		size_t nmemb, void *buffer_)
 {
 	size_t size = eltsize * nmemb;
 	struct rpc_state *rpc = buffer_;
+	update_pktline_state(rpc, ptr, size);
 	write_or_die(rpc->in, ptr, size);
 	return size;
 }
@@ -659,6 +737,8 @@ static int post_rpc(struct rpc_state *rpc)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
 	curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
 
+	rpc->pktline_state = RPC_PKTLINE_INITIAL;
+
 	err = run_slot(slot, NULL);
 	if (err == HTTP_REAUTH && !large_request) {
 		credential_fill(&http_auth);
@@ -667,6 +747,11 @@ static int post_rpc(struct rpc_state *rpc)
 	if (err != HTTP_OK)
 		err = -1;
 
+	if (rpc->pktline_state != RPC_PKTLINE_FLUSH) {
+		error("invalid or truncated response from http server");
+		err = -1;
+	}
+
 	curl_slist_free_all(headers);
 	free(gzip_body);
 	return err;

^ permalink raw reply related

* Re: [PATCH v3 01/11] mwifiex: check tx_hw_pending before downloading sleep confirm
From: Brian Norris @ 2016-11-14 19:40 UTC (permalink / raw)
  To: Amitkumar Karwar, Kalle Valo
  Cc: linux-wireless, Cathy Luo, Nishant Sarmukadam, rajatja,
	dmitry.torokhov, Shengzhen Li
In-Reply-To: <1478869818-4340-1-git-send-email-akarwar@marvell.com>

Hi Amit, Kalle,

On Fri, Nov 11, 2016 at 06:40:08PM +0530, Amitkumar Karwar wrote:
> From: Shengzhen Li <szli@marvell.com>
> 
> We may get SLEEP event from firmware even if TXDone interrupt
> for last Tx packet is still pending. In this case, we may
> end up accessing PCIe memory for handling TXDone after power
> save handshake is completed. This causes kernel crash with
> external abort.
> 
> This patch will only allow downloading sleep confirm
> when no tx done interrupt is pending in the hardware.
> 
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Shengzhen Li <szli@marvell.com>
> Tested-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> ---
> v2: address format issues(Brain)
> RESEND v2(Applicable for complete patch series):
>     1) Fixed syntax issue "changelog not placed after the  Sign-offs"
>        pointed by Brian.
>     2) Dropped "[v2,03/12] mwifiex: don't do unbalanced free()'ing in
>        cleanup_if()" patch in this series. It was already sent by Brian
>        separately.
> v3: Same as RESEND v2.
> 
> Hi Kalle,
> 
> There are multiple mwifiex patches under review. I want you consider them
> in following sequence(first being oldest) to avoid conflicts

Thanks for doing this! It's a little confusing about what's outstanding
at the moment (and I think I was just confused on a review a bit ago; I
wasn't 100% sure what it was based on), so this listing helps.

If it helps, I'll put my comments here, since I've reviewed most of
these:

> [v3] mwifiex: report wakeup for wowlan

Reviewed, SGMT.

> mwifiex: add power save parameters in hs_cfg cmd

Didn't review. No comment.

> [2/2] mwifiex: ignore calibration data failure (Note: 1/2 has dropped)

Didn't review. But FWIW, Kalle expressed a preference for full series,
not partial.

> [v6] mwifiex: parse device tree node for PCIe

This one is marked Deferred in patchwork, and I had some comments about
it, since it introduced a double-free issue. I'd prefer it get fixed and
resent, and I expect Kalle is also waiting for this.

> [v2,1/3] mwifiex: Allow mwifiex early access to device structure
> [v2,2/3] mwifiex: Introduce mwifiex_probe_of() to parse common properties
> [v2,3/3] mwifiex: Enable WoWLAN for both sdio and pcie

You sent v3 for the above, and those LGTM (I provided my review). I was
probably also confused because they were based on the above "[v6]
mwifiex: parse device tree node for PCIe", which was not completely
correct.

> mwifiex: don't do unbalanced free()'ing in cleanup_if()
> mwifiex: printk() overflow with 32-byte SSIDs
> mwifiex: fix memory leak in mwifiex_save_hidden_ssid_channels()

I wrote or reviewed the above 3. LGTM.

> [v3,01/11] mwifiex: check tx_hw_pending before downloading sleep confirm
> [v3,02/11] mwifiex: complete blocked power save handshake in main process
> [v3,03/11] mwifiex: resolve races between async FW init (failure) and device removal
> [v3,04/11] mwifiex: remove redundant pdev check in suspend/resume handlers
> [v3,05/11] mwifiex: don't pretend to resume while remove()'ing
> [v3,06/11] mwifiex: resolve suspend() race with async FW init failure
> [v3,07/11] mwifiex: reset card->adapter during device unregister
> [v3,08/11] mwifiex: usb: handle HS failures
> [v3,09/11] mwifiex: sdio: don't check for NULL sdio_func
> [v3,10/11] mwifiex: stop checking for NULL drvata/intfdata
> [v3,11/11] mwifiex: pcie: stop checking for NULL adapter->card

For this entire series, I looked over them again (and I wrote several in
the first place), so for all 11:

Reviewed-by: Brian Norris <briannorris@chromium.org>

> ---
>  drivers/net/wireless/marvell/mwifiex/cmdevt.c | 5 +++--
>  drivers/net/wireless/marvell/mwifiex/init.c   | 1 +
>  drivers/net/wireless/marvell/mwifiex/main.h   | 1 +
>  drivers/net/wireless/marvell/mwifiex/pcie.c   | 5 +++++
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
[...] 

^ permalink raw reply

* [PATCH] gstreamer1.0-plugins-bad: Add imxgpu2d override to SRC_URI
From: Fabio Berton @ 2016-11-14 19:38 UTC (permalink / raw)
  To: meta-freescale

We can't append SRC_URI here without using an override because it add
patches for all machines, including non imx machines, e.g QEMU machines.

Signed-off-by: Fabio Berton <fabio.berton@ossystems.com.br>
---
 recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
index 55fc760..7f321cc 100644
--- a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
+++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
@@ -10,7 +10,7 @@ PACKAGECONFIG_GL_imxgpu2d = "${@bb.utils.contains('DISTRO_FEATURES', 'opengl x11
 PACKAGECONFIG_GL_imxgpu3d = "${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gles2', '', d)}"
 
 
-SRC_URI_append = " \
+SRC_URI_append_imxgpu2d = " \
     file://0001-glplugin-Change-wayland-default-res-to-1024x768.patch \
     file://0002-Support-fb-backend-for-gl-plugins.patch \
     file://0003-Add-directviv-to-glimagesink-to-improve-playback-per.patch \
-- 
2.1.4



^ permalink raw reply related

* Re: Debugging Ethernet issues
From: Florian Fainelli @ 2016-11-14 19:19 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Sebastian Frias, Mason, Andrew Lunn, netdev, Sergei Shtylyov,
	Tom Lendacky, Zach Brown, Shaohui Xie, Tim Beale, Brian Hill,
	Vince Bridgers, Balakumaran Kannan, David S. Miller,
	Kirill Kapranov
In-Reply-To: <yw1xpolxga3o.fsf@unicorn.mansr.com>

On 11/14/2016 11:00 AM, Måns Rullgård wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> On 11/14/2016 10:20 AM, Florian Fainelli wrote:
>>> On 11/14/2016 09:59 AM, Sebastian Frias wrote:
>>>> On 11/14/2016 06:32 PM, Florian Fainelli wrote:
>>>>> On 11/14/2016 07:33 AM, Mason wrote:
>>>>>> On 14/11/2016 15:58, Mason wrote:
>>>>>>
>>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>>>>> vs
>>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>>>>
>>>>>>> I'm not sure whether "flow control" is relevant...
>>>>>>
>>>>>> Based on phy_print_status()
>>>>>> phydev->pause ? "rx/tx" : "off"
>>>>>> I added the following patch.
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>>>>>> index defc22a15f67..4e758c1cfa4e 100644
>>>>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>>>>> @@ -667,6 +667,8 @@ static void nb8800_link_reconfigure(struct net_device *dev)
>>>>>>         struct phy_device *phydev = priv->phydev;
>>>>>>         int change = 0;
>>>>>>  
>>>>>> +       printk("%s from %pf\n", __func__, __builtin_return_address(0));
>>>>>> +
>>>>>>         if (phydev->link) {
>>>>>>                 if (phydev->speed != priv->speed) {
>>>>>>                         priv->speed = phydev->speed;
>>>>>> @@ -1274,9 +1276,9 @@ static int nb8800_hw_init(struct net_device *dev)
>>>>>>         nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
>>>>>>  
>>>>>>         /* Auto-negotiate by default */
>>>>>> -       priv->pause_aneg = true;
>>>>>> -       priv->pause_rx = true;
>>>>>> -       priv->pause_tx = true;
>>>>>> +       priv->pause_aneg = false;
>>>>>> +       priv->pause_rx = false;
>>>>>> +       priv->pause_tx = false;
>>>>>>  
>>>>>>         nb8800_mc_init(dev, 0);
>>>>>>  
>>>>>>
> 
> [...]
> 
>>>>> And the time difference is clearly accounted for auto-negotiation time
>>>>> here, as you can see it takes about 3 seconds for Gigabit Ethernet to
>>>>> auto-negotiate and that seems completely acceptable and normal to me
>>>>> since it is a more involved process than lower speeds.
>>>>>
>>>>>>
>>>>>>
>>>>>> OK, so now it works (by accident?) even on 100 Mbps switch, but it still
>>>>>> prints "flow control rx/tx"...
>>>>>
>>>>> Because your link partner advertises flow control, and that's what
>>>>> phydev->pause and phydev->asym_pause report (I know it's confusing, but
>>>>> that's what it is at the moment).
>>>>
>>>> Thanks.
>>>> Could you confirm that Mason's patch is correct and/or that it does not
>>>> has negative side-effects?
>>>
>>> The patch is not correct nor incorrect per-se, it changes the default
>>> policy of having pause frames advertised by default to not having them
>>> advertised by default.
> 
> I was advised to advertise flow control by default back when I was
> working on the driver, and I think it makes sense to do so.
> 
>>> This influences both your Ethernet MAC and the link partner in that
>>> the result is either flow control is enabled (before) or it is not
>>> (with the patch). There must be something amiss if you see packet
>>> loss or some kind of problem like that with an early exchange such as
>>> DHCP. Flow control tend to kick in under higher packet rates (at
>>> least, that's what you expect).
>>>
>>>>
>>>> Right now we know that Mason's patch makes this work, but we do not
>>>> understand why nor its implications.
>>>
>>> You need to understand why, right now, the way this problem is
>>> presented, you came up with a workaround, not with the root cause or the
>>> solution. What does your link partner (switch?) reports, that is, what
>>> is the ethtool output when you have a link up from  your nb8800 adapter?
>>
>> Actually, nb8800_pause_config() seems to be doing a complete MAC/DMA
>> reconfiguration when pause frames get auto-negotiated while the link is
>> UP,
> 
> This is due to a silly hardware limitation.  The register containing the
> flow control bits can't be written while rx is enabled.

You do a DMA stop, but you don't disable the MAC receiver unlike what
nb8800_stop() does, why is not calling nb8800_mac_rx() necessary here?

> 
>> and it does not differentiate being called from
>> ethtool::set_pauseparam or the PHYLIB adjust_link callback (which it
>> probably should),
> 
> Differentiate how?

Differentiate in that when you are called from adjust_link, why bother
checking with netif_running() since you are only configuring the pause
settings when phydev->link is set. Not that this matters much, but
that's something the caller can tell you.

> 
>> wondering if there is a not a remote chance you can get the reply to
>> arrive right when you just got signaled a link UP?
> 
> If you're attempting to send or receive things before you get the link
> up notification, you shouldn't expect anything to work reliably.

No kidding.
-- 
Florian

^ permalink raw reply

* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Karthik Nayak @ 2016-11-14 19:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <xmqq60nqzuye.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 14, 2016 at 7:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>>>> index 600b703..f4ad297 100644
>>>> --- a/Documentation/git-for-each-ref.txt
>>>> +++ b/Documentation/git-for-each-ref.txt
>>>> @@ -96,7 +96,9 @@ refname::
>>>>         slash-separated path components from the front of the refname
>>>>         (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
>>>>         `<N>` must be a positive integer.  If a displayed ref has fewer
>>>> -       components than `<N>`, the command aborts with an error.
>>>> +       components than `<N>`, the command aborts with an error. For the base
>>>> +       directory of the ref (i.e. foo in refs/foo/bar/boz) append
>>>> +       `:base`. For the entire directory path append `:dir`.
>
> Sorry that I missed this so far and I do not know how many recent
> rerolls had them like this, but I am not sure about these :base and
> :dir suffixes.  From their names I think readers would expect that
> they do rough equivalents to basename() and dirname() applied to the
> refname, but the example contradicts with that intuition.  The
> result of applying basename() to 'refs/boo/bar/boz' would be 'boz'
> and not 'foo' as the example says.
>

True that the options ':dir' and ':base' seem to be conflicting with
the use case
of basename() and dirname(), These were names which I thought best fit
the options
with no relation to basename() and dirname(). But now that you mention
it, it would
make sense to change these names to something more suitable, any suggestions
would be welcome.

> So assuming that :base and :dir are unrelated to basename() and
> dirname():
>
>  - I think calling these :base and :dir may be misleading
>
>  - More importantly, what do these do?  I do not think of a good
>    description that generalizes "base of refs/foo/bar/boz is foo" to
>    explain your :base.

$ ./git for-each-ref --format "%(refname)%(end) %(refname:dir)"
refs/heads/master                  refs/heads
refs/heads/ref-filter                refs/heads
refs/remotes/junio/va/i18n     refs/remotes/junio/va

$ ./git for-each-ref  refs/heads --format
"%(align:left,30)%(refname)%(end) %(refname:base)"
refs/heads/master                 heads
refs/heads/ref-filter                heads
refs/remotes/junio/va/i18n     remotes

I guess this should clear it up.

>
>  - A :dir that corresponds to the :base that picks 'foo' from
>    'refs/foo/bar/boz' needs an example, too.
>

I could add in an example.

> Or is the above example simply a typo?  Is refs/foo/bar/boz:base
> 'boz', not 'foo'?
>
>

Not a typo, but open to changes in name.

-- 
Regards,
Karthik Nayak

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