Re: [PATCH] dt-bindings: Drop kernel copy of common reserved-memory bindings

[Simon Glass]

On Fri, 13 Oct 2023 at 13:45, Rob Herring  wrote:
>
> The common reserved-memory bindings have recently been copied from the
> kernel tree into dtschema. The preference is to host common, stable
> bindings in dtschema. As reserved-memory is documented in the DT Spec,
> it meets the criteria.
>
> The v2023.09 version of dtschema is what contains the reserved-memory
> schemas we depend on, so bump the minimum version to that. Otherwise,
> references to these schemas will generate errors.
>
> Signed-off-by: Rob Herring 
> ---
>  Documentation/devicetree/bindings/Makefile|   2 +-
>  .../remoteproc/renesas,rcar-rproc.yaml|   2 +-
>  .../bindings/reserved-memory/framebuffer.yaml |  52 -
>  .../reserved-memory/memory-region.yaml|  40 
>  .../reserved-memory/reserved-memory.txt   |   2 +-
>  .../reserved-memory/reserved-memory.yaml  | 181 --
>  .../reserved-memory/shared-dma-pool.yaml  |  97 --
>  .../bindings/sound/mediatek,mt8188-afe.yaml   |   2 +-
>  8 files changed, 4 insertions(+), 374 deletions(-)
>  delete mode 100644 
> Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
>  delete mode 100644 
> Documentation/devicetree/bindings/reserved-memory/memory-region.yaml
>  delete mode 100644 
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
>  delete mode 100644 
> Documentation/devicetree/bindings/reserved-memory/shared-dma-pool.yaml
>

Reviewed-by: Simon Glass 

Re: [SPECIFICATION RFC] The firmware and bootloader log specification

[Simon Glass]

Hi Daniel,

On Fri, 13 Nov 2020 at 19:07, Daniel Kiper  wrote:
>
> Hey,
>
> This is next attempt to create firmware and bootloader log specification.
> Due to high interest among industry it is an extension to the initial
> bootloader log only specification. It takes into the account most of the
> comments which I got up until now.
>
> The goal is to pass all logs produced by various boot components to the
> running OS. The OS kernel should expose these logs to the user space
> and/or process them internally if needed. The content of these logs
> should be human readable. However, they should also contain the
> information which allows admins to do e.g. boot time analysis.
>
> The log specification should be as much as possible platform agnostic
> and self contained. The final version of this spec should be merged into
> existing specifications, e.g. UEFI, ACPI, Multiboot2, or be a standalone
> spec, e.g. as a part of OASIS Standards. The former seems better but is
> not perfect too...
>
> Here is the description (pseudocode) of the structures which will be
> used to store the log data.
>
>   struct bf_log
>   {
> uint32_t   version;
> char   producer[64];
> uint64_t   flags;
> uint64_t   next_bf_log_addr;
> uint32_t   next_msg_off;
> bf_log_msg msgs[];
>   }
>
>   struct bf_log_msg
>   {
> uint32_t size;
> uint64_t ts_nsec;
> uint32_t level;
> uint32_t facility;
> uint32_t msg_off;
> char strings[];
>   }
>
> The members of struct bf_log:
>   - version: the firmware and bootloader log format version number, 1 for now,
>   - producer: the producer/firmware/bootloader/... type; the length
> allows ASCII UUID storage if somebody needs that functionality,
>   - flags: it can be used to store information about log state, e.g.
> it was truncated or not (does it make sense to have an information
> about the number of lost messages?),
>   - next_bf_log_addr: address of next bf_log struct; none if zero (I think
> newer spec versions should not change anything in first 5 bf_log members;
> this way older log parsers will be able to traverse/copy all logs 
> regardless
> of version used in one log or another),
>   - next_msg_off: the offset, in bytes, from the beginning of the bf_log 
> struct,
> of the next byte after the last log message in the msgs[]; i.e. the offset
> of the next available log message slot; it is equal to the total size of
> the log buffer including the bf_log struct,
>   - msgs: the array of log messages,
>   - should we add CRC or hash or signatures here?
>
> The members of struct bf_log_msg:
>   - size: total size of bf_log_msg struct,
>   - ts_nsec: timestamp expressed in nanoseconds starting from 0,
>   - level: similar to syslog meaning; can be used to differentiate normal 
> messages
> from debug messages; the exact interpretation depends on the current 
> producer
> type specified in the bf_log.producer,
>   - facility: similar to syslog meaning; can be used to differentiate the 
> sources of
> the messages, e.g. message produced by networking module; the exact 
> interpretation
> depends on the current producer type specified in the bf_log.producer,
>   - msg_off: the log message offset in strings[],
>   - strings[0]: the beginning of log message type, similar to the facility 
> member but
> NUL terminated string instead of integer; this will be used by, e.g., the 
> GRUB2
> for messages printed using grub_dprintf(),
>   - strings[msg_off]: the beginning of log message, NUL terminated string.
>
> Note: The producers are free to use/ignore any given set of level, facility 
> and/or
>   log type members. Though the usage of these members has to be clearly 
> defined.
>   Ignored integer members should be set to 0. Ignored log message type 
> should
>   contain an empty NUL terminated string. The log message is mandatory 
> but can
>   be an empty NUL terminated string.
>
> There is still not fully solved problem how the logs should be presented to 
> the OS.
> On the UEFI platforms we can use config tables to do that. Then probably
> bf_log.next_bf_log_addr should not be used. On the ACPI and Device Tree 
> platforms
> we can use these mechanisms to present the logs to the OSes. The situation 
> gets more
> difficult if neither of these mechanisms are present. However, maybe we 
> should not
> bother too much about that because probably these platforms getting less and 
> less
> common.
>
> Anyway, I am aware that this is not specification per se. The goal of this 
> email is
> to continue the discussion about the idea of the firmware and booloader log 
> and to
> find out where the final specification should land. Of course taking into the 
> account
> assumptions made above.
>
> You can find previous discussions about related topics at [1], [2] and [3].
>
> Additionally, I am going to present this during GRUB mini-summit session on 
> Tuesday,
> 17th of 

Re: [PATCH 1/3] platform/chrome: cros_ec_spi: Don't overwrite spi::mode

[Simon Glass]

On Fri, 4 Dec 2020 at 12:35, Stephen Boyd  wrote:
>
> There isn't any need to overwrite the mode here in the driver with what
> has been detected by the firmware, such as DT or ACPI. In fact, if we
> use the SPI CS gpio descriptor feature we will overwrite the mode with
> SPI_MODE_0 where it already contains SPI_MODE_0 and more importantly
> SPI_CS_HIGH. Clearing the SPI_CS_HIGH bit causes the CS line to toggle
> when the device is probed when it shouldn't change, confusing the driver
> and making it fail to probe. Drop the assignment and let the spi core
> take care of it.
>
> Fixes: a17d94f0b6e1 ("mfd: Add ChromeOS EC SPI driver")
> Cc: Simon Glass 
> Cc: Gwendal Grignou 
> Reviewed-by: Douglas Anderson 
> Tested-by: Douglas Anderson 
> Acked-by: Enric Balletbo i Serra 
> Cc: Alexandru M Stan 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/platform/chrome/cros_ec_spi.c | 1 -
>  1 file changed, 1 deletion(-)


Reviewed-by: Simon Glass 


>
>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c 
> b/drivers/platform/chrome/cros_ec_spi.c
> index dfa1f816a45f..f9df218fc2bb 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -742,7 +742,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
> int err;
>
> spi->bits_per_word = 8;
> -   spi->mode = SPI_MODE_0;
> spi->rt = true;
> err = spi_setup(spi);
> if (err < 0)
> --
> https://chromeos.dev
>

Re: [PATCH 2/3] platform/chrome: cros_ec_spi: Drop bits_per_word assignment

[Simon Glass]

On Fri, 4 Dec 2020 at 12:35, Stephen Boyd  wrote:
>
> This is already handed by default in spi_setup() if the bits_per_word is
> 0, so just drop it to shave off a line.
>
> Cc: Simon Glass 
> Cc: Gwendal Grignou 
> Reviewed-by: Douglas Anderson 
> Tested-by: Douglas Anderson 
> Acked-by: Enric Balletbo i Serra 
> Cc: Alexandru M Stan 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/platform/chrome/cros_ec_spi.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Simon Glass 


>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c 
> b/drivers/platform/chrome/cros_ec_spi.c
> index f9df218fc2bb..14c4046fa04d 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -741,7 +741,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
> struct cros_ec_spi *ec_spi;
> int err;
>
> -   spi->bits_per_word = 8;
> spi->rt = true;
> err = spi_setup(spi);
> if (err < 0)
> --
> https://chromeos.dev
>

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Wed, 17 Jun 2020 at 13:15, Nicolas Saenz Julienne
 wrote:
>
> On Tue, 2020-06-16 at 17:31 -0600, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
> >  wrote:
> > > On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> > > > Hi Nicolas,
> > > >
> > > > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> > > >  wrote:
> > > > > There is no distinction in DT between the PCI controller device and 
> > > > > the
> > > > > root bridge, whereas such distinction exists from dm's perspective. 
> > > > > Make
> > > > > sure the root bridge ofnode is assigned to the controller's platform
> > > > > device node.
> > > > >
> > > > > This permits setups like this to work correctly:
> > > > >
> > > > > pcie {
> > > > > compatible = "...";
> > > > > ...
> > > > > dev {
> > > > > reg = <0 0 0 0 0>;
> > > > > ...
> > > > > };
> > > > > };
> > > > >
> > > > > Without this the dev node is assigned to the root bridge and the
> > > > > actual device search starts one level lower than expected.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne 
> > > > > ---
> > > > >  drivers/pci/pci-uclass.c | 15 ++-
> > > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > Can you update the tests to handle this case please?
> > >
> > > I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give
> > > me
> > > some pointers on where/how to test this?
> > >
> >
> > Yes it is at test/dm/pci.c and the device tree is test.dts
> >
> > 'make qcheck' to run all tests. To run one test, build for sandbox and
> > then something like
> >
> > u-boot -T -c "ut dm pci_swapcase"
> >
> > for example.
> >
> > You can perhaps use an existing PCI controller in test.dts but feel
> > free to add one more if you need it for your test. Make sure that you
> > don't break other tests.
>
> Thanks for the info.
>
> Actually adding the tests made me doubleguess myself, and now I'm pretty sure
> that what I shoudl've done in DT is the following:
>
>  {
>pci@0 {
>#address-cells = <3>;
>#size-cells = <2>;
>ranges;
>
>reg = <0 0 0 0 0>;
>
>usb@1,0 {
>reg = <0x1 0 0 0 0>;
>resets = < 
> RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
>};
>};
> };
>
>
> (with "lspci -tv": [:00]---00.0-[01]00.0  VIA Technologies, Inc. 
> VL805 USB 3.0 Host Controller)
>
> With this the patch above isn't needed, which is great.
>
> I'll send this to upstream Linux just to get a confirmation this is correct,
> although if you have any comments it'll be appreciated.

Yes it looks OK to me. U-Boot allows PCI and USB devices to be
represented in the device tree.

Regards,
Simon

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Tue, 16 Jun 2020 at 08:09, Nicolas Saenz Julienne
 wrote:
>
> On Tue, 2020-06-16 at 07:43 -0600, Simon Glass wrote:
> > Hi Nicolas,
> >
> > On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
> >  wrote:
> > > There is no distinction in DT between the PCI controller device and the
> > > root bridge, whereas such distinction exists from dm's perspective. Make
> > > sure the root bridge ofnode is assigned to the controller's platform
> > > device node.
> > >
> > > This permits setups like this to work correctly:
> > >
> > > pcie {
> > > compatible = "...";
> > > ...
> > > dev {
> > > reg = <0 0 0 0 0>;
> > > ...
> > > };
> > > };
> > >
> > > Without this the dev node is assigned to the root bridge and the
> > > actual device search starts one level lower than expected.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne 
> > > ---
> > >  drivers/pci/pci-uclass.c | 15 ++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > Can you update the tests to handle this case please?
>
> I'd be glad to, but I'm not familiar with the test FW in u-booy, coud give me
> some pointers on where/how to test this?
>

Yes it is at test/dm/pci.c and the device tree is test.dts

'make qcheck' to run all tests. To run one test, build for sandbox and
then something like

u-boot -T -c "ut dm pci_swapcase"

for example.

You can perhaps use an existing PCI controller in test.dts but feel
free to add one more if you need it for your test. Make sure that you
don't break other tests.

Regards,
Simon

Re: [PATCH v4 4/5] dm: pci: Assign controller device node to root bridge

[Simon Glass]

Hi Nicolas,

On Fri, 12 Jun 2020 at 10:47, Nicolas Saenz Julienne
 wrote:
>
> There is no distinction in DT between the PCI controller device and the
> root bridge, whereas such distinction exists from dm's perspective. Make
> sure the root bridge ofnode is assigned to the controller's platform
> device node.
>
> This permits setups like this to work correctly:
>
> pcie {
> compatible = "...";
> ...
> dev {
> reg = <0 0 0 0 0>;
> ...
> };
> };
>
> Without this the dev node is assigned to the root bridge and the
> actual device search starts one level lower than expected.
>
> Signed-off-by: Nicolas Saenz Julienne 
> ---
>  drivers/pci/pci-uclass.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Can you update the tests to handle this case please?

Regards,
Simon

Re: [PATCH v3] patman: Use the Change-Id, version, and prefix in the Message-Id

[Simon Glass]

On Fri, 27 Sep 2019 at 10:25, Douglas Anderson  wrote:
>
> As per the centithread on ksummit-discuss [1], there are folks who
> feel that if a Change-Id is present in a developer's local commit that
> said Change-Id could be interesting to include in upstream posts.
> Specifically if two commits are posted with the same Change-Id there's
> a reasonable chance that they are either the same commit or a newer
> version of the same commit.  Specifically this is because that's how
> gerrit has trained people to work.
>
> There is much angst about Change-Id in upstream Linux, but one thing
> that seems safe and non-controversial is to include the Change-Id as
> part of the string of crud that makes up a Message-Id.
>
> Let's give that a try.
>
> In theory (if there is enough adoption) this could help a tool more
> reliably find various versions of a commit.  This actually might work
> pretty well for U-Boot where (I believe) quite a number of developers
> use patman, so there could be critical mass (assuming that enough of
> these people also use a git hook that adds Change-Id to their
> commits).  I was able to find this git hook by searching for "gerrit
> change id git hook" in my favorite search engine.
>
> In theory one could imagine something like this could be integrated
> into other tools, possibly even git-send-email.  Getting it into
> patman seems like a sane first step, though.
>
> NOTE: this patch is being posted using a patman containing this patch,
> so you should be able to see the Message-Id of this patch and see that
> it contains my local Change-Id, which ends in 2b9 if you want to
> check.
>
> [1] 
> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2019-August/006739.html
>
> Signed-off-by: Douglas Anderson 
> ---
>
> Changes in v3:
> - Allow faking the time of Message-Id for testing (testBasic)
> - Add skip_blank for Change-Id (testBasic).
> - Check the Message-Id in testBasic.

Applied to u-boot-dm/next, thanks!

>
> Changes in v2:
> - Add a "v" before the version part of the Message-Id
> - Reorder the parts of the Message-Id as per Johannes.
>
>  tools/patman/README |  8 -
>  tools/patman/commit.py  |  3 ++
>  tools/patman/patchstream.py | 64 +++--
>  tools/patman/test.py| 15 +++--
>  4 files changed, 85 insertions(+), 5 deletions(-)

Re: [PATCH] RFC: Example schema files written in Python

[Simon Glass]

Hi Rob,

On Wed, 8 May 2019 at 13:21, Rob Herring  wrote:
>
> On Mon, Apr 29, 2019 at 5:41 PM Simon Glass  wrote:
> >
> > Most of these are hand-written, but xilinx-xadc.py is auto-generated by
> > binding_to_py.py as an example of the use of that tool.
> >
> > This is part of a proof-of-concept device-tree validator. See the patch
> > on the dtc mailing list for details:
>
> Honestly, we are pretty far down the path of using json-schema to
> consider changing to something else. We've already gone thru plenty of
> concepts over the years with different languages for the schema.

I don't think I saw much of that. I did hear that others has suggested
such options but the yaml/json design is the only one I'm aware of.

Anyway, it sounds like things are pretty set in stone right now. Even
so, I'll reply to this email.

>
> While I think there are some cases where being able to do schema with
> code is useful or necessary, the vast majority of cases can be handled
> just fine with structured data. I'd rather see how we could augment
> the data with code. Maybe that's snippets of code within the schema or
> making the validation code more modular. I would like to see the dtc
> checks infrastructure be extendable without modifying dtc. That could
> include supporting checks written in python.
>
> One example where we need more than just schema data is validating
> properties that depend on a provider #.*-cells property. We can't
> really do that with json-schema. At least the number of cells being
> correct is covered by dtc already. So it would really be how do we
> validate the cell data itself. OTOH, I think that is pretty far down
> the list in priorities of things to validate. There's already
> thousands of warnings generated by dtc and the json-schema which are
> slow to get fixed (though some are really subjective and more what to
> avoid for new users).
>
> >
> >RFC: Python-based device-tree validation
> >
> > Signed-off-by: Simon Glass 
> > ---
>
> I'll use this one to comment on. Comments are most around goals for
> the binding doc format.
>
> > diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py 
> > b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py
> > new file mode 100644
> > index 0..9f55f48f7cde7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +
> > +# Xilinx XADC device driver
>
> Having some defined structure at the top-level is beneficial for
> extracting data and automating review checks.

What does this refer to?

>
> > +
> > +from kschema import NodeDesc, PropBool, PropClocks, PropInt, PropIntList, 
> > PropInterrupts, PropReg, PropStringList
> > +
> > +schema = [
> > +NodeDesc('xilinx-xadc', ['xlnx,zynq-xadc-1.00.a', 
> > 'xlnx,axi-xadc-1.00.a'], False, desc=
>
> If one desires to generate a list of all possible compatible strings
> (to find undocumented ones), how would you do that?

Read in all the schema files and then walk through the entire schema
looking for compatible strings.

>
> > +'This binding document describes the bindings for both of them 
> > since the'
> > +'bindings are very similar. The Xilinx XADC is a ADC that can 
> > be found in the'
> > +'series 7 FPGAs from Xilinx. The XADC has a DRP interface for 
> > communication.'
> > +'Currently two different frontends for the DRP interface 
> > exist. One that is only'
> > +'available on the ZYNQ family as a hardmacro in the SoC 
> > portion of the ZYNQ. The'
> > +'other one is available on all series 7 platforms and is a 
> > softmacro with a AXI'
> > +'interface. This binding document describes the bindings for 
> > both of them since'
> > +'the bindings are very similar.', elements=[
>
> One goal with the schema (at least core ones) is to generate
> documentation from it. That would need to be a format such as rST so
> we can have formatting. And we'd want to be able to parse the
> properties and generate tables from them.

The docs above are taken verbatim from the binding, so there is no
formatting really, except for blank lines.1

>
> If someone really gets an itch, we'll rewrite sections of the DT spec
> in schema.
>
> > +PropReg(required=True,
> > +desc='Address and length of the register set for the device'),
>
> For any standard property, we'd have to create the class before
> bindings can use it.

There is a 'generic' property (Pr

[PATCH] RFC: Example schema files written in Python

[Simon Glass]

Most of these are hand-written, but xilinx-xadc.py is auto-generated by
binding_to_py.py as an example of the use of that tool.

This is part of a proof-of-concept device-tree validator. See the patch
on the dtc mailing list for details:

   RFC: Python-based device-tree validation

Signed-off-by: Simon Glass 
---

 Documentation/__init__.py |   0
 Documentation/devicetree/__init__.py  |   0
 Documentation/devicetree/bindings/__init__.py |   0
 .../devicetree/bindings/arm/__init__.py   |   0
 Documentation/devicetree/bindings/arm/cpus.py | 125 ++
 Documentation/devicetree/bindings/arm/pmu.py  |  38 ++
 .../devicetree/bindings/arm/rockchip.py   |  16 +++
 .../devicetree/bindings/arm/xilinx.py |  24 
 Documentation/devicetree/bindings/base.py |  14 ++
 Documentation/devicetree/bindings/chosen.py   |  15 +++
 .../devicetree/bindings/cpufreq/cpufreq-dt.py |  15 +++
 .../devicetree/bindings/fpga/fpga-region.py   |  15 +++
 .../bindings/iio/adc/xilinx-xadc.py   |  61 +
 .../bindings/iio/adc/xilinx-xadc.txt  |  34 ++---
 Documentation/devicetree/bindings/memory.py   |  15 +++
 Documentation/devicetree/bindings/opp/opp.py  |  15 +++
 .../bindings/regulator/fixed-regulator.py |  14 ++
 .../bindings/regulator/regulator.py   |  19 +++
 .../reserved-memory/reserved-memory.py|  14 ++
 .../devicetree/bindings/thermal/thermal.py|  15 +++
 .../devicetree/bindings/usb/usb-nop-xceiv.py  |  16 +++
 21 files changed, 449 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/__init__.py
 create mode 100644 Documentation/devicetree/__init__.py
 create mode 100644 Documentation/devicetree/bindings/__init__.py
 create mode 100644 Documentation/devicetree/bindings/arm/__init__.py
 create mode 100644 Documentation/devicetree/bindings/arm/cpus.py
 create mode 100644 Documentation/devicetree/bindings/arm/pmu.py
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip.py
 create mode 100644 Documentation/devicetree/bindings/arm/xilinx.py
 create mode 100644 Documentation/devicetree/bindings/base.py
 create mode 100644 Documentation/devicetree/bindings/chosen.py
 create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-dt.py
 create mode 100644 Documentation/devicetree/bindings/fpga/fpga-region.py
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.py
 create mode 100644 Documentation/devicetree/bindings/memory.py
 create mode 100644 Documentation/devicetree/bindings/opp/opp.py
 create mode 100644 
Documentation/devicetree/bindings/regulator/fixed-regulator.py
 create mode 100644 Documentation/devicetree/bindings/regulator/regulator.py
 create mode 100644 
Documentation/devicetree/bindings/reserved-memory/reserved-memory.py
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal.py
 create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.py

diff --git a/Documentation/__init__.py b/Documentation/__init__.py
new file mode 100644
index 0..e69de29bb2d1d
diff --git a/Documentation/devicetree/__init__.py 
b/Documentation/devicetree/__init__.py
new file mode 100644
index 0..e69de29bb2d1d
diff --git a/Documentation/devicetree/bindings/__init__.py 
b/Documentation/devicetree/bindings/__init__.py
new file mode 100644
index 0..e69de29bb2d1d
diff --git a/Documentation/devicetree/bindings/arm/__init__.py 
b/Documentation/devicetree/bindings/arm/__init__.py
new file mode 100644
index 0..e69de29bb2d1d
diff --git a/Documentation/devicetree/bindings/arm/cpus.py 
b/Documentation/devicetree/bindings/arm/cpus.py
new file mode 100644
index 0..6a2e94903d438
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cpus.py
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# /cpu bindings
+#
+# Copyright 2018 Google LLC
+#
+
+from kschema import NodeCpu, NodeCpus
+from kschema import PropClocks, PropPhandle, PropReg, PropString, PropSupply
+
+enable_methods = [
+'actions,s500-smp',
+'allwinner,sun6i-a31',
+'allwinner,sun8i-a23',
+'allwinner,sun9i-a80-smp',
+'amlogic,meson8-smp',
+'amlogic,meson8b-smp',
+'arm,realview-smp',
+'brcm,bcm11351-cpu-method',
+'brcm,bcm23550',
+'brcm,bcm2836-smp',
+'brcm,bcm-nsp-smp',
+'brcm,brahma-b15',
+'marvell,armada-375-smp',
+'marvell,armada-380-smp',
+'marvell,armada-390-smp',
+'marvell,armada-xp-smp',
+'marvell,98dx3236-smp',
+'mediatek,mt6589-smp',
+'mediatek,mt81xx-tz-smp',
+'qcom,gcc-msm8660',
+'qcom,kpss-acc-v1',
+'qcom,kpss-acc-v2',
+'renesas,apmu',
+'renesas,r9a06g032-smp',
+'rockchip,rk3036-smp',
+'rockchip,rk3066-smp',
+'ste,dbx500-smp',
+]
+
+schema = [
+NodeCpus(),
+NodeCpu(['arm,arm710t',
+  'arm,arm720t',
+  'arm,arm740t',
+  'arm,arm7ej-s',
+  'arm,arm7tdmi',
+  'arm

Re: [PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

Hi Jean,

On 27 April 2018 at 01:58, Jean Delvare <jdelv...@suse.de> wrote:
> Hi Simon,
>
> On Tue, 24 Apr 2018 15:11:11 -0600, Simon Glass wrote:
>> This is used in some systems from user space for determining the identity
>> of the device.
>>
>> Expose this as a file so that that user-space tools don't need to read
>> from /sys/firmware/dmi/tables/DMI
>>
>> Signed-off-by: Simon Glass <s...@chromium.org>
>> ---
>>
>>  drivers/firmware/dmi-id.c   | 2 ++
>>  drivers/firmware/dmi_scan.c | 1 +
>>  include/linux/mod_devicetable.h | 1 +
>>  3 files changed, 4 insertions(+)
>> (...)
>
> Looks good to me. Applied, thanks. For consistency I have moved product
> SKU before product family in all files, same order as in the DMI entry
> itself.

Sounds good, thanks.

- Simon

Re: [PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

Hi Jean,

On 27 April 2018 at 01:58, Jean Delvare  wrote:
> Hi Simon,
>
> On Tue, 24 Apr 2018 15:11:11 -0600, Simon Glass wrote:
>> This is used in some systems from user space for determining the identity
>> of the device.
>>
>> Expose this as a file so that that user-space tools don't need to read
>> from /sys/firmware/dmi/tables/DMI
>>
>> Signed-off-by: Simon Glass 
>> ---
>>
>>  drivers/firmware/dmi-id.c   | 2 ++
>>  drivers/firmware/dmi_scan.c | 1 +
>>  include/linux/mod_devicetable.h | 1 +
>>  3 files changed, 4 insertions(+)
>> (...)
>
> Looks good to me. Applied, thanks. For consistency I have moved product
> SKU before product family in all files, same order as in the DMI entry
> itself.

Sounds good, thanks.

- Simon

Re: [PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

Hi Vinod,

On 24 April 2018 at 20:51, Vinod Koul <vinod.k...@intel.com> wrote:
> On Tue, Apr 24, 2018 at 03:11:11PM -0600, Simon Glass wrote:
>> This is used in some systems from user space for determining the identity
>> of the device.
>>
>> Expose this as a file so that that user-space tools don't need to read
>> from /sys/firmware/dmi/tables/DMI
>
> sysfs is an ABI and needs to be documented, I don't see that in this patch,
> pls add.

Thanks for taking a look. The only mention of the dmi/id I can find is
in Kconfig:

config DMIID
bool "Export DMI identification via sysfs to userspace"
depends on DMI
default y
help
  Say Y here if you want to query SMBIOS/DMI system identification
  information from userspace through /sys/class/dmi/id/ or if you want
  DMI-based module auto-loading.


Are you suggesting that I create a list of the things in the /id node,
or something else?

Regards,
Simon

Re: [PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

Hi Vinod,

On 24 April 2018 at 20:51, Vinod Koul  wrote:
> On Tue, Apr 24, 2018 at 03:11:11PM -0600, Simon Glass wrote:
>> This is used in some systems from user space for determining the identity
>> of the device.
>>
>> Expose this as a file so that that user-space tools don't need to read
>> from /sys/firmware/dmi/tables/DMI
>
> sysfs is an ABI and needs to be documented, I don't see that in this patch,
> pls add.

Thanks for taking a look. The only mention of the dmi/id I can find is
in Kconfig:

config DMIID
bool "Export DMI identification via sysfs to userspace"
depends on DMI
default y
help
  Say Y here if you want to query SMBIOS/DMI system identification
  information from userspace through /sys/class/dmi/id/ or if you want
  DMI-based module auto-loading.


Are you suggesting that I create a list of the things in the /id node,
or something else?

Regards,
Simon

[PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

This is used in some systems from user space for determining the identity
of the device.

Expose this as a file so that that user-space tools don't need to read
from /sys/firmware/dmi/tables/DMI

Signed-off-by: Simon Glass <s...@chromium.org>
---

 drivers/firmware/dmi-id.c   | 2 ++
 drivers/firmware/dmi_scan.c | 1 +
 include/linux/mod_devicetable.h | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index 951b6c79f166a..fe78c7f801163 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -48,6 +48,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(product_version,0444, 
DMI_PRODUCT_VERSION);
 DEFINE_DMI_ATTR_WITH_SHOW(product_serial,  0400, DMI_PRODUCT_SERIAL);
 DEFINE_DMI_ATTR_WITH_SHOW(product_uuid,0400, DMI_PRODUCT_UUID);
 DEFINE_DMI_ATTR_WITH_SHOW(product_family,  0444, DMI_PRODUCT_FAMILY);
+DEFINE_DMI_ATTR_WITH_SHOW(product_sku, 0444, DMI_PRODUCT_SKU);
 DEFINE_DMI_ATTR_WITH_SHOW(board_vendor,0444, DMI_BOARD_VENDOR);
 DEFINE_DMI_ATTR_WITH_SHOW(board_name,  0444, DMI_BOARD_NAME);
 DEFINE_DMI_ATTR_WITH_SHOW(board_version,   0444, DMI_BOARD_VERSION);
@@ -193,6 +194,7 @@ static void __init dmi_id_init_attr_table(void)
ADD_DMI_ATTR(product_serial,DMI_PRODUCT_SERIAL);
ADD_DMI_ATTR(product_uuid,  DMI_PRODUCT_UUID);
ADD_DMI_ATTR(product_family,DMI_PRODUCT_FAMILY);
+   ADD_DMI_ATTR(product_sku,   DMI_PRODUCT_SKU);
ADD_DMI_ATTR(board_vendor,  DMI_BOARD_VENDOR);
ADD_DMI_ATTR(board_name,DMI_BOARD_NAME);
ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 54e66adef2525..f2483548cde92 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -447,6 +447,7 @@ static void __init dmi_decode(const struct dmi_header *dm, 
void *dummy)
dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
+   dmi_save_ident(dm, DMI_PRODUCT_SKU, 25);
dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
break;
case 2: /* Base Board Information */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7d361be2e24f4..cb8487e29d3ae 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -491,6 +491,7 @@ enum dmi_field {
DMI_PRODUCT_SERIAL,
DMI_PRODUCT_UUID,
DMI_PRODUCT_FAMILY,
+   DMI_PRODUCT_SKU,
DMI_BOARD_VENDOR,
DMI_BOARD_NAME,
DMI_BOARD_VERSION,
-- 
2.17.0.484.g0c8726318c-goog

[PATCH] firmware: dmi: Add access to the SKU ID string

[Simon Glass]

This is used in some systems from user space for determining the identity
of the device.

Expose this as a file so that that user-space tools don't need to read
from /sys/firmware/dmi/tables/DMI

Signed-off-by: Simon Glass 
---

 drivers/firmware/dmi-id.c   | 2 ++
 drivers/firmware/dmi_scan.c | 1 +
 include/linux/mod_devicetable.h | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index 951b6c79f166a..fe78c7f801163 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -48,6 +48,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(product_version,0444, 
DMI_PRODUCT_VERSION);
 DEFINE_DMI_ATTR_WITH_SHOW(product_serial,  0400, DMI_PRODUCT_SERIAL);
 DEFINE_DMI_ATTR_WITH_SHOW(product_uuid,0400, DMI_PRODUCT_UUID);
 DEFINE_DMI_ATTR_WITH_SHOW(product_family,  0444, DMI_PRODUCT_FAMILY);
+DEFINE_DMI_ATTR_WITH_SHOW(product_sku, 0444, DMI_PRODUCT_SKU);
 DEFINE_DMI_ATTR_WITH_SHOW(board_vendor,0444, DMI_BOARD_VENDOR);
 DEFINE_DMI_ATTR_WITH_SHOW(board_name,  0444, DMI_BOARD_NAME);
 DEFINE_DMI_ATTR_WITH_SHOW(board_version,   0444, DMI_BOARD_VERSION);
@@ -193,6 +194,7 @@ static void __init dmi_id_init_attr_table(void)
ADD_DMI_ATTR(product_serial,DMI_PRODUCT_SERIAL);
ADD_DMI_ATTR(product_uuid,  DMI_PRODUCT_UUID);
ADD_DMI_ATTR(product_family,DMI_PRODUCT_FAMILY);
+   ADD_DMI_ATTR(product_sku,   DMI_PRODUCT_SKU);
ADD_DMI_ATTR(board_vendor,  DMI_BOARD_VENDOR);
ADD_DMI_ATTR(board_name,DMI_BOARD_NAME);
ADD_DMI_ATTR(board_version, DMI_BOARD_VERSION);
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 54e66adef2525..f2483548cde92 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -447,6 +447,7 @@ static void __init dmi_decode(const struct dmi_header *dm, 
void *dummy)
dmi_save_ident(dm, DMI_PRODUCT_VERSION, 6);
dmi_save_ident(dm, DMI_PRODUCT_SERIAL, 7);
dmi_save_uuid(dm, DMI_PRODUCT_UUID, 8);
+   dmi_save_ident(dm, DMI_PRODUCT_SKU, 25);
dmi_save_ident(dm, DMI_PRODUCT_FAMILY, 26);
break;
case 2: /* Base Board Information */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7d361be2e24f4..cb8487e29d3ae 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -491,6 +491,7 @@ enum dmi_field {
DMI_PRODUCT_SERIAL,
DMI_PRODUCT_UUID,
DMI_PRODUCT_FAMILY,
+   DMI_PRODUCT_SKU,
DMI_BOARD_VENDOR,
DMI_BOARD_NAME,
DMI_BOARD_VERSION,
-- 
2.17.0.484.g0c8726318c-goog

Re: [PATCH v3 02/10] video: add support of MIPI DSI interface

[Simon Glass]

Hi,

On 13 March 2018 at 07:50, yannick fertre  wrote:
>
> Mipi_display.c contains a set of dsi helpers.
> This file is a copy of file drm_mipi_dsi.c (linux kernel).
>
> Signed-off-by: yannick fertre 
> ---
>  drivers/video/Kconfig|   7 +
>  drivers/video/Makefile   |   1 +
>  drivers/video/mipi_display.c | 807 
> +++
>  include/mipi_display.h   | 257 +-

Please add function comments for the functions in this file,
explaining args and what the functions do.

Shouldn't DSI be its own uclass? We normally separate drivers for
different peripherals in U-Boot.

>  4 files changed, 1071 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/mipi_display.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2fc0def..1981298 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -75,6 +75,13 @@ config VIDEO_ANSI
>   Enable ANSI escape sequence decoding for a more fully functional
>   console.
>
> +config VIDEO_MIPI_DSI
> +   bool "Support MIPI DSI interface"
> +   depends on DM_VIDEO
> +   default y if DM_VIDEO
> +   help
> + Support MIPI DSI interface for driving a MIPI compatible LCD panel.

Please expand out what MIPI stands for and what it is, same with DSI.

Re: [PATCH v3 02/10] video: add support of MIPI DSI interface

[Simon Glass]

Hi,

On 13 March 2018 at 07:50, yannick fertre  wrote:
>
> Mipi_display.c contains a set of dsi helpers.
> This file is a copy of file drm_mipi_dsi.c (linux kernel).
>
> Signed-off-by: yannick fertre 
> ---
>  drivers/video/Kconfig|   7 +
>  drivers/video/Makefile   |   1 +
>  drivers/video/mipi_display.c | 807 
> +++
>  include/mipi_display.h   | 257 +-

Please add function comments for the functions in this file,
explaining args and what the functions do.

Shouldn't DSI be its own uclass? We normally separate drivers for
different peripherals in U-Boot.

>  4 files changed, 1071 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/mipi_display.c
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 2fc0def..1981298 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -75,6 +75,13 @@ config VIDEO_ANSI
>   Enable ANSI escape sequence decoding for a more fully functional
>   console.
>
> +config VIDEO_MIPI_DSI
> +   bool "Support MIPI DSI interface"
> +   depends on DM_VIDEO
> +   default y if DM_VIDEO
> +   help
> + Support MIPI DSI interface for driving a MIPI compatible LCD panel.

Please expand out what MIPI stands for and what it is, same with DSI.

Re: [PATCH v2 00/10] splash screen on the stm32f769 disco board

[Simon Glass]

On 2 March 2018 at 08:44, yannick fertre  wrote:
>
> Version 2:
> - Replace debug log by pr_error, pr_warn or pr_info.
> - Rework bridge between ltdc & dsi panel
> - Rework backligh management (with or witout gpio)
> - Rework panel otm8009a
> - Add new panel raydium rm68200
>
> Version 1:
> - Initial commit
>
> This serie contains all patchsets needed for displaying a splash screen
> on the stm32f769 disco board.
>
> yannick fertre (10):
>   video: stm32: stm32_ltdc: add bridge to display controller
>   video: stm32: stm32_ltdc: update debug log
>   video: add support of MIPI DSI interface
>   video: add support of panel OTM8009A
>   video: add MIPI DSI host controller bridge
>   video: add support of STM32 MIPI DSI controller driver
>   video: add support of panel rm68200
>   arm: dts: stm32: add dsi for STM32F746
>   arm: dts: stm32: add display for STM32F769 disco board
>   board: Add STM32F769 SoC, discovery board support
>
>  arch/arm/dts/stm32f746.dtsi|  12 +
>  arch/arm/dts/stm32f769-disco.dts   |  71 
>  configs/stm32f769-disco_defconfig  |  63 +++
>  drivers/video/Kconfig  |  32 ++
>  drivers/video/Makefile |   4 +
>  drivers/video/dw_mipi_dsi.c| 822 
> +
>  drivers/video/mipi_display.c   | 807 
>  drivers/video/orisetech_otm8009a.c | 329 +++
>  drivers/video/raydium-rm68200.c| 329 +++
>  drivers/video/stm32/Kconfig|  10 +
>  drivers/video/stm32/Makefile   |   1 +
>  drivers/video/stm32/stm32_dsi.c| 427 +++
>  drivers/video/stm32/stm32_ltdc.c   | 144 ---
>  include/dw_mipi_dsi.h  |  32 ++
>  include/mipi_display.h | 257 +++-
>  15 files changed, 3285 insertions(+), 55 deletions(-)
>  create mode 100644 configs/stm32f769-disco_defconfig
>  create mode 100644 drivers/video/dw_mipi_dsi.c
>  create mode 100644 drivers/video/mipi_display.c
>  create mode 100644 drivers/video/orisetech_otm8009a.c
>  create mode 100644 drivers/video/raydium-rm68200.c
>  create mode 100644 drivers/video/stm32/stm32_dsi.c
>  create mode 100644 include/dw_mipi_dsi.h

Does this use driver model? I cannot see it.

Regards,
Simon

Re: [PATCH v2 00/10] splash screen on the stm32f769 disco board

[Simon Glass]

On 2 March 2018 at 08:44, yannick fertre  wrote:
>
> Version 2:
> - Replace debug log by pr_error, pr_warn or pr_info.
> - Rework bridge between ltdc & dsi panel
> - Rework backligh management (with or witout gpio)
> - Rework panel otm8009a
> - Add new panel raydium rm68200
>
> Version 1:
> - Initial commit
>
> This serie contains all patchsets needed for displaying a splash screen
> on the stm32f769 disco board.
>
> yannick fertre (10):
>   video: stm32: stm32_ltdc: add bridge to display controller
>   video: stm32: stm32_ltdc: update debug log
>   video: add support of MIPI DSI interface
>   video: add support of panel OTM8009A
>   video: add MIPI DSI host controller bridge
>   video: add support of STM32 MIPI DSI controller driver
>   video: add support of panel rm68200
>   arm: dts: stm32: add dsi for STM32F746
>   arm: dts: stm32: add display for STM32F769 disco board
>   board: Add STM32F769 SoC, discovery board support
>
>  arch/arm/dts/stm32f746.dtsi|  12 +
>  arch/arm/dts/stm32f769-disco.dts   |  71 
>  configs/stm32f769-disco_defconfig  |  63 +++
>  drivers/video/Kconfig  |  32 ++
>  drivers/video/Makefile |   4 +
>  drivers/video/dw_mipi_dsi.c| 822 
> +
>  drivers/video/mipi_display.c   | 807 
>  drivers/video/orisetech_otm8009a.c | 329 +++
>  drivers/video/raydium-rm68200.c| 329 +++
>  drivers/video/stm32/Kconfig|  10 +
>  drivers/video/stm32/Makefile   |   1 +
>  drivers/video/stm32/stm32_dsi.c| 427 +++
>  drivers/video/stm32/stm32_ltdc.c   | 144 ---
>  include/dw_mipi_dsi.h  |  32 ++
>  include/mipi_display.h | 257 +++-
>  15 files changed, 3285 insertions(+), 55 deletions(-)
>  create mode 100644 configs/stm32f769-disco_defconfig
>  create mode 100644 drivers/video/dw_mipi_dsi.c
>  create mode 100644 drivers/video/mipi_display.c
>  create mode 100644 drivers/video/orisetech_otm8009a.c
>  create mode 100644 drivers/video/raydium-rm68200.c
>  create mode 100644 drivers/video/stm32/stm32_dsi.c
>  create mode 100644 include/dw_mipi_dsi.h

Does this use driver model? I cannot see it.

Regards,
Simon

Re: [PATCH 2/6] x86/boot: Move compressed kernel to end of decompression buffer

[Simon Glass]

HI Matt,

On 16 August 2016 at 20:25, Matt Mullins  wrote:
> On Tue, Aug 16, 2016 at 12:19:42PM -0700, Yinghai Lu wrote:
>> On Mon, Aug 15, 2016 at 9:01 PM, Matt Mullins  wrote:
>> >
>> > This appears to have a negative effect on booting the Intel Edison 
>> > platform, as
>> > it uses u-boot as its bootloader.  u-boot does not copy the init_size 
>> > parameter
>> > when booting a bzImage: it copies a fixed-size setup_header [1], and its
>> > definition of setup_header doesn't include the parameters beyond 
>> > setup_data [2].
>> >
>> > With a zero value for init_size, this calculates a %rsp value of 
>> > 0x101ff9600.
>> > This causes the boot process to hard-stop at the immediately-following 
>> > pushq, as
>> > this platform has no usable physical addresses above 4G.
>> >
>> > What are the options for getting this type of platform to function again?  
>> > For
>> > now, kexec from a working Linux system does seem to be a work-around, but 
>> > there
>> > appears to be other x86 hardware using u-boot: the chromium.org folks seem 
>> > to be
>> > maintaining the u-boot x86 tree.
>> >
>> > [1] 
>> > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/x86/lib/zimage.c;h=1b33c771391f49ffe82864ff1582bdfd07e5e97d;hb=HEAD#l156
>> > [2] 
>> > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/x86/include/asm/bootparam.h;h=140095117e5a2daef0a097c55f0ed10e08acc781;hb=HEAD#l24
>>
>> Then should fix the u-boot about header_size assumption.
>
> I was hoping to avoid that, since the Edison's u-boot is 10,000-line patch 
> atop
> the upstream -- I don't trust myself to build and flash one quite yet.

It would be good to upstream that!

>
> If this turned out to affect Chromebooks, I'd spend more effort pushing for
> a kernel fix, but it seems that ChromeOS has a different kernel load procedure
> and doesn't use "zboot".  For now, I'll probably just keep a local patch that
> hard-codes a value large enough to decompress and launch the kernel.
>
> I may turn that local patch into something gated by a Kconfig eventually, in
> hopes that users of the other x86 u-boot platforms will see it in a "make
> oldconfig" run.

Well, I think this patch is useful. But also, let's fix U-Boot.

Regards,
Simon

Re: [PATCH 2/6] x86/boot: Move compressed kernel to end of decompression buffer

[Simon Glass]

HI Matt,

On 16 August 2016 at 20:25, Matt Mullins  wrote:
> On Tue, Aug 16, 2016 at 12:19:42PM -0700, Yinghai Lu wrote:
>> On Mon, Aug 15, 2016 at 9:01 PM, Matt Mullins  wrote:
>> >
>> > This appears to have a negative effect on booting the Intel Edison 
>> > platform, as
>> > it uses u-boot as its bootloader.  u-boot does not copy the init_size 
>> > parameter
>> > when booting a bzImage: it copies a fixed-size setup_header [1], and its
>> > definition of setup_header doesn't include the parameters beyond 
>> > setup_data [2].
>> >
>> > With a zero value for init_size, this calculates a %rsp value of 
>> > 0x101ff9600.
>> > This causes the boot process to hard-stop at the immediately-following 
>> > pushq, as
>> > this platform has no usable physical addresses above 4G.
>> >
>> > What are the options for getting this type of platform to function again?  
>> > For
>> > now, kexec from a working Linux system does seem to be a work-around, but 
>> > there
>> > appears to be other x86 hardware using u-boot: the chromium.org folks seem 
>> > to be
>> > maintaining the u-boot x86 tree.
>> >
>> > [1] 
>> > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/x86/lib/zimage.c;h=1b33c771391f49ffe82864ff1582bdfd07e5e97d;hb=HEAD#l156
>> > [2] 
>> > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/x86/include/asm/bootparam.h;h=140095117e5a2daef0a097c55f0ed10e08acc781;hb=HEAD#l24
>>
>> Then should fix the u-boot about header_size assumption.
>
> I was hoping to avoid that, since the Edison's u-boot is 10,000-line patch 
> atop
> the upstream -- I don't trust myself to build and flash one quite yet.

It would be good to upstream that!

>
> If this turned out to affect Chromebooks, I'd spend more effort pushing for
> a kernel fix, but it seems that ChromeOS has a different kernel load procedure
> and doesn't use "zboot".  For now, I'll probably just keep a local patch that
> hard-codes a value large enough to decompress and launch the kernel.
>
> I may turn that local patch into something gated by a Kconfig eventually, in
> hopes that users of the other x86 u-boot platforms will see it in a "make
> oldconfig" run.

Well, I think this patch is useful. But also, let's fix U-Boot.

Regards,
Simon

Re: [ANNOUNCE] git-series: track changes to a patch series over time

[Simon Glass]

Hi Josh,

On 1 August 2016 at 12:37, Josh Triplett  wrote:
> On Mon, Aug 01, 2016 at 09:14:54AM -0600, Stephen Warren wrote:
>> On 07/29/2016 12:40 AM, Josh Triplett wrote:
>> > I'd like to announce a project I've been working on for a while:
>> >
>> > git-series provides a tool for managing patch series with git, tracking
>> > the "history of history". git series tracks changes to the patch series
>> > over time, including rebases and other non-fast-forwarding changes. git
>> > series also tracks a cover letter for the patch series, formats the
>> > series for email, and prepares pull requests.
>>
>> Just as an FYI, I wouldn't be surprised if there's some overlap, or
>> potential for merging of tools, between this tool and the "patman" tool
>> that's part of the U-Boot source tree:
>>
>> http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README;h=e36857dedea1d0dbafa41732aaf9bf0988d63f38;hb=HEAD
>
> Interesting tool; thanks for the link.
>
> As far as I can tell from that documentation, patman doesn't track old
> versions of a patch series; you rebase to modify patches or change
> patman tags (embedded in commit messages), and nothing preserves the
> previous version.  And it tracks the cover letter and similar in one of
> the commit messages in the series, so previous versions of that don't
> get saved either.  If you wanted to track the history of your changes,
> you'd have to use branch names or similar.

That's right. Normally you would keep the old branch around, or tag
it. Of course old branches are often based on older versions the
upstream repo, so they are not that useful for diiff, etc. But the
normal procedure when updating a series to a new version is:

git checkout -b wibble-v2 wibble
git rebase upstream/master
git commit --amend
# Edit commit to add 'Series-version: 2', update cover letter etc.

Of course any change log is preserved when you move to v3, since you
just add more 'Series-changes:' tags. The old version of the cover
letter, and the old version of the commits can be preserved with 'git
tag'.

>
> In addition, tracking metadata in commit messages only works with a
> patches-by-mail workflow where the messages get processed when
> generating patches; that doesn't work for please-pull workflows.

Can you explain what a please-pull workflow looks like, and what tags
are expected?

>
> patman does have quite a few interesting ideas, though.  git-series
> needs some way of handling To/Cc addresses for patches and the cover
> letter (beyond just scripts/get_maintainer.pl), and more automatic
> handling of series versioning (v2, v3, ...) and associated series
> changelogs.  Suggestions welcome.

Patman builds the cover letter change lists from the commits. The main
point of patman is to automate the error-prone process of submitting a
perfectly formed patch series.

In particular, patman requires no change to the normal workflow that
people use with git.

>
> - Josh Triplett

Regards,
Simon

Re: [ANNOUNCE] git-series: track changes to a patch series over time

[Simon Glass]

Hi Josh,

On 1 August 2016 at 12:37, Josh Triplett  wrote:
> On Mon, Aug 01, 2016 at 09:14:54AM -0600, Stephen Warren wrote:
>> On 07/29/2016 12:40 AM, Josh Triplett wrote:
>> > I'd like to announce a project I've been working on for a while:
>> >
>> > git-series provides a tool for managing patch series with git, tracking
>> > the "history of history". git series tracks changes to the patch series
>> > over time, including rebases and other non-fast-forwarding changes. git
>> > series also tracks a cover letter for the patch series, formats the
>> > series for email, and prepares pull requests.
>>
>> Just as an FYI, I wouldn't be surprised if there's some overlap, or
>> potential for merging of tools, between this tool and the "patman" tool
>> that's part of the U-Boot source tree:
>>
>> http://git.denx.de/?p=u-boot.git;a=blob;f=tools/patman/README;h=e36857dedea1d0dbafa41732aaf9bf0988d63f38;hb=HEAD
>
> Interesting tool; thanks for the link.
>
> As far as I can tell from that documentation, patman doesn't track old
> versions of a patch series; you rebase to modify patches or change
> patman tags (embedded in commit messages), and nothing preserves the
> previous version.  And it tracks the cover letter and similar in one of
> the commit messages in the series, so previous versions of that don't
> get saved either.  If you wanted to track the history of your changes,
> you'd have to use branch names or similar.

That's right. Normally you would keep the old branch around, or tag
it. Of course old branches are often based on older versions the
upstream repo, so they are not that useful for diiff, etc. But the
normal procedure when updating a series to a new version is:

git checkout -b wibble-v2 wibble
git rebase upstream/master
git commit --amend
# Edit commit to add 'Series-version: 2', update cover letter etc.

Of course any change log is preserved when you move to v3, since you
just add more 'Series-changes:' tags. The old version of the cover
letter, and the old version of the commits can be preserved with 'git
tag'.

>
> In addition, tracking metadata in commit messages only works with a
> patches-by-mail workflow where the messages get processed when
> generating patches; that doesn't work for please-pull workflows.

Can you explain what a please-pull workflow looks like, and what tags
are expected?

>
> patman does have quite a few interesting ideas, though.  git-series
> needs some way of handling To/Cc addresses for patches and the cover
> letter (beyond just scripts/get_maintainer.pl), and more automatic
> handling of series versioning (v2, v3, ...) and associated series
> changelogs.  Suggestions welcome.

Patman builds the cover letter change lists from the commits. The main
point of patman is to automate the error-prone process of submitting a
perfectly formed patch series.

In particular, patman requires no change to the normal workflow that
people use with git.

>
> - Josh Triplett

Regards,
Simon

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi,

On Friday, 28 August 2015, Simon Glass  wrote:
>
> Hi Rob,
>
> On 25 August 2015 at 10:22, Rob Herring  wrote:
> > On Sat, Aug 15, 2015 at 8:46 AM, Simon Glass  wrote:
> >> Hi Rob,
> >>
> >> On 14 August 2015 at 14:29, Rob Herring  wrote:
> >>> On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass  wrote:
> >>>> -linux-tegra
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 12 August 2015 at 07:21, Simon Glass  wrote:
> >>>>> Hi Lucas,
> >>>>>
> >>>>> On 11 August 2015 at 11:05, Lucas Stach  wrote:
> >>>>>> Hi Simon,
> >>>>>>
> >>>>>> why did you send this to the Tegra ML?
> >>>>>>
> >>>>>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
> >>>>>>> This updates the device tree from the kernel version to something 
> >>>>>>> suitable
> >>>>>>> for U-Boot:
> >>>>>>>
> >>>>>>> - Add stdout-path alias for console
> >>>>>>> - Mark the /soc node to be available pre-relocation so that the early
> >>>>>>> serial console works (we need the 'ranges' property to be available)
> >>>
> >>> I find it quite strange that you must explicitly enable the parent
> >>> node, but not the uart node.
> >>
> >> U-Boot tries hard to find and bind the UART using the stdout-path
> >> link. I would like to add it in the UART node also but we were able to
> >> work around it so far.
> >>
> >>>
> >>>>>>>
> >>>>>>> Signed-off-by: Simon Glass 
> >>>>>>> ---
> >>>>>>>
> >>>>>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
> >>>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
> >>>>>>> b/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> index 301c73f..bd6bff6 100644
> >>>>>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> @@ -8,6 +8,7 @@
> >>>>>>>
> >>>>>>>   chosen {
> >>>>>>>   bootargs = "earlyprintk console=ttyAMA0";
> >>>>>>> + stdout-path = 
> >>>>>>>   };
> >>>>>>>
> >>>>>>>   soc {
> >>>>>>> @@ -16,6 +17,7 @@
> >>>>>>>   #size-cells = <1>;
> >>>>>>>   ranges = <0x7e00 0x2000 0x0200>;
> >>>>>>>   dma-ranges = <0x4000 0x 0x2000>;
> >>>>>>> + u-boot,dm-pre-reloc;
> >>>>>>
> >>>>>> Why do you need this and why should upstream carry your favourite
> >>>>>> bootloaders configuration? This is in no way hardware description.
> >>>>>
> >>>>> I'm not sure how much you know about U-Boot, so let me know if you
> >>>>> need more info.
> >>>>>
> >>>>> U-Boot normally starts up by setting up its serial UART and displaying
> >>>>> a banner message. At this stage typically only a few devices are
> >>>>> initialised (e.g. maybe just the UART). It then relocates itself to
> >>>>> the top of memory and starts up all the devices. It throws away any
> >>>>> previous devices that it set up before relocation and starts again.
> >>>>>
> >>>>> U-Boot uses a thing called driver model (dm) which handles driver
> >>>>> binding and probing. Driver model has the device tree and would
> >>>>> normally scan through it and create devices for everything it finds.
> >>>
> >>> How do you debug the DM itself? It seems like you still would need
> >>> something earlier for debug like earlycon in the kernel. u-boot DM is
> >>> probably simple enough you can get away with using it early, but you
> >>> often can't as the complexity increases. Ultimately you need something
> >>> simple that just hits all the registers needed to get characters out.
&g

Re: set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier)

[Simon Glass]

Hi,

On 8 September 2015 at 16:46, Heiko Stübner  wrote:
>
> Hi Andy,
>
> Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
> > rockchip platform have a protocol to pass the the kernel
> > reboot mode to bootloader by some special registers when
> > system reboot.By this way the bootloader can take different
> > action according to the different kernel reboot mode, for
> > example, command "reboot loader" will reboot the board to
> > rockusb mode, this is a very convenient way to get the board
> > to download mode.
> >
> > Signed-off-by: Andy Yan 
>
> [...]
>
> > @@ -0,0 +1,22 @@
> > +#ifndef __MACH_ROCKCHIP_LOADER_H
> > +#define __MACH_ROCKCHIP_LOADER_H
> > +
> > +/*high 24 bits is tag, low 8 bits is type*/
> > +#define SYS_LOADER_REBOOT_FLAG   0x5242C300
> > +
> > +enum {
> > + BOOT_NORMAL = 0, /* normal boot */
> > + BOOT_LOADER, /* enter loader rockusb mode */
> > + BOOT_MASKROM,/* enter maskrom rockusb mode (not support now) */
> > + BOOT_RECOVER,/* enter recover */
> > + BOOT_NORECOVER,  /* do not enter recover */
> > + BOOT_SECONDOS,   /* boot second OS (not support now)*/
> > + BOOT_WIPEDATA,   /* enter recover and wipe data. */
> > + BOOT_WIPEALL,/* enter recover and wipe all data. */
> > + BOOT_CHECKIMG,   /* check firmware img with backup part*/
> > + BOOT_FASTBOOT,   /* enter fast boot mode */
> > + BOOT_SECUREBOOT_DISABLE,
> > + BOOT_CHARGING,   /* enter charge mode */
> > + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> > +};
> > +#endif
>
> These flags rely on code in the bootloader to actually handle the target
> action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
> actively moving away from that, with the recent rk3288 addition to mainline
> uboot.
>
> So unless you convince uboot people that the _underlying special
> functionality_ behind these flags should be part of uboot, I don't think this
> is going to fly.
>
>
> In a way this is similar to gpu kernel code talking to proprietary userspace
> libs - these are also not eligible for the kernel. (meaning stuff like the
> mali kernel driver not being allowed).

I don't want to comment on what Linux does or does not want. But I can
see this sort of feature being useful for devs at least. So long as it
is defined in a way that is not Rockchip-specific (and the above enum
looks pretty reasonable on that front, I think it makes sense.

Of course it's a bit odd to target a downstream U-Boot with a Linux
feature. But hopefully Rockchip's U-Boot support and development will
move to mainline with time.

>
> [...]
>
> > +static int rockchip_reboot_notify(struct notifier_block *this,
> > +   unsigned long mode, void *cmd)
> > +{
> > + u32 flag;
> > +
> > + rockchip_get_reboot_flag(cmd, );
> > + regmap_write(regmap, flag_reg, flag);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rockchip_reboot_handler = {
> > + .notifier_call = rockchip_reboot_notify,
> > + .priority = 150,
> > +};
>
> the restart handlers are meant to really only restart the system, not to
> execute some actions before the restart happens.
>
> See https://lkml.org/lkml/2015/6/3/707 for a similar case.
>
>
> Heiko

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: set rockchip-specific uboot bootmode flags on reboot (was: [PATCH] ARM: rockchip: add reboot notifier)

[Simon Glass]

Hi,

On 8 September 2015 at 16:46, Heiko Stübner  wrote:
>
> Hi Andy,
>
> Am Dienstag, 8. September 2015, 20:43:07 schrieb Andy Yan:
> > rockchip platform have a protocol to pass the the kernel
> > reboot mode to bootloader by some special registers when
> > system reboot.By this way the bootloader can take different
> > action according to the different kernel reboot mode, for
> > example, command "reboot loader" will reboot the board to
> > rockusb mode, this is a very convenient way to get the board
> > to download mode.
> >
> > Signed-off-by: Andy Yan 
>
> [...]
>
> > @@ -0,0 +1,22 @@
> > +#ifndef __MACH_ROCKCHIP_LOADER_H
> > +#define __MACH_ROCKCHIP_LOADER_H
> > +
> > +/*high 24 bits is tag, low 8 bits is type*/
> > +#define SYS_LOADER_REBOOT_FLAG   0x5242C300
> > +
> > +enum {
> > + BOOT_NORMAL = 0, /* normal boot */
> > + BOOT_LOADER, /* enter loader rockusb mode */
> > + BOOT_MASKROM,/* enter maskrom rockusb mode (not support now) */
> > + BOOT_RECOVER,/* enter recover */
> > + BOOT_NORECOVER,  /* do not enter recover */
> > + BOOT_SECONDOS,   /* boot second OS (not support now)*/
> > + BOOT_WIPEDATA,   /* enter recover and wipe data. */
> > + BOOT_WIPEALL,/* enter recover and wipe all data. */
> > + BOOT_CHECKIMG,   /* check firmware img with backup part*/
> > + BOOT_FASTBOOT,   /* enter fast boot mode */
> > + BOOT_SECUREBOOT_DISABLE,
> > + BOOT_CHARGING,   /* enter charge mode */
> > + BOOT_MAX /* MAX VALID BOOT TYPE.*/
> > +};
> > +#endif
>
> These flags rely on code in the bootloader to actually handle the target
> action. Nowadays this is uboot, but still a rockchip-specific fork. And we're
> actively moving away from that, with the recent rk3288 addition to mainline
> uboot.
>
> So unless you convince uboot people that the _underlying special
> functionality_ behind these flags should be part of uboot, I don't think this
> is going to fly.
>
>
> In a way this is similar to gpu kernel code talking to proprietary userspace
> libs - these are also not eligible for the kernel. (meaning stuff like the
> mali kernel driver not being allowed).

I don't want to comment on what Linux does or does not want. But I can
see this sort of feature being useful for devs at least. So long as it
is defined in a way that is not Rockchip-specific (and the above enum
looks pretty reasonable on that front, I think it makes sense.

Of course it's a bit odd to target a downstream U-Boot with a Linux
feature. But hopefully Rockchip's U-Boot support and development will
move to mainline with time.

>
> [...]
>
> > +static int rockchip_reboot_notify(struct notifier_block *this,
> > +   unsigned long mode, void *cmd)
> > +{
> > + u32 flag;
> > +
> > + rockchip_get_reboot_flag(cmd, );
> > + regmap_write(regmap, flag_reg, flag);
> > +
> > + return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block rockchip_reboot_handler = {
> > + .notifier_call = rockchip_reboot_notify,
> > + .priority = 150,
> > +};
>
> the restart handlers are meant to really only restart the system, not to
> execute some actions before the restart happens.
>
> See https://lkml.org/lkml/2015/6/3/707 for a similar case.
>
>
> Heiko

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi,

On Friday, 28 August 2015, Simon Glass <s...@chromium.org> wrote:
>
> Hi Rob,
>
> On 25 August 2015 at 10:22, Rob Herring <robherri...@gmail.com> wrote:
> > On Sat, Aug 15, 2015 at 8:46 AM, Simon Glass <s...@chromium.org> wrote:
> >> Hi Rob,
> >>
> >> On 14 August 2015 at 14:29, Rob Herring <robherri...@gmail.com> wrote:
> >>> On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass <s...@chromium.org> wrote:
> >>>> -linux-tegra
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 12 August 2015 at 07:21, Simon Glass <s...@chromium.org> wrote:
> >>>>> Hi Lucas,
> >>>>>
> >>>>> On 11 August 2015 at 11:05, Lucas Stach <d...@lynxeye.de> wrote:
> >>>>>> Hi Simon,
> >>>>>>
> >>>>>> why did you send this to the Tegra ML?
> >>>>>>
> >>>>>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
> >>>>>>> This updates the device tree from the kernel version to something 
> >>>>>>> suitable
> >>>>>>> for U-Boot:
> >>>>>>>
> >>>>>>> - Add stdout-path alias for console
> >>>>>>> - Mark the /soc node to be available pre-relocation so that the early
> >>>>>>> serial console works (we need the 'ranges' property to be available)
> >>>
> >>> I find it quite strange that you must explicitly enable the parent
> >>> node, but not the uart node.
> >>
> >> U-Boot tries hard to find and bind the UART using the stdout-path
> >> link. I would like to add it in the UART node also but we were able to
> >> work around it so far.
> >>
> >>>
> >>>>>>>
> >>>>>>> Signed-off-by: Simon Glass <s...@chromium.org>
> >>>>>>> ---
> >>>>>>>
> >>>>>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
> >>>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
> >>>>>>> b/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> index 301c73f..bd6bff6 100644
> >>>>>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
> >>>>>>> @@ -8,6 +8,7 @@
> >>>>>>>
> >>>>>>>   chosen {
> >>>>>>>   bootargs = "earlyprintk console=ttyAMA0";
> >>>>>>> + stdout-path = 
> >>>>>>>   };
> >>>>>>>
> >>>>>>>   soc {
> >>>>>>> @@ -16,6 +17,7 @@
> >>>>>>>   #size-cells = <1>;
> >>>>>>>   ranges = <0x7e00 0x2000 0x0200>;
> >>>>>>>   dma-ranges = <0x4000 0x 0x2000>;
> >>>>>>> + u-boot,dm-pre-reloc;
> >>>>>>
> >>>>>> Why do you need this and why should upstream carry your favourite
> >>>>>> bootloaders configuration? This is in no way hardware description.
> >>>>>
> >>>>> I'm not sure how much you know about U-Boot, so let me know if you
> >>>>> need more info.
> >>>>>
> >>>>> U-Boot normally starts up by setting up its serial UART and displaying
> >>>>> a banner message. At this stage typically only a few devices are
> >>>>> initialised (e.g. maybe just the UART). It then relocates itself to
> >>>>> the top of memory and starts up all the devices. It throws away any
> >>>>> previous devices that it set up before relocation and starts again.
> >>>>>
> >>>>> U-Boot uses a thing called driver model (dm) which handles driver
> >>>>> binding and probing. Driver model has the device tree and would
> >>>>> normally scan through it and create devices for everything it finds.
> >>>
> >>> How do you debug the DM itself? It seems like you still would need
> >>> something earlier for debug like earlycon in the kernel. u-boot DM is
> >>> probably simple enough you can get awa

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Rob,

On 25 August 2015 at 10:22, Rob Herring  wrote:
> On Sat, Aug 15, 2015 at 8:46 AM, Simon Glass  wrote:
>> Hi Rob,
>>
>> On 14 August 2015 at 14:29, Rob Herring  wrote:
>>> On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass  wrote:
>>>> -linux-tegra
>>>>
>>>> Hi,
>>>>
>>>> On 12 August 2015 at 07:21, Simon Glass  wrote:
>>>>> Hi Lucas,
>>>>>
>>>>> On 11 August 2015 at 11:05, Lucas Stach  wrote:
>>>>>> Hi Simon,
>>>>>>
>>>>>> why did you send this to the Tegra ML?
>>>>>>
>>>>>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>>>>>>> This updates the device tree from the kernel version to something 
>>>>>>> suitable
>>>>>>> for U-Boot:
>>>>>>>
>>>>>>> - Add stdout-path alias for console
>>>>>>> - Mark the /soc node to be available pre-relocation so that the early
>>>>>>> serial console works (we need the 'ranges' property to be available)
>>>
>>> I find it quite strange that you must explicitly enable the parent
>>> node, but not the uart node.
>>
>> U-Boot tries hard to find and bind the UART using the stdout-path
>> link. I would like to add it in the UART node also but we were able to
>> work around it so far.
>>
>>>
>>>>>>>
>>>>>>> Signed-off-by: Simon Glass 
>>>>>>> ---
>>>>>>>
>>>>>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
>>>>>>> b/arch/arm/boot/dts/bcm2835.dtsi
>>>>>>> index 301c73f..bd6bff6 100644
>>>>>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>>>>>>> @@ -8,6 +8,7 @@
>>>>>>>
>>>>>>>   chosen {
>>>>>>>   bootargs = "earlyprintk console=ttyAMA0";
>>>>>>> + stdout-path = 
>>>>>>>   };
>>>>>>>
>>>>>>>   soc {
>>>>>>> @@ -16,6 +17,7 @@
>>>>>>>   #size-cells = <1>;
>>>>>>>   ranges = <0x7e00 0x2000 0x0200>;
>>>>>>>   dma-ranges = <0x4000 0x 0x2000>;
>>>>>>> + u-boot,dm-pre-reloc;
>>>>>>
>>>>>> Why do you need this and why should upstream carry your favourite
>>>>>> bootloaders configuration? This is in no way hardware description.
>>>>>
>>>>> I'm not sure how much you know about U-Boot, so let me know if you
>>>>> need more info.
>>>>>
>>>>> U-Boot normally starts up by setting up its serial UART and displaying
>>>>> a banner message. At this stage typically only a few devices are
>>>>> initialised (e.g. maybe just the UART). It then relocates itself to
>>>>> the top of memory and starts up all the devices. It throws away any
>>>>> previous devices that it set up before relocation and starts again.
>>>>>
>>>>> U-Boot uses a thing called driver model (dm) which handles driver
>>>>> binding and probing. Driver model has the device tree and would
>>>>> normally scan through it and create devices for everything it finds.
>>>
>>> How do you debug the DM itself? It seems like you still would need
>>> something earlier for debug like earlycon in the kernel. u-boot DM is
>>> probably simple enough you can get away with using it early, but you
>>> often can't as the complexity increases. Ultimately you need something
>>> simple that just hits all the registers needed to get characters out.
>>> What happens when you add pinmux, clocks, PMIC, power domains, etc. to
>>> the DM and they all become dependencies for the UART?
>>
>> This doesn't seem like a device tree binding question but let me try
>> to answer it.
>>
>> This is a problem - one of the challenges of driver model is that the
>> UART gets further away from the reset vector.
>
> This is a common problem for any complex embedded system. Nothin

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Rob,

On 25 August 2015 at 10:22, Rob Herring robherri...@gmail.com wrote:
 On Sat, Aug 15, 2015 at 8:46 AM, Simon Glass s...@chromium.org wrote:
 Hi Rob,

 On 14 August 2015 at 14:29, Rob Herring robherri...@gmail.com wrote:
 On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass s...@chromium.org wrote:
 -linux-tegra

 Hi,

 On 12 August 2015 at 07:21, Simon Glass s...@chromium.org wrote:
 Hi Lucas,

 On 11 August 2015 at 11:05, Lucas Stach d...@lynxeye.de wrote:
 Hi Simon,

 why did you send this to the Tegra ML?

 Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
 This updates the device tree from the kernel version to something 
 suitable
 for U-Boot:

 - Add stdout-path alias for console
 - Mark the /soc node to be available pre-relocation so that the early
 serial console works (we need the 'ranges' property to be available)

 I find it quite strange that you must explicitly enable the parent
 node, but not the uart node.

 U-Boot tries hard to find and bind the UART using the stdout-path
 link. I would like to add it in the UART node also but we were able to
 work around it so far.



 Signed-off-by: Simon Glass s...@chromium.org
 ---

  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
 b/arch/arm/boot/dts/bcm2835.dtsi
 index 301c73f..bd6bff6 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -8,6 +8,7 @@

   chosen {
   bootargs = earlyprintk console=ttyAMA0;
 + stdout-path = uart;
   };

   soc {
 @@ -16,6 +17,7 @@
   #size-cells = 1;
   ranges = 0x7e00 0x2000 0x0200;
   dma-ranges = 0x4000 0x 0x2000;
 + u-boot,dm-pre-reloc;

 Why do you need this and why should upstream carry your favourite
 bootloaders configuration? This is in no way hardware description.

 I'm not sure how much you know about U-Boot, so let me know if you
 need more info.

 U-Boot normally starts up by setting up its serial UART and displaying
 a banner message. At this stage typically only a few devices are
 initialised (e.g. maybe just the UART). It then relocates itself to
 the top of memory and starts up all the devices. It throws away any
 previous devices that it set up before relocation and starts again.

 U-Boot uses a thing called driver model (dm) which handles driver
 binding and probing. Driver model has the device tree and would
 normally scan through it and create devices for everything it finds.

 How do you debug the DM itself? It seems like you still would need
 something earlier for debug like earlycon in the kernel. u-boot DM is
 probably simple enough you can get away with using it early, but you
 often can't as the complexity increases. Ultimately you need something
 simple that just hits all the registers needed to get characters out.
 What happens when you add pinmux, clocks, PMIC, power domains, etc. to
 the DM and they all become dependencies for the UART?

 This doesn't seem like a device tree binding question but let me try
 to answer it.

 This is a problem - one of the challenges of driver model is that the
 UART gets further away from the reset vector.

 This is a common problem for any complex embedded system. Nothing
 special about u-boot here. So either we don't need anything because
 everyone else has dealt with the problem in some way or we need a
 common solution because we all have this problem.

I'm struggling to understand the value of this statement - is it just
philosophizing? I do have a real problem and would like to solve it.
Please make a concrete suggestion.


 In U-Boot there is a single debug UART which can be supported by the
 various serial drivers (only one can be selected on each platform).
 There is a special debug_uart_init() function which is supposed to set
 up the UART for that platform. After that, and until driver model UART
 is up, console output can go through the debug UART.

 So you don't need this for the common case of an early console, but
 only for platforms needing some other platform specific setup?

The debug UART is generally disabled and has very limited use. It is
hard-coded and does not use the device tree. I mentioned it because
you asked How do you debug the DM itself. You dropped that from the
context so this thread is now quite confusing.

Let's ignore the debug UART.


 Before relocation we don't need every device. Also the CPU is often
 running slowly, perhaps without the cache enabled. SDRAM may not be
 available yet so space is short. We want to avoid starting up things
 that will not be used.

 So this property indicates that the device is needed before relocation
 and should be set up by driver model. We need it to avoid a very slow
 and memory-hungry startup.

 Can't the need for that property change over time? Either as more
 drivers are converted to DM you need to add this or you add some
 feature

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Stephen,

On 14 August 2015 at 21:46, Stephen Warren  wrote:
> On 08/12/2015 07:21 AM, Simon Glass wrote:
>> Hi Lucas,
>>
>> On 11 August 2015 at 11:05, Lucas Stach  wrote:
>>> Hi Simon,
>>>
>>> why did you send this to the Tegra ML?
>>>
>>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>>>> This updates the device tree from the kernel version to something suitable
>>>> for U-Boot:
>>>>
>>>> - Add stdout-path alias for console
>>>> - Mark the /soc node to be available pre-relocation so that the early
>>>> serial console works (we need the 'ranges' property to be available)
>>>>
>>>> Signed-off-by: Simon Glass 
>>>> ---
>>>>
>>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
>>>> b/arch/arm/boot/dts/bcm2835.dtsi
>>>> index 301c73f..bd6bff6 100644
>>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>>>> @@ -8,6 +8,7 @@
>>>>
>>>>   chosen {
>>>>   bootargs = "earlyprintk console=ttyAMA0";
>>>> + stdout-path = 
>>>>   };
>>>>
>>>>   soc {
>>>> @@ -16,6 +17,7 @@
>>>>   #size-cells = <1>;
>>>>   ranges = <0x7e00 0x2000 0x0200>;
>>>>   dma-ranges = <0x4000 0x 0x2000>;
>>>> + u-boot,dm-pre-reloc;
>>>
>>> Why do you need this and why should upstream carry your favourite
>>> bootloaders configuration? This is in no way hardware description.
>>
>> I'm not sure how much you know about U-Boot, so let me know if you
>> need more info.
>>
>> U-Boot normally starts up by setting up its serial UART and displaying
>> a banner message. At this stage typically only a few devices are
>> initialised (e.g. maybe just the UART). It then relocates itself to
>> the top of memory and starts up all the devices. It throws away any
>> previous devices that it set up before relocation and starts again.
>>
>> U-Boot uses a thing called driver model (dm) which handles driver
>> binding and probing. Driver model has the device tree and would
>> normally scan through it and create devices for everything it finds.
>>
>> Before relocation we don't need every device. Also the CPU is often
>> running slowly, perhaps without the cache enabled. SDRAM may not be
>> available yet so space is short. We want to avoid starting up things
>> that will not be used.
>>
>> So this property indicates that the device is needed before relocation
>> and should be set up by driver model. We need it to avoid a very slow
>> and memory-hungry startup.
>>
>> As to why upstream should accept it, my understanding of upstream is
>> that people can send patches to it and in fact are encouraged to do
>> so, to avoid misunderstandings and duplication. The device tree files
>> are stored in Linux so any binding or source file changes should end
>> up there. Otherwise the files tend to diverge and we end up with
>> multiple bindings and multiple versions of the same source file.
>
> On many platforms, we have U-Boot SPL running first, then the main
> U-Boot. The main U-Boot binary contains both the code to do the
> relocation and the binary that runs after relocation. It seems like it'd
> be simpler to split these up into 3 binaries that each do a single job:
>
> 1) SPL, roughly as-is today (varying jobs depending on platform)
>
> 2) Relocator, which does nothing but work out where to copy U-Boot,
> memcpy()s it there, relocates the image (if not PIE), and jumps to it.
>
> 3) The main U-Boot.
>
> Item (2) above should be simple enough that it can use a very simple
> debug mechanism rather like DEBUG_LL in the Linux kernel. Similar to
> what Rob mentioned in his email.
>
> Item (3) could use DM and DT/ACPI/... to get device information in a
> complete non-hard-coded manner.

This comment does no seem to relate to my patch. We could certainly
re-architect U-Boot to work this way. There are  lot of reasons why
U-Boot works as it does and many platforms don't have SPL.

Relating what you said to the current U-Boot, your item (2) is
analogous to us not setting up driver model before relocation at all,
and just having a debug UART. That's a huge topic though, well beyond
the scope of my original patch. I think it would be better for you to
start a thread on the U-Boot mailing list with your proposal. At least
on x86 (which has no SPL) there are all sorts of things that currently
happen before relocation.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Rob,

On 14 August 2015 at 14:29, Rob Herring  wrote:
> On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass  wrote:
>> -linux-tegra
>>
>> Hi,
>>
>> On 12 August 2015 at 07:21, Simon Glass  wrote:
>>> Hi Lucas,
>>>
>>> On 11 August 2015 at 11:05, Lucas Stach  wrote:
>>>> Hi Simon,
>>>>
>>>> why did you send this to the Tegra ML?
>>>>
>>>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>>>>> This updates the device tree from the kernel version to something suitable
>>>>> for U-Boot:
>>>>>
>>>>> - Add stdout-path alias for console
>>>>> - Mark the /soc node to be available pre-relocation so that the early
>>>>> serial console works (we need the 'ranges' property to be available)
>
> I find it quite strange that you must explicitly enable the parent
> node, but not the uart node.

U-Boot tries hard to find and bind the UART using the stdout-path
link. I would like to add it in the UART node also but we were able to
work around it so far.

>
>>>>>
>>>>> Signed-off-by: Simon Glass 
>>>>> ---
>>>>>
>>>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
>>>>> b/arch/arm/boot/dts/bcm2835.dtsi
>>>>> index 301c73f..bd6bff6 100644
>>>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>>>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>>>>> @@ -8,6 +8,7 @@
>>>>>
>>>>>   chosen {
>>>>>   bootargs = "earlyprintk console=ttyAMA0";
>>>>> + stdout-path = 
>>>>>   };
>>>>>
>>>>>   soc {
>>>>> @@ -16,6 +17,7 @@
>>>>>   #size-cells = <1>;
>>>>>   ranges = <0x7e00 0x2000 0x0200>;
>>>>>   dma-ranges = <0x4000 0x 0x2000>;
>>>>> + u-boot,dm-pre-reloc;
>>>>
>>>> Why do you need this and why should upstream carry your favourite
>>>> bootloaders configuration? This is in no way hardware description.
>>>
>>> I'm not sure how much you know about U-Boot, so let me know if you
>>> need more info.
>>>
>>> U-Boot normally starts up by setting up its serial UART and displaying
>>> a banner message. At this stage typically only a few devices are
>>> initialised (e.g. maybe just the UART). It then relocates itself to
>>> the top of memory and starts up all the devices. It throws away any
>>> previous devices that it set up before relocation and starts again.
>>>
>>> U-Boot uses a thing called driver model (dm) which handles driver
>>> binding and probing. Driver model has the device tree and would
>>> normally scan through it and create devices for everything it finds.
>
> How do you debug the DM itself? It seems like you still would need
> something earlier for debug like earlycon in the kernel. u-boot DM is
> probably simple enough you can get away with using it early, but you
> often can't as the complexity increases. Ultimately you need something
> simple that just hits all the registers needed to get characters out.
> What happens when you add pinmux, clocks, PMIC, power domains, etc. to
> the DM and they all become dependencies for the UART?

This doesn't seem like a device tree binding question but let me try
to answer it.

This is a problem - one of the challenges of driver model is that the
UART gets further away from the reset vector.

In U-Boot there is a single debug UART which can be supported by the
various serial drivers (only one can be selected on each platform).
There is a special debug_uart_init() function which is supposed to set
up the UART for that platform. After that, and until driver model UART
is up, console output can go through the debug UART.

>
>>> Before relocation we don't need every device. Also the CPU is often
>>> running slowly, perhaps without the cache enabled. SDRAM may not be
>>> available yet so space is short. We want to avoid starting up things
>>> that will not be used.
>>>
>>> So this property indicates that the device is needed before relocation
>>> and should be set up by driver model. We need it to avoid a very slow
>>> and memory-hungry startup.
>
> Can't the need for that property change over time? Either as more
> driv

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Stephen,

On 14 August 2015 at 21:46, Stephen Warren swar...@wwwdotorg.org wrote:
 On 08/12/2015 07:21 AM, Simon Glass wrote:
 Hi Lucas,

 On 11 August 2015 at 11:05, Lucas Stach d...@lynxeye.de wrote:
 Hi Simon,

 why did you send this to the Tegra ML?

 Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
 This updates the device tree from the kernel version to something suitable
 for U-Boot:

 - Add stdout-path alias for console
 - Mark the /soc node to be available pre-relocation so that the early
 serial console works (we need the 'ranges' property to be available)

 Signed-off-by: Simon Glass s...@chromium.org
 ---

  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
 b/arch/arm/boot/dts/bcm2835.dtsi
 index 301c73f..bd6bff6 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -8,6 +8,7 @@

   chosen {
   bootargs = earlyprintk console=ttyAMA0;
 + stdout-path = uart;
   };

   soc {
 @@ -16,6 +17,7 @@
   #size-cells = 1;
   ranges = 0x7e00 0x2000 0x0200;
   dma-ranges = 0x4000 0x 0x2000;
 + u-boot,dm-pre-reloc;

 Why do you need this and why should upstream carry your favourite
 bootloaders configuration? This is in no way hardware description.

 I'm not sure how much you know about U-Boot, so let me know if you
 need more info.

 U-Boot normally starts up by setting up its serial UART and displaying
 a banner message. At this stage typically only a few devices are
 initialised (e.g. maybe just the UART). It then relocates itself to
 the top of memory and starts up all the devices. It throws away any
 previous devices that it set up before relocation and starts again.

 U-Boot uses a thing called driver model (dm) which handles driver
 binding and probing. Driver model has the device tree and would
 normally scan through it and create devices for everything it finds.

 Before relocation we don't need every device. Also the CPU is often
 running slowly, perhaps without the cache enabled. SDRAM may not be
 available yet so space is short. We want to avoid starting up things
 that will not be used.

 So this property indicates that the device is needed before relocation
 and should be set up by driver model. We need it to avoid a very slow
 and memory-hungry startup.

 As to why upstream should accept it, my understanding of upstream is
 that people can send patches to it and in fact are encouraged to do
 so, to avoid misunderstandings and duplication. The device tree files
 are stored in Linux so any binding or source file changes should end
 up there. Otherwise the files tend to diverge and we end up with
 multiple bindings and multiple versions of the same source file.

 On many platforms, we have U-Boot SPL running first, then the main
 U-Boot. The main U-Boot binary contains both the code to do the
 relocation and the binary that runs after relocation. It seems like it'd
 be simpler to split these up into 3 binaries that each do a single job:

 1) SPL, roughly as-is today (varying jobs depending on platform)

 2) Relocator, which does nothing but work out where to copy U-Boot,
 memcpy()s it there, relocates the image (if not PIE), and jumps to it.

 3) The main U-Boot.

 Item (2) above should be simple enough that it can use a very simple
 debug mechanism rather like DEBUG_LL in the Linux kernel. Similar to
 what Rob mentioned in his email.

 Item (3) could use DM and DT/ACPI/... to get device information in a
 complete non-hard-coded manner.

This comment does no seem to relate to my patch. We could certainly
re-architect U-Boot to work this way. There are  lot of reasons why
U-Boot works as it does and many platforms don't have SPL.

Relating what you said to the current U-Boot, your item (2) is
analogous to us not setting up driver model before relocation at all,
and just having a debug UART. That's a huge topic though, well beyond
the scope of my original patch. I think it would be better for you to
start a thread on the U-Boot mailing list with your proposal. At least
on x86 (which has no SPL) there are all sorts of things that currently
happen before relocation.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Rob,

On 14 August 2015 at 14:29, Rob Herring robherri...@gmail.com wrote:
 On Fri, Aug 14, 2015 at 1:34 PM, Simon Glass s...@chromium.org wrote:
 -linux-tegra

 Hi,

 On 12 August 2015 at 07:21, Simon Glass s...@chromium.org wrote:
 Hi Lucas,

 On 11 August 2015 at 11:05, Lucas Stach d...@lynxeye.de wrote:
 Hi Simon,

 why did you send this to the Tegra ML?

 Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
 This updates the device tree from the kernel version to something suitable
 for U-Boot:

 - Add stdout-path alias for console
 - Mark the /soc node to be available pre-relocation so that the early
 serial console works (we need the 'ranges' property to be available)

 I find it quite strange that you must explicitly enable the parent
 node, but not the uart node.

U-Boot tries hard to find and bind the UART using the stdout-path
link. I would like to add it in the UART node also but we were able to
work around it so far.



 Signed-off-by: Simon Glass s...@chromium.org
 ---

  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/boot/dts/bcm2835.dtsi 
 b/arch/arm/boot/dts/bcm2835.dtsi
 index 301c73f..bd6bff6 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -8,6 +8,7 @@

   chosen {
   bootargs = earlyprintk console=ttyAMA0;
 + stdout-path = uart;
   };

   soc {
 @@ -16,6 +17,7 @@
   #size-cells = 1;
   ranges = 0x7e00 0x2000 0x0200;
   dma-ranges = 0x4000 0x 0x2000;
 + u-boot,dm-pre-reloc;

 Why do you need this and why should upstream carry your favourite
 bootloaders configuration? This is in no way hardware description.

 I'm not sure how much you know about U-Boot, so let me know if you
 need more info.

 U-Boot normally starts up by setting up its serial UART and displaying
 a banner message. At this stage typically only a few devices are
 initialised (e.g. maybe just the UART). It then relocates itself to
 the top of memory and starts up all the devices. It throws away any
 previous devices that it set up before relocation and starts again.

 U-Boot uses a thing called driver model (dm) which handles driver
 binding and probing. Driver model has the device tree and would
 normally scan through it and create devices for everything it finds.

 How do you debug the DM itself? It seems like you still would need
 something earlier for debug like earlycon in the kernel. u-boot DM is
 probably simple enough you can get away with using it early, but you
 often can't as the complexity increases. Ultimately you need something
 simple that just hits all the registers needed to get characters out.
 What happens when you add pinmux, clocks, PMIC, power domains, etc. to
 the DM and they all become dependencies for the UART?

This doesn't seem like a device tree binding question but let me try
to answer it.

This is a problem - one of the challenges of driver model is that the
UART gets further away from the reset vector.

In U-Boot there is a single debug UART which can be supported by the
various serial drivers (only one can be selected on each platform).
There is a special debug_uart_init() function which is supposed to set
up the UART for that platform. After that, and until driver model UART
is up, console output can go through the debug UART.


 Before relocation we don't need every device. Also the CPU is often
 running slowly, perhaps without the cache enabled. SDRAM may not be
 available yet so space is short. We want to avoid starting up things
 that will not be used.

 So this property indicates that the device is needed before relocation
 and should be set up by driver model. We need it to avoid a very slow
 and memory-hungry startup.

 Can't the need for that property change over time? Either as more
 drivers are converted to DM you need to add this or you add some
 feature that depends on a driver (e.g. get a board rev or boot mode
 from GPIO). You would have backwards compatibility issues with this.
 I'm somewhat less worried about that for u-boot as we should be
 bundling the dtb and bootloader rather than kernel and dtb. For the
 UART, you can just get which UART to initialize early from
 stdout-path. But for other cases, couldn't you just have the platform
 provide the list of needed drivers. Then when u-boot needs change, you
 just change u-boot.

Yes U-Boot and its device tree are normally built from the same tree
at the same time. We don't expect to have to support an older or newer
device tree with the same U-Boot binary. So I don't see a problem
here.

The UART should have as few dependencies as possible. But if the
particular platform needs to check a GPIO to find out the UART number
(i.e. stdout-path cannot be used) then we would need to support that
case. It hasn't come up yet but it might. I certainly see platforms
where we need some other nodes before

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

-linux-tegra

Hi,

On 12 August 2015 at 07:21, Simon Glass  wrote:
> Hi Lucas,
>
> On 11 August 2015 at 11:05, Lucas Stach  wrote:
>> Hi Simon,
>>
>> why did you send this to the Tegra ML?
>>
>> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>>> This updates the device tree from the kernel version to something suitable
>>> for U-Boot:
>>>
>>> - Add stdout-path alias for console
>>> - Mark the /soc node to be available pre-relocation so that the early
>>> serial console works (we need the 'ranges' property to be available)
>>>
>>> Signed-off-by: Simon Glass 
>>> ---
>>>
>>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
>>> index 301c73f..bd6bff6 100644
>>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>>> @@ -8,6 +8,7 @@
>>>
>>>   chosen {
>>>   bootargs = "earlyprintk console=ttyAMA0";
>>> + stdout-path = 
>>>   };
>>>
>>>   soc {
>>> @@ -16,6 +17,7 @@
>>>   #size-cells = <1>;
>>>   ranges = <0x7e00 0x2000 0x0200>;
>>>   dma-ranges = <0x4000 0x 0x2000>;
>>> + u-boot,dm-pre-reloc;
>>
>> Why do you need this and why should upstream carry your favourite
>> bootloaders configuration? This is in no way hardware description.
>
> I'm not sure how much you know about U-Boot, so let me know if you
> need more info.
>
> U-Boot normally starts up by setting up its serial UART and displaying
> a banner message. At this stage typically only a few devices are
> initialised (e.g. maybe just the UART). It then relocates itself to
> the top of memory and starts up all the devices. It throws away any
> previous devices that it set up before relocation and starts again.
>
> U-Boot uses a thing called driver model (dm) which handles driver
> binding and probing. Driver model has the device tree and would
> normally scan through it and create devices for everything it finds.
>
> Before relocation we don't need every device. Also the CPU is often
> running slowly, perhaps without the cache enabled. SDRAM may not be
> available yet so space is short. We want to avoid starting up things
> that will not be used.
>
> So this property indicates that the device is needed before relocation
> and should be set up by driver model. We need it to avoid a very slow
> and memory-hungry startup.
>
> As to why upstream should accept it, my understanding of upstream is
> that people can send patches to it and in fact are encouraged to do
> so, to avoid misunderstandings and duplication. The device tree files
> are stored in Linux so any binding or source file changes should end
> up there. Otherwise the files tend to diverge and we end up with
> multiple bindings and multiple versions of the same source file.

Is the above explanation sufficient? Will this patch be applied?

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

-linux-tegra

Hi,

On 12 August 2015 at 07:21, Simon Glass s...@chromium.org wrote:
 Hi Lucas,

 On 11 August 2015 at 11:05, Lucas Stach d...@lynxeye.de wrote:
 Hi Simon,

 why did you send this to the Tegra ML?

 Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
 This updates the device tree from the kernel version to something suitable
 for U-Boot:

 - Add stdout-path alias for console
 - Mark the /soc node to be available pre-relocation so that the early
 serial console works (we need the 'ranges' property to be available)

 Signed-off-by: Simon Glass s...@chromium.org
 ---

  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
 index 301c73f..bd6bff6 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -8,6 +8,7 @@

   chosen {
   bootargs = earlyprintk console=ttyAMA0;
 + stdout-path = uart;
   };

   soc {
 @@ -16,6 +17,7 @@
   #size-cells = 1;
   ranges = 0x7e00 0x2000 0x0200;
   dma-ranges = 0x4000 0x 0x2000;
 + u-boot,dm-pre-reloc;

 Why do you need this and why should upstream carry your favourite
 bootloaders configuration? This is in no way hardware description.

 I'm not sure how much you know about U-Boot, so let me know if you
 need more info.

 U-Boot normally starts up by setting up its serial UART and displaying
 a banner message. At this stage typically only a few devices are
 initialised (e.g. maybe just the UART). It then relocates itself to
 the top of memory and starts up all the devices. It throws away any
 previous devices that it set up before relocation and starts again.

 U-Boot uses a thing called driver model (dm) which handles driver
 binding and probing. Driver model has the device tree and would
 normally scan through it and create devices for everything it finds.

 Before relocation we don't need every device. Also the CPU is often
 running slowly, perhaps without the cache enabled. SDRAM may not be
 available yet so space is short. We want to avoid starting up things
 that will not be used.

 So this property indicates that the device is needed before relocation
 and should be set up by driver model. We need it to avoid a very slow
 and memory-hungry startup.

 As to why upstream should accept it, my understanding of upstream is
 that people can send patches to it and in fact are encouraged to do
 so, to avoid misunderstandings and duplication. The device tree files
 are stored in Linux so any binding or source file changes should end
 up there. Otherwise the files tend to diverge and we end up with
 multiple bindings and multiple versions of the same source file.

Is the above explanation sufficient? Will this patch be applied?

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Lucas,

On 11 August 2015 at 11:05, Lucas Stach  wrote:
> Hi Simon,
>
> why did you send this to the Tegra ML?
>
> Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
>> This updates the device tree from the kernel version to something suitable
>> for U-Boot:
>>
>> - Add stdout-path alias for console
>> - Mark the /soc node to be available pre-relocation so that the early
>> serial console works (we need the 'ranges' property to be available)
>>
>> Signed-off-by: Simon Glass 
>> ---
>>
>>  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
>> index 301c73f..bd6bff6 100644
>> --- a/arch/arm/boot/dts/bcm2835.dtsi
>> +++ b/arch/arm/boot/dts/bcm2835.dtsi
>> @@ -8,6 +8,7 @@
>>
>>   chosen {
>>   bootargs = "earlyprintk console=ttyAMA0";
>> + stdout-path = 
>>   };
>>
>>   soc {
>> @@ -16,6 +17,7 @@
>>   #size-cells = <1>;
>>   ranges = <0x7e00 0x2000 0x0200>;
>>   dma-ranges = <0x4000 0x 0x2000>;
>> + u-boot,dm-pre-reloc;
>
> Why do you need this and why should upstream carry your favourite
> bootloaders configuration? This is in no way hardware description.

I'm not sure how much you know about U-Boot, so let me know if you
need more info.

U-Boot normally starts up by setting up its serial UART and displaying
a banner message. At this stage typically only a few devices are
initialised (e.g. maybe just the UART). It then relocates itself to
the top of memory and starts up all the devices. It throws away any
previous devices that it set up before relocation and starts again.

U-Boot uses a thing called driver model (dm) which handles driver
binding and probing. Driver model has the device tree and would
normally scan through it and create devices for everything it finds.

Before relocation we don't need every device. Also the CPU is often
running slowly, perhaps without the cache enabled. SDRAM may not be
available yet so space is short. We want to avoid starting up things
that will not be used.

So this property indicates that the device is needed before relocation
and should be set up by driver model. We need it to avoid a very slow
and memory-hungry startup.

As to why upstream should accept it, my understanding of upstream is
that people can send patches to it and in fact are encouraged to do
so, to avoid misunderstandings and duplication. The device tree files
are stored in Linux so any binding or source file changes should end
up there. Otherwise the files tend to diverge and we end up with
multiple bindings and multiple versions of the same source file.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

Hi Lucas,

On 11 August 2015 at 11:05, Lucas Stach d...@lynxeye.de wrote:
 Hi Simon,

 why did you send this to the Tegra ML?

 Am Dienstag, den 11.08.2015, 08:25 -0600 schrieb Simon Glass:
 This updates the device tree from the kernel version to something suitable
 for U-Boot:

 - Add stdout-path alias for console
 - Mark the /soc node to be available pre-relocation so that the early
 serial console works (we need the 'ranges' property to be available)

 Signed-off-by: Simon Glass s...@chromium.org
 ---

  arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
 index 301c73f..bd6bff6 100644
 --- a/arch/arm/boot/dts/bcm2835.dtsi
 +++ b/arch/arm/boot/dts/bcm2835.dtsi
 @@ -8,6 +8,7 @@

   chosen {
   bootargs = earlyprintk console=ttyAMA0;
 + stdout-path = uart;
   };

   soc {
 @@ -16,6 +17,7 @@
   #size-cells = 1;
   ranges = 0x7e00 0x2000 0x0200;
   dma-ranges = 0x4000 0x 0x2000;
 + u-boot,dm-pre-reloc;

 Why do you need this and why should upstream carry your favourite
 bootloaders configuration? This is in no way hardware description.

I'm not sure how much you know about U-Boot, so let me know if you
need more info.

U-Boot normally starts up by setting up its serial UART and displaying
a banner message. At this stage typically only a few devices are
initialised (e.g. maybe just the UART). It then relocates itself to
the top of memory and starts up all the devices. It throws away any
previous devices that it set up before relocation and starts again.

U-Boot uses a thing called driver model (dm) which handles driver
binding and probing. Driver model has the device tree and would
normally scan through it and create devices for everything it finds.

Before relocation we don't need every device. Also the CPU is often
running slowly, perhaps without the cache enabled. SDRAM may not be
available yet so space is short. We want to avoid starting up things
that will not be used.

So this property indicates that the device is needed before relocation
and should be set up by driver model. We need it to avoid a very slow
and memory-hungry startup.

As to why upstream should accept it, my understanding of upstream is
that people can send patches to it and in fact are encouraged to do
so, to avoid misunderstandings and duplication. The device tree files
are stored in Linux so any binding or source file changes should end
up there. Otherwise the files tend to diverge and we end up with
multiple bindings and multiple versions of the same source file.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

This updates the device tree from the kernel version to something suitable
for U-Boot:

- Add stdout-path alias for console
- Mark the /soc node to be available pre-relocation so that the early
serial console works (we need the 'ranges' property to be available)

Signed-off-by: Simon Glass 
---

 arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 301c73f..bd6bff6 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -8,6 +8,7 @@
 
chosen {
bootargs = "earlyprintk console=ttyAMA0";
+   stdout-path = 
};
 
soc {
@@ -16,6 +17,7 @@
#size-cells = <1>;
ranges = <0x7e00 0x2000 0x0200>;
dma-ranges = <0x4000 0x 0x2000>;
+   u-boot,dm-pre-reloc;
 
timer@7e003000 {
compatible = "brcm,bcm2835-system-timer";
@@ -92,7 +94,7 @@
#interrupt-cells = <2>;
};
 
-   uart@7e201000 {
+   uart: uart@7e201000 {
compatible = "brcm,bcm2835-pl011", "arm,pl011", 
"arm,primecell";
reg = <0x7e201000 0x1000>;
interrupts = <2 25>;
-- 
2.5.0.rc2.392.g76e840b

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] arm: rpi: Device tree modifications for U-Boot

[Simon Glass]

This updates the device tree from the kernel version to something suitable
for U-Boot:

- Add stdout-path alias for console
- Mark the /soc node to be available pre-relocation so that the early
serial console works (we need the 'ranges' property to be available)

Signed-off-by: Simon Glass s...@chromium.org
---

 arch/arm/boot/dts/bcm2835.dtsi | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 301c73f..bd6bff6 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -8,6 +8,7 @@
 
chosen {
bootargs = earlyprintk console=ttyAMA0;
+   stdout-path = uart;
};
 
soc {
@@ -16,6 +17,7 @@
#size-cells = 1;
ranges = 0x7e00 0x2000 0x0200;
dma-ranges = 0x4000 0x 0x2000;
+   u-boot,dm-pre-reloc;
 
timer@7e003000 {
compatible = brcm,bcm2835-system-timer;
@@ -92,7 +94,7 @@
#interrupt-cells = 2;
};
 
-   uart@7e201000 {
+   uart: uart@7e201000 {
compatible = brcm,bcm2835-pl011, arm,pl011, 
arm,primecell;
reg = 0x7e201000 0x1000;
interrupts = 2 25;
-- 
2.5.0.rc2.392.g76e840b

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ARM: tegra: Enable TPM on tegra124 nyan boards

[Simon Glass]

Regenerate the pinmux from the latest tegra-pinmux-scripts.

Signed-off-by: Simon Glass 
---

 arch/arm/boot/dts/tegra124-nyan-big.dts   | 22 +++---
 arch/arm/boot/dts/tegra124-nyan-blaze.dts | 30 +++---
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts 
b/arch/arm/boot/dts/tegra124-nyan-big.dts
index 2d21253..0eb2b0f 100644
--- a/arch/arm/boot/dts/tegra124-nyan-big.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-big.dts
@@ -30,7 +30,7 @@
pinctrl-names = "default";
pinctrl-0 = <_default>;
 
-   pinmux_default: common {
+   state_default: pinmux {
clk_32k_out_pa0 {
nvidia,pins = "clk_32k_out_pa0";
nvidia,pull = ;
@@ -1098,19 +1098,19 @@
};
cam_i2c_scl_pbb1 {
nvidia,pins = "cam_i2c_scl_pbb1";
-   nvidia,function = "rsvd3";
-   nvidia,pull = ;
-   nvidia,tristate = ;
-   nvidia,enable-input = ;
-   nvidia,open-drain = ;
+   nvidia,function = "i2c3";
+   nvidia,pull = ;
+   nvidia,tristate = ;
+   nvidia,enable-input = ;
+   nvidia,open-drain = ;
};
cam_i2c_sda_pbb2 {
nvidia,pins = "cam_i2c_sda_pbb2";
-   nvidia,function = "rsvd3";
-   nvidia,pull = ;
-   nvidia,tristate = ;
-   nvidia,enable-input = ;
-   nvidia,open-drain = ;
+   nvidia,function = "i2c3";
+   nvidia,pull = ;
+   nvidia,tristate = ;
+   nvidia,enable-input = ;
+   nvidia,open-drain = ;
};
pbb3 {
nvidia,pins = "pbb3";
diff --git a/arch/arm/boot/dts/tegra124-nyan-blaze.dts 
b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
index 0d30c51..1fc0da9 100644
--- a/arch/arm/boot/dts/tegra124-nyan-blaze.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
@@ -26,7 +26,7 @@
pinctrl-names = "default";
pinctrl-0 = <_default>;
 
-   pinmux_default: common {
+   state_default: pinmux {
clk_32k_out_pa0 {
nvidia,pins = "clk_32k_out_pa0";
nvidia,pull = ;
@@ -437,18 +437,18 @@
usb_vbus_en0_pn4 {
nvidia,pins = "usb_vbus_en0_pn4";
nvidia,function = "usb";
-   nvidia,pull = ;
+   nvidia,pull = ;
nvidia,tristate = ;
nvidia,enable-input = ;
-   nvidia,open-drain = ;
+   nvidia,open-drain = ;
};
usb_vbus_en1_pn5 {
nvidia,pins = "usb_vbus_en1_pn5";
nvidia,function = "usb";
-   nvidia,pull = ;
+   nvidia,pull = ;
nvidia,tristate = ;
nvidia,enable-input = ;
-   nvidia,open-drain = ;
+   nvidia,open-drain = ;
};
hdmi_int_pn7 {
nvidia,pins = "hdmi_int_pn7";
@@ -1094,19 +1094,19 @@
};
cam_i2c_scl_pbb1 {
nvidia,pins = "cam_i2c_scl_pbb1";
-   nvidia,function = "rsvd3";
-   nvidia,pull = ;
-   nvidia,tristate = ;
-   nvidia,enable-input = ;
-   nvidia,open-drain = ;
+   nvidia,function = "i2c3";
+   nvidia,pull = ;
+   nvidia,tristate = ;
+   nvidia,enable-input = ;
+   nvidia,open-drain = ;
};
 

[pinmux scripts PATCH] Support TPM on nyan boards

[Simon Glass]

There is a TPM on I2C3, so set up the pinmux for that.

Signed-off-by: Simon Glass 
---

 configs/nyan-big.board   | 4 ++--
 configs/nyan-blaze.board | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configs/nyan-big.board b/configs/nyan-big.board
index 6ebe466..18c2e52 100644
--- a/configs/nyan-big.board
+++ b/configs/nyan-big.board
@@ -40,8 +40,8 @@ pins = (
 ('ulpi_dir_py1',   'spi1',None,  'none', False, True,  
False, False),
 ('ulpi_nxt_py2',   'spi1',None,  'none', False, False, 
False, False),
 ('ulpi_stp_py3',   'spi1',None,  'none', False, False, 
False, False),
-('cam_i2c_scl_pbb1',   'rsvd3',   None,  'down', True,  False, 
False, False),
-('cam_i2c_sda_pbb2',   'rsvd3',   None,  'down', True,  False, 
False, False),
+('cam_i2c_scl_pbb1',   'i2c3',None,  'none', False, True,  
True,  False),
+('cam_i2c_sda_pbb2',   'i2c3',None,  'none', False, True,  
True,  False),
 ('cam_mclk_pcc0',  'vi',  None,  'down', True,  False, 
False, False),
 ('pbb0',   'vgp6',None,  'down', True,  False, 
False, False),
 ('pbb3',   'vgp3',None,  'down', True,  False, 
False, False),
diff --git a/configs/nyan-blaze.board b/configs/nyan-blaze.board
index 39a2022..eee472f 100644
--- a/configs/nyan-blaze.board
+++ b/configs/nyan-blaze.board
@@ -40,8 +40,8 @@ pins = (
 ('ulpi_dir_py1',   'spi1',None,  'none', False, True,  
False, False),
 ('ulpi_nxt_py2',   'spi1',None,  'none', False, False, 
False, False),
 ('ulpi_stp_py3',   'spi1',None,  'none', False, False, 
False, False),
-('cam_i2c_scl_pbb1',   'rsvd3',   None,  'down', True,  False, 
False, False),
-('cam_i2c_sda_pbb2',   'rsvd3',   None,  'down', True,  False, 
False, False),
+('cam_i2c_scl_pbb1',   'i2c3',None,  'none', False, True,  
True,  False),
+('cam_i2c_sda_pbb2',   'i2c3',None,  'none', False, True,  
True,  False),
 ('cam_mclk_pcc0',  'vi',  None,  'down', True,  False, 
False, False),
 ('pbb0',   'vgp6',None,  'down', True,  False, 
False, False),
 ('pbb3',   'vgp3',None,  'down', True,  False, 
False, False),
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[pinmux scripts PATCH] Support TPM on nyan boards

[Simon Glass]

There is a TPM on I2C3, so set up the pinmux for that.

Signed-off-by: Simon Glass s...@chromium.org
---

 configs/nyan-big.board   | 4 ++--
 configs/nyan-blaze.board | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configs/nyan-big.board b/configs/nyan-big.board
index 6ebe466..18c2e52 100644
--- a/configs/nyan-big.board
+++ b/configs/nyan-big.board
@@ -40,8 +40,8 @@ pins = (
 ('ulpi_dir_py1',   'spi1',None,  'none', False, True,  
False, False),
 ('ulpi_nxt_py2',   'spi1',None,  'none', False, False, 
False, False),
 ('ulpi_stp_py3',   'spi1',None,  'none', False, False, 
False, False),
-('cam_i2c_scl_pbb1',   'rsvd3',   None,  'down', True,  False, 
False, False),
-('cam_i2c_sda_pbb2',   'rsvd3',   None,  'down', True,  False, 
False, False),
+('cam_i2c_scl_pbb1',   'i2c3',None,  'none', False, True,  
True,  False),
+('cam_i2c_sda_pbb2',   'i2c3',None,  'none', False, True,  
True,  False),
 ('cam_mclk_pcc0',  'vi',  None,  'down', True,  False, 
False, False),
 ('pbb0',   'vgp6',None,  'down', True,  False, 
False, False),
 ('pbb3',   'vgp3',None,  'down', True,  False, 
False, False),
diff --git a/configs/nyan-blaze.board b/configs/nyan-blaze.board
index 39a2022..eee472f 100644
--- a/configs/nyan-blaze.board
+++ b/configs/nyan-blaze.board
@@ -40,8 +40,8 @@ pins = (
 ('ulpi_dir_py1',   'spi1',None,  'none', False, True,  
False, False),
 ('ulpi_nxt_py2',   'spi1',None,  'none', False, False, 
False, False),
 ('ulpi_stp_py3',   'spi1',None,  'none', False, False, 
False, False),
-('cam_i2c_scl_pbb1',   'rsvd3',   None,  'down', True,  False, 
False, False),
-('cam_i2c_sda_pbb2',   'rsvd3',   None,  'down', True,  False, 
False, False),
+('cam_i2c_scl_pbb1',   'i2c3',None,  'none', False, True,  
True,  False),
+('cam_i2c_sda_pbb2',   'i2c3',None,  'none', False, True,  
True,  False),
 ('cam_mclk_pcc0',  'vi',  None,  'down', True,  False, 
False, False),
 ('pbb0',   'vgp6',None,  'down', True,  False, 
False, False),
 ('pbb3',   'vgp3',None,  'down', True,  False, 
False, False),
-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ARM: tegra: Enable TPM on tegra124 nyan boards

[Simon Glass]

Regenerate the pinmux from the latest tegra-pinmux-scripts.

Signed-off-by: Simon Glass s...@chromium.org
---

 arch/arm/boot/dts/tegra124-nyan-big.dts   | 22 +++---
 arch/arm/boot/dts/tegra124-nyan-blaze.dts | 30 +++---
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts 
b/arch/arm/boot/dts/tegra124-nyan-big.dts
index 2d21253..0eb2b0f 100644
--- a/arch/arm/boot/dts/tegra124-nyan-big.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-big.dts
@@ -30,7 +30,7 @@
pinctrl-names = default;
pinctrl-0 = pinmux_default;
 
-   pinmux_default: common {
+   state_default: pinmux {
clk_32k_out_pa0 {
nvidia,pins = clk_32k_out_pa0;
nvidia,pull = TEGRA_PIN_PULL_NONE;
@@ -1098,19 +1098,19 @@
};
cam_i2c_scl_pbb1 {
nvidia,pins = cam_i2c_scl_pbb1;
-   nvidia,function = rsvd3;
-   nvidia,pull = TEGRA_PIN_PULL_DOWN;
-   nvidia,tristate = TEGRA_PIN_ENABLE;
-   nvidia,enable-input = TEGRA_PIN_DISABLE;
-   nvidia,open-drain = TEGRA_PIN_DISABLE;
+   nvidia,function = i2c3;
+   nvidia,pull = TEGRA_PIN_PULL_NONE;
+   nvidia,tristate = TEGRA_PIN_DISABLE;
+   nvidia,enable-input = TEGRA_PIN_ENABLE;
+   nvidia,open-drain = TEGRA_PIN_ENABLE;
};
cam_i2c_sda_pbb2 {
nvidia,pins = cam_i2c_sda_pbb2;
-   nvidia,function = rsvd3;
-   nvidia,pull = TEGRA_PIN_PULL_DOWN;
-   nvidia,tristate = TEGRA_PIN_ENABLE;
-   nvidia,enable-input = TEGRA_PIN_DISABLE;
-   nvidia,open-drain = TEGRA_PIN_DISABLE;
+   nvidia,function = i2c3;
+   nvidia,pull = TEGRA_PIN_PULL_NONE;
+   nvidia,tristate = TEGRA_PIN_DISABLE;
+   nvidia,enable-input = TEGRA_PIN_ENABLE;
+   nvidia,open-drain = TEGRA_PIN_ENABLE;
};
pbb3 {
nvidia,pins = pbb3;
diff --git a/arch/arm/boot/dts/tegra124-nyan-blaze.dts 
b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
index 0d30c51..1fc0da9 100644
--- a/arch/arm/boot/dts/tegra124-nyan-blaze.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
@@ -26,7 +26,7 @@
pinctrl-names = default;
pinctrl-0 = pinmux_default;
 
-   pinmux_default: common {
+   state_default: pinmux {
clk_32k_out_pa0 {
nvidia,pins = clk_32k_out_pa0;
nvidia,pull = TEGRA_PIN_PULL_NONE;
@@ -437,18 +437,18 @@
usb_vbus_en0_pn4 {
nvidia,pins = usb_vbus_en0_pn4;
nvidia,function = usb;
-   nvidia,pull = TEGRA_PIN_PULL_UP;
+   nvidia,pull = TEGRA_PIN_PULL_NONE;
nvidia,tristate = TEGRA_PIN_DISABLE;
nvidia,enable-input = TEGRA_PIN_ENABLE;
-   nvidia,open-drain = TEGRA_PIN_DISABLE;
+   nvidia,open-drain = TEGRA_PIN_ENABLE;
};
usb_vbus_en1_pn5 {
nvidia,pins = usb_vbus_en1_pn5;
nvidia,function = usb;
-   nvidia,pull = TEGRA_PIN_PULL_UP;
+   nvidia,pull = TEGRA_PIN_PULL_NONE;
nvidia,tristate = TEGRA_PIN_DISABLE;
nvidia,enable-input = TEGRA_PIN_ENABLE;
-   nvidia,open-drain = TEGRA_PIN_DISABLE;
+   nvidia,open-drain = TEGRA_PIN_ENABLE;
};
hdmi_int_pn7 {
nvidia,pins = hdmi_int_pn7;
@@ -1094,19 +1094,19 @@
};
cam_i2c_scl_pbb1 {
nvidia,pins = cam_i2c_scl_pbb1;
-   nvidia,function = rsvd3;
-   nvidia,pull = TEGRA_PIN_PULL_DOWN;
-   nvidia,tristate = TEGRA_PIN_ENABLE;
-   nvidia,enable-input = TEGRA_PIN_DISABLE

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 14:43, Richard Weinberger  wrote:
> Am 03.05.2015 um 22:40 schrieb Simon Glass:
>>> But I don't think it makes much sense to carry it with the Linux kernel 
>>> tree.
>>> Other projects can also use it and it does not seem to be very Linux kernel
>>> specific.
>>> git, quilt and other great tools also have their own repositories.
>>
>> My reasoning is that:
>>
>> - more will find it / use it if it is in-tree
>> - it avoids installation and old-version problems (e.g. I suppose this
>> is why the device tree compiler is built-in)
>> - it is somewhat Linux-specific (e.g. uses get_maintainers,
>> checkpatch.pl) and can break if checkpatch.pl if the wrong version
>> (e.g. you check out and send patches from an older tree)
>> - it could be built into the Linux workflow [1] and might thereby
>> reduce the amount of confusion and errors (did you run checkpatch?,
>> your change log is in the wrong place, you forgot to add your
>> sign-off, etc.)
>
> If we'd follow these arguments we'd have to move the whole GNU into the
> kernel tree. ;-)

Well maybe the first two.

> checkpatch.pl and get_maintainers.pl are not really a show-stopper.
> Other projects are using them too. You can make them also configurable.
> i.e. check_script and get_maintaner_script.

Understood, I'm just explaining my reasoning for sending this patch.
With U-Boot it has been very convenient to be able to rely on this
being available in the tree. 3000 lines is a drop in the ocean with
Linux's 22m lines.

But I fully understand your point of view. If nothing else, at least
this series provides an easy way for people to try it out.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 14:13, Richard Weinberger  wrote:
> Simon,
>
> Am 03.05.2015 um 21:54 schrieb Simon Glass:
>> Hi Richard,
>>
>> On 3 May 2015 at 13:16, Richard Weinberger  
>> wrote:
>>> On Sun, May 3, 2015 at 8:29 PM, Simon Glass  wrote:
>>>> This tool is a Python script which:
>>>> - Creates patch directly from your branch
>>>> - Cleans them up by removing unwanted tags
>>>> - Inserts a cover letter with change lists
>>>> - Runs the patches through checkpatch.pl and its own checks
>>>> - Optionally emails them out to selected people
>>>
>>> Don't get me wrong but is this really worth 3000+ lines of python?
>>> The tasks you describe can be done using a few lines bash.
>>
>> #!/bin/bash
>> patman $@
>>
>> I obviously failed in my attempt to briefly explain what it does.
>> Please check out the cover letter [1], README [2], or perhaps use it
>> on a series. With respect to the length, it could be slimmed down a
>> bit if that is important.
>
> the README file did the trick. ;)
> Sounds like a useful tool to manage patch series.
>
> But I don't think it makes much sense to carry it with the Linux kernel tree.
> Other projects can also use it and it does not seem to be very Linux kernel
> specific.
> git, quilt and other great tools also have their own repositories.

My reasoning is that:

- more will find it / use it if it is in-tree
- it avoids installation and old-version problems (e.g. I suppose this
is why the device tree compiler is built-in)
- it is somewhat Linux-specific (e.g. uses get_maintainers,
checkpatch.pl) and can break if checkpatch.pl if the wrong version
(e.g. you check out and send patches from an older tree)
- it could be built into the Linux workflow [1] and might thereby
reduce the amount of confusion and errors (did you run checkpatch?,
your change log is in the wrong place, you forgot to add your
sign-off, etc.)

That said, I could see this having a repo of its own, with the Linux
version a downstream copy, a bit like dtc (and maybe Kbuild/Kconfig -
I don't know). The feature set is probably mature enough to support
that now.

Regards,
Simon

[1] https://www.kernel.org/doc/Documentation/SubmittingPatches section 14
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 13:16, Richard Weinberger  wrote:
> On Sun, May 3, 2015 at 8:29 PM, Simon Glass  wrote:
>> This tool is a Python script which:
>> - Creates patch directly from your branch
>> - Cleans them up by removing unwanted tags
>> - Inserts a cover letter with change lists
>> - Runs the patches through checkpatch.pl and its own checks
>> - Optionally emails them out to selected people
>
> Don't get me wrong but is this really worth 3000+ lines of python?
> The tasks you describe can be done using a few lines bash.

#!/bin/bash
patman $@

I obviously failed in my attempt to briefly explain what it does.
Please check out the cover letter [1], README [2], or perhaps use it
on a series. With respect to the length, it could be slimmed down a
bit if that is important.

Regards,
Simon

[1] https://lkml.org/lkml/2015/5/3/105
[2] https://lkml.org/lkml/2015/5/3/95
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 0/3] Add a tool to simplify patch checking and posting (patman)

[Simon Glass]

Preparing, checking and sending patches to a mailing list is a tedious and
error-prone process. Dealing with multiple series, each which its own
revision history, CC list, cover letter requires concentation. Mistakes
are easy to make.

This tool aims to help with this. A single command generates the patch
series, runs it through checkpatch, generates the cover letter, adds a
change list to each patch and the cover letter, determines who should
receive the patch and Ccs people on the patch either using maintainer
information or tags in the commit subject. A dry-run mode allows output
to be checked.

This tool makes patch series repeatable, since everything needed to create
and send patches in stored in the git branch containing that series.
When making a change to one commit you can update the change log in that
commit and know that everything will turn out OK.

Specifically this tool is a Python script which (from the README):
- Creates patch directly from your branch
- Cleans them up by removing unwanted tags
- Inserts a cover letter with change lists
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people

It is intended to automate patch creation and make it a less
error-prone process. It is useful for U-Boot and Linux work so far,
since it uses the checkpatch.pl script.

It is configured almost entirely by tags it finds in your commits.
This means that you can work on a number of different branches at
once, and keep the settings with each branch rather than having to
git format-patch, git send-email, etc. with the correct parameters
each time. So for example if you put:

Series-to: fred.bl...@napier.co.nz

in one of your commits, the series will be sent there.

In Linux and U-Boot this will also call get_maintainer.pl on each of your
patches automatically (unless you use -m to disable this).

I am submitting this to LKML to raise awareness, since those who are not
involved in U-Boot probably don't know about it. I could not find a
specific linux-tools list but may have missed something.


Simon Glass (3):
  Add patman patch automation script
  Add tests for patman
  Add documentation for patman

 MAINTAINERS |   5 +
 tools/patman/.gitignore |   1 +
 tools/patman/README | 475 
 tools/patman/checkpatch.py  | 173 
 tools/patman/command.py | 123 +
 tools/patman/commit.py  |  88 ++
 tools/patman/cros_subprocess.py | 397 +++
 tools/patman/get_maintainer.py  |  47 
 tools/patman/gitutil.py | 582 
 tools/patman/patchstream.py | 488 +
 tools/patman/patman |   1 +
 tools/patman/patman.py  | 167 
 tools/patman/project.py |  27 ++
 tools/patman/series.py  | 271 +++
 tools/patman/settings.py| 295 
 tools/patman/terminal.py| 158 +++
 tools/patman/test.py| 243 +
 17 files changed, 3541 insertions(+)
 create mode 100644 tools/patman/.gitignore
 create mode 100644 tools/patman/README
 create mode 100644 tools/patman/checkpatch.py
 create mode 100644 tools/patman/command.py
 create mode 100644 tools/patman/commit.py
 create mode 100644 tools/patman/cros_subprocess.py
 create mode 100644 tools/patman/get_maintainer.py
 create mode 100644 tools/patman/gitutil.py
 create mode 100644 tools/patman/patchstream.py
 create mode 12 tools/patman/patman
 create mode 100755 tools/patman/patman.py
 create mode 100644 tools/patman/project.py
 create mode 100644 tools/patman/series.py
 create mode 100644 tools/patman/settings.py
 create mode 100644 tools/patman/terminal.py
 create mode 100644 tools/patman/test.py

-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3] Add documentation for patman

[Simon Glass]

Add an entry to the MAINTAINERS file, plus a README which explains how to
use patman.

Signed-off-by: Simon Glass 
---

 MAINTAINERS |   5 +
 tools/patman/README | 475 
 2 files changed, 480 insertions(+)
 create mode 100644 tools/patman/README

diff --git a/MAINTAINERS b/MAINTAINERS
index 1622775..cd415d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7404,6 +7404,11 @@ F:   drivers/video/fbdev/sti*
 F: drivers/video/console/sti*
 F: drivers/video/logo/logo_parisc*
 
+PATMAN PATCH AUTOMATION TOOL
+M: Simon Glass 
+S: Maintained
+F: tools/patman/
+
 PC87360 HARDWARE MONITORING DRIVER
 M: Jim Cromie 
 L: lm-sens...@lm-sensors.org
diff --git a/tools/patman/README b/tools/patman/README
new file mode 100644
index 000..27ec90a
--- /dev/null
+++ b/tools/patman/README
@@ -0,0 +1,475 @@
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+What is this?
+=
+
+This tool is a Python script which:
+- Creates patch directly from your branch
+- Cleans them up by removing unwanted tags
+- Inserts a cover letter with change lists
+- Runs the patches through checkpatch.pl and its own checks
+- Optionally emails them out to selected people
+
+It is intended to automate patch creation and make it a less
+error-prone process. It is useful for U-Boot and Linux work so far,
+since it uses the checkpatch.pl script.
+
+It is configured almost entirely by tags it finds in your commits.
+This means that you can work on a number of different branches at
+once, and keep the settings with each branch rather than having to
+git format-patch, git send-email, etc. with the correct parameters
+each time. So for example if you put:
+
+Series-to: fred.bl...@napier.co.nz
+
+in one of your commits, the series will be sent there.
+
+In Linux and U-Boot this will also call get_maintainer.pl on each of your
+patches automatically (unless you use -m to disable this).
+
+
+How to use this tool
+
+
+This tool requires a certain way of working:
+
+- Maintain a number of branches, one for each patch series you are
+working on
+- Add tags into the commits within each branch to indicate where the
+series should be sent, cover letter, version, etc. Most of these are
+normally in the top commit so it is easy to change them with 'git
+commit --amend'
+- Each branch tracks the upstream branch, so that this script can
+automatically determine the number of commits in it (optional)
+- Check out a branch, and run this script to create and send out your
+patches. Weeks later, change the patches and repeat, knowing that you
+will get a consistent result each time.
+
+
+How to configure it
+===
+
+For most cases of using patman for U-Boot development, patman can use the
+file 'doc/git-mailrc' in your U-Boot directory to supply the email aliases
+you need. To make this work, tell git where to find the file by typing
+this once:
+
+git config sendemail.aliasesfile doc/git-mailrc
+
+For both Linux and U-Boot the 'scripts/get_maintainer.pl' handles figuring
+out where to send patches pretty well.
+
+During the first run patman creates a config file for you by taking the default
+user name and email address from the global .gitconfig file.
+
+To add your own, create a file ~/.patman like this:
+
+>>>>
+# patman alias file
+
+[alias]
+me: Simon Glass 
+
+u-boot: U-Boot Mailing List 
+wolfgang: Wolfgang Denk 
+others: Mike Frysinger , Fred Bloggs 
+
+<<<<
+
+Aliases are recursive.
+
+The checkpatch.pl in the U-Boot tools/ subdirectory will be located and
+used. Failing that you can put it into your path or ~/bin/checkpatch.pl
+
+
+If you want to change the defaults for patman's command-line arguments,
+you can add a [settings] section to your .patman file.  This can be used
+for any command line option by referring to the "dest" for the option in
+patman.py.  For reference, the useful ones (at the moment) shown below
+(all with the non-default setting):
+
+>>>
+
+[settings]
+ignore_errors: True
+process_tags: False
+verbose: True
+
+<<<
+
+
+If you want to adjust settings (or aliases) that affect just a single
+project you can add a section that looks like [project_settings] or
+[project_alias].  If you want to use tags for your linux work, you could
+do:
+
+>>>
+
+[linux_settings]
+process_tags: True
+
+<<<
+
+
+How to run it
+=
+
+First do a dry run:
+
+$ ./tools/patman/patman -n
+
+If it can't detect the upstream branch, try telling it how many patches
+there are in your series:
+
+$ ./tools/patman/patman -n -c5
+
+This will create patch files in your current directory and tell you who
+it is thinking of sending them to. Take a look at the patch files.
+
+$ ./tools/patman/patman -n -c5 -s1
+
+Similar to the above, but skip the first commit and take the next 5. This
+is use

[PATCH 1/3] Add patman patch automation script

[Simon Glass]

This tool is a Python script which:
- Creates patch directly from your branch
- Cleans them up by removing unwanted tags
- Inserts a cover letter with change lists
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people

Add the main part of the code, excluding tests and documentation.

Signed-off-by: Simon Glass 
---

 tools/patman/.gitignore |   1 +
 tools/patman/checkpatch.py  | 173 
 tools/patman/command.py | 123 +
 tools/patman/commit.py  |  88 ++
 tools/patman/cros_subprocess.py | 397 +++
 tools/patman/get_maintainer.py  |  47 
 tools/patman/gitutil.py | 582 
 tools/patman/patchstream.py | 488 +
 tools/patman/patman |   1 +
 tools/patman/patman.py  | 167 
 tools/patman/project.py |  27 ++
 tools/patman/series.py  | 271 +++
 tools/patman/settings.py| 295 
 tools/patman/terminal.py| 158 +++
 14 files changed, 2818 insertions(+)
 create mode 100644 tools/patman/.gitignore
 create mode 100644 tools/patman/checkpatch.py
 create mode 100644 tools/patman/command.py
 create mode 100644 tools/patman/commit.py
 create mode 100644 tools/patman/cros_subprocess.py
 create mode 100644 tools/patman/get_maintainer.py
 create mode 100644 tools/patman/gitutil.py
 create mode 100644 tools/patman/patchstream.py
 create mode 12 tools/patman/patman
 create mode 100755 tools/patman/patman.py
 create mode 100644 tools/patman/project.py
 create mode 100644 tools/patman/series.py
 create mode 100644 tools/patman/settings.py
 create mode 100644 tools/patman/terminal.py

diff --git a/tools/patman/.gitignore b/tools/patman/.gitignore
new file mode 100644
index 000..0d20b64
--- /dev/null
+++ b/tools/patman/.gitignore
@@ -0,0 +1 @@
+*.pyc
diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
new file mode 100644
index 000..34a3bd2
--- /dev/null
+++ b/tools/patman/checkpatch.py
@@ -0,0 +1,173 @@
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+import collections
+import command
+import gitutil
+import os
+import re
+import sys
+import terminal
+
+def FindCheckPatch():
+top_level = gitutil.GetTopLevel()
+try_list = [
+os.getcwd(),
+os.path.join(os.getcwd(), '..', '..'),
+os.path.join(top_level, 'tools'),
+os.path.join(top_level, 'scripts'),
+'%s/bin' % os.getenv('HOME'),
+]
+# Look in current dir
+for path in try_list:
+fname = os.path.join(path, 'checkpatch.pl')
+if os.path.isfile(fname):
+return fname
+
+# Look upwwards for a Chrome OS tree
+while not os.path.ismount(path):
+fname = os.path.join(path, 'src', 'third_party', 'kernel', 'files',
+'scripts', 'checkpatch.pl')
+if os.path.isfile(fname):
+return fname
+path = os.path.dirname(path)
+
+sys.exit('Cannot find checkpatch.pl - please put it in your ' +
+ '~/bin directory or use --no-check')
+
+def CheckPatch(fname, verbose=False):
+"""Run checkpatch.pl on a file.
+
+Returns:
+namedtuple containing:
+ok: False=failure, True=ok
+problems: List of problems, each a dict:
+'type'; error or warning
+'msg': text message
+'file' : filename
+'line': line number
+errors: Number of errors
+warnings: Number of warnings
+checks: Number of checks
+lines: Number of lines
+stdout: Full output of checkpatch
+"""
+fields = ['ok', 'problems', 'errors', 'warnings', 'checks', 'lines',
+  'stdout']
+result = collections.namedtuple('CheckPatchResult', fields)
+result.ok = False
+result.errors, result.warning, result.checks = 0, 0, 0
+result.lines = 0
+result.problems = []
+chk = FindCheckPatch()
+item = {}
+result.stdout = command.Output(chk, '--no-tree', fname)
+#pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+#stdout, stderr = pipe.communicate()
+
+# total: 0 errors, 0 warnings, 159 lines checked
+# or:
+# total: 0 errors, 2 warnings, 7 checks, 473 lines checked
+re_stats = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)')
+re_stats_full = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)'
+   ' checks, (\d+)')
+re_ok = re.compile('.*has no obvious style problems')
+re_bad = re.compile('.*has style problems, please review')
+re_error = re.compile('ERROR: (.*)')
+re_warning = re.compile('WARNING: (.*)')
+re_check = re.compile('CHECK: (.*)')
+re_file = re.compile('#\d+: FILE: ([

[PATCH 2/3] Add tests for patman

[Simon Glass]

Add some simple tests which can be run with the '--test' option.

Signed-off-by: Simon Glass 
---

 tools/patman/test.py | 243 +++
 1 file changed, 243 insertions(+)
 create mode 100644 tools/patman/test.py

diff --git a/tools/patman/test.py b/tools/patman/test.py
new file mode 100644
index 000..e8f7472
--- /dev/null
+++ b/tools/patman/test.py
@@ -0,0 +1,243 @@
+#
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+import os
+import tempfile
+import unittest
+
+import checkpatch
+import gitutil
+import patchstream
+import series
+
+
+class TestPatch(unittest.TestCase):
+"""Test this program
+
+TODO: Write tests for the rest of the functionality
+"""
+
+def testBasic(self):
+"""Test basic filter operation"""
+data='''
+
+From 656c9a8c31fa65859d924cd21da920d6ba537fad Mon Sep 17 00:00:00 2001
+From: Simon Glass 
+Date: Thu, 28 Apr 2011 09:58:51 -0700
+Subject: [PATCH (resend) 3/7] Tegra2: Add more clock support
+
+This adds functions to enable/disable clocks and reset to on-chip peripherals.
+
+BUG=chromium-os:13875
+TEST=build U-Boot for Seaboard, boot
+
+Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413
+
+Review URL: http://codereview.chromium.org/696
+
+Signed-off-by: Simon Glass 
+---
+ arch/arm/cpu/armv7/tegra2/Makefile |2 +-
+ arch/arm/cpu/armv7/tegra2/ap20.c   |   57 ++
+ arch/arm/cpu/armv7/tegra2/clock.c  |  163 +
+'''
+expected='''
+
+From 656c9a8c31fa65859d924cd21da920d6ba537fad Mon Sep 17 00:00:00 2001
+From: Simon Glass 
+Date: Thu, 28 Apr 2011 09:58:51 -0700
+Subject: [PATCH (resend) 3/7] Tegra2: Add more clock support
+
+This adds functions to enable/disable clocks and reset to on-chip peripherals.
+
+Signed-off-by: Simon Glass 
+---
+
+ arch/arm/cpu/armv7/tegra2/Makefile |2 +-
+ arch/arm/cpu/armv7/tegra2/ap20.c   |   57 ++
+ arch/arm/cpu/armv7/tegra2/clock.c  |  163 +
+'''
+out = ''
+inhandle, inname = tempfile.mkstemp()
+infd = os.fdopen(inhandle, 'w')
+infd.write(data)
+infd.close()
+
+exphandle, expname = tempfile.mkstemp()
+expfd = os.fdopen(exphandle, 'w')
+expfd.write(expected)
+expfd.close()
+
+patchstream.FixPatch(None, inname, series.Series(), None)
+rc = os.system('diff -u %s %s' % (inname, expname))
+self.assertEqual(rc, 0)
+
+os.remove(inname)
+os.remove(expname)
+
+def GetData(self, data_type):
+data='''
+From 4924887af52713cabea78420eff03badea8f0035 Mon Sep 17 00:00:00 2001
+From: Simon Glass 
+Date: Thu, 7 Apr 2011 10:14:41 -0700
+Subject: [PATCH 1/4] Add microsecond boot time measurement
+
+This defines the basics of a new boot time measurement feature. This allows
+logging of very accurate time measurements as the boot proceeds, by using
+an available microsecond counter.
+
+%s
+---
+ README  |   11 
+ common/bootstage.c  |   50 
+ include/bootstage.h |   71 +++
+ include/common.h|8 ++
+ 5 files changed, 141 insertions(+), 0 deletions(-)
+ create mode 100644 common/bootstage.c
+ create mode 100644 include/bootstage.h
+
+diff --git a/README b/README
+index 6f3748d..f9e4e65 100644
+--- a/README
 b/README
+@@ -2026,6 +2026,17 @@ The following options need to be configured:
+   example, some LED's) on your board. At the moment,
+   the following checkpoints are implemented:
+
++- Time boot progress
++  CONFIG_BOOTSTAGE
++
++  Define this option to enable microsecond boot stage timing
++  on supported platforms. For this to work your platform
++  needs to define a function timer_get_us() which returns the
++  number of microseconds since reset. This would normally
++  be done in your SOC or board timer.c file.
++
++  You can add calls to bootstage_mark() to set time markers.
++
+ - Standalone program support:
+   CONFIG_STANDALONE_LOAD_ADDR
+
+diff --git a/common/bootstage.c b/common/bootstage.c
+new file mode 100644
+index 000..2234c87
+--- /dev/null
 b/common/bootstage.c
+@@ -0,0 +1,39 @@
++/*
++ * Copyright (c) 2011, Google Inc. All rights reserved.
++ *
++ * SPDX-License-Identifier:   GPL-2.0+
++ */
++
++
++/*
++ * This module records the progress of boot and arbitrary commands, and
++ * permits accurate timestamping of each. The records can optionally be
++ * passed to kernel in the ATAGs
++ */
++
++#include 
++
++
++struct bootstage_record {
++  uint32_t time_us;
++  const char *name;
++};
++
++static struct bootstage_record record[BOOTSTAGE_COUNT];
++
++uint32_t bootstage_mark(enum bootstage_id id, cons

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 14:43, Richard Weinberger rich...@nod.at wrote:
 Am 03.05.2015 um 22:40 schrieb Simon Glass:
 But I don't think it makes much sense to carry it with the Linux kernel 
 tree.
 Other projects can also use it and it does not seem to be very Linux kernel
 specific.
 git, quilt and other great tools also have their own repositories.

 My reasoning is that:

 - more will find it / use it if it is in-tree
 - it avoids installation and old-version problems (e.g. I suppose this
 is why the device tree compiler is built-in)
 - it is somewhat Linux-specific (e.g. uses get_maintainers,
 checkpatch.pl) and can break if checkpatch.pl if the wrong version
 (e.g. you check out and send patches from an older tree)
 - it could be built into the Linux workflow [1] and might thereby
 reduce the amount of confusion and errors (did you run checkpatch?,
 your change log is in the wrong place, you forgot to add your
 sign-off, etc.)

 If we'd follow these arguments we'd have to move the whole GNU into the
 kernel tree. ;-)

Well maybe the first two.

 checkpatch.pl and get_maintainers.pl are not really a show-stopper.
 Other projects are using them too. You can make them also configurable.
 i.e. check_script and get_maintaner_script.

Understood, I'm just explaining my reasoning for sending this patch.
With U-Boot it has been very convenient to be able to rely on this
being available in the tree. 3000 lines is a drop in the ocean with
Linux's 22m lines.

But I fully understand your point of view. If nothing else, at least
this series provides an easy way for people to try it out.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 14:13, Richard Weinberger rich...@nod.at wrote:
 Simon,

 Am 03.05.2015 um 21:54 schrieb Simon Glass:
 Hi Richard,

 On 3 May 2015 at 13:16, Richard Weinberger richard.weinber...@gmail.com 
 wrote:
 On Sun, May 3, 2015 at 8:29 PM, Simon Glass s...@chromium.org wrote:
 This tool is a Python script which:
 - Creates patch directly from your branch
 - Cleans them up by removing unwanted tags
 - Inserts a cover letter with change lists
 - Runs the patches through checkpatch.pl and its own checks
 - Optionally emails them out to selected people

 Don't get me wrong but is this really worth 3000+ lines of python?
 The tasks you describe can be done using a few lines bash.

 #!/bin/bash
 patman $@

 I obviously failed in my attempt to briefly explain what it does.
 Please check out the cover letter [1], README [2], or perhaps use it
 on a series. With respect to the length, it could be slimmed down a
 bit if that is important.

 the README file did the trick. ;)
 Sounds like a useful tool to manage patch series.

 But I don't think it makes much sense to carry it with the Linux kernel tree.
 Other projects can also use it and it does not seem to be very Linux kernel
 specific.
 git, quilt and other great tools also have their own repositories.

My reasoning is that:

- more will find it / use it if it is in-tree
- it avoids installation and old-version problems (e.g. I suppose this
is why the device tree compiler is built-in)
- it is somewhat Linux-specific (e.g. uses get_maintainers,
checkpatch.pl) and can break if checkpatch.pl if the wrong version
(e.g. you check out and send patches from an older tree)
- it could be built into the Linux workflow [1] and might thereby
reduce the amount of confusion and errors (did you run checkpatch?,
your change log is in the wrong place, you forgot to add your
sign-off, etc.)

That said, I could see this having a repo of its own, with the Linux
version a downstream copy, a bit like dtc (and maybe Kbuild/Kconfig -
I don't know). The feature set is probably mature enough to support
that now.

Regards,
Simon

[1] https://www.kernel.org/doc/Documentation/SubmittingPatches section 14
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/3] Add tests for patman

[Simon Glass]

Add some simple tests which can be run with the '--test' option.

Signed-off-by: Simon Glass s...@chromium.org
---

 tools/patman/test.py | 243 +++
 1 file changed, 243 insertions(+)
 create mode 100644 tools/patman/test.py

diff --git a/tools/patman/test.py b/tools/patman/test.py
new file mode 100644
index 000..e8f7472
--- /dev/null
+++ b/tools/patman/test.py
@@ -0,0 +1,243 @@
+#
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+import os
+import tempfile
+import unittest
+
+import checkpatch
+import gitutil
+import patchstream
+import series
+
+
+class TestPatch(unittest.TestCase):
+Test this program
+
+TODO: Write tests for the rest of the functionality
+
+
+def testBasic(self):
+Test basic filter operation
+data='''
+
+From 656c9a8c31fa65859d924cd21da920d6ba537fad Mon Sep 17 00:00:00 2001
+From: Simon Glass s...@chromium.org
+Date: Thu, 28 Apr 2011 09:58:51 -0700
+Subject: [PATCH (resend) 3/7] Tegra2: Add more clock support
+
+This adds functions to enable/disable clocks and reset to on-chip peripherals.
+
+BUG=chromium-os:13875
+TEST=build U-Boot for Seaboard, boot
+
+Change-Id: I80fe1d0c0b7dd10aa58ce5bb1d9290b6664d5413
+
+Review URL: http://codereview.chromium.org/696
+
+Signed-off-by: Simon Glass s...@chromium.org
+---
+ arch/arm/cpu/armv7/tegra2/Makefile |2 +-
+ arch/arm/cpu/armv7/tegra2/ap20.c   |   57 ++
+ arch/arm/cpu/armv7/tegra2/clock.c  |  163 +
+'''
+expected='''
+
+From 656c9a8c31fa65859d924cd21da920d6ba537fad Mon Sep 17 00:00:00 2001
+From: Simon Glass s...@chromium.org
+Date: Thu, 28 Apr 2011 09:58:51 -0700
+Subject: [PATCH (resend) 3/7] Tegra2: Add more clock support
+
+This adds functions to enable/disable clocks and reset to on-chip peripherals.
+
+Signed-off-by: Simon Glass s...@chromium.org
+---
+
+ arch/arm/cpu/armv7/tegra2/Makefile |2 +-
+ arch/arm/cpu/armv7/tegra2/ap20.c   |   57 ++
+ arch/arm/cpu/armv7/tegra2/clock.c  |  163 +
+'''
+out = ''
+inhandle, inname = tempfile.mkstemp()
+infd = os.fdopen(inhandle, 'w')
+infd.write(data)
+infd.close()
+
+exphandle, expname = tempfile.mkstemp()
+expfd = os.fdopen(exphandle, 'w')
+expfd.write(expected)
+expfd.close()
+
+patchstream.FixPatch(None, inname, series.Series(), None)
+rc = os.system('diff -u %s %s' % (inname, expname))
+self.assertEqual(rc, 0)
+
+os.remove(inname)
+os.remove(expname)
+
+def GetData(self, data_type):
+data='''
+From 4924887af52713cabea78420eff03badea8f0035 Mon Sep 17 00:00:00 2001
+From: Simon Glass s...@chromium.org
+Date: Thu, 7 Apr 2011 10:14:41 -0700
+Subject: [PATCH 1/4] Add microsecond boot time measurement
+
+This defines the basics of a new boot time measurement feature. This allows
+logging of very accurate time measurements as the boot proceeds, by using
+an available microsecond counter.
+
+%s
+---
+ README  |   11 
+ common/bootstage.c  |   50 
+ include/bootstage.h |   71 +++
+ include/common.h|8 ++
+ 5 files changed, 141 insertions(+), 0 deletions(-)
+ create mode 100644 common/bootstage.c
+ create mode 100644 include/bootstage.h
+
+diff --git a/README b/README
+index 6f3748d..f9e4e65 100644
+--- a/README
 b/README
+@@ -2026,6 +2026,17 @@ The following options need to be configured:
+   example, some LED's) on your board. At the moment,
+   the following checkpoints are implemented:
+
++- Time boot progress
++  CONFIG_BOOTSTAGE
++
++  Define this option to enable microsecond boot stage timing
++  on supported platforms. For this to work your platform
++  needs to define a function timer_get_us() which returns the
++  number of microseconds since reset. This would normally
++  be done in your SOC or board timer.c file.
++
++  You can add calls to bootstage_mark() to set time markers.
++
+ - Standalone program support:
+   CONFIG_STANDALONE_LOAD_ADDR
+
+diff --git a/common/bootstage.c b/common/bootstage.c
+new file mode 100644
+index 000..2234c87
+--- /dev/null
 b/common/bootstage.c
+@@ -0,0 +1,39 @@
++/*
++ * Copyright (c) 2011, Google Inc. All rights reserved.
++ *
++ * SPDX-License-Identifier:   GPL-2.0+
++ */
++
++
++/*
++ * This module records the progress of boot and arbitrary commands, and
++ * permits accurate timestamping of each. The records can optionally be
++ * passed to kernel in the ATAGs
++ */
++
++#include common.h
++
++
++struct bootstage_record {
++  uint32_t time_us;
++  const char *name;
++};
++
++static struct bootstage_record record[BOOTSTAGE_COUNT];
++
++uint32_t

[PATCH 0/3] Add a tool to simplify patch checking and posting (patman)

[Simon Glass]

Preparing, checking and sending patches to a mailing list is a tedious and
error-prone process. Dealing with multiple series, each which its own
revision history, CC list, cover letter requires concentation. Mistakes
are easy to make.

This tool aims to help with this. A single command generates the patch
series, runs it through checkpatch, generates the cover letter, adds a
change list to each patch and the cover letter, determines who should
receive the patch and Ccs people on the patch either using maintainer
information or tags in the commit subject. A dry-run mode allows output
to be checked.

This tool makes patch series repeatable, since everything needed to create
and send patches in stored in the git branch containing that series.
When making a change to one commit you can update the change log in that
commit and know that everything will turn out OK.

Specifically this tool is a Python script which (from the README):
- Creates patch directly from your branch
- Cleans them up by removing unwanted tags
- Inserts a cover letter with change lists
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people

It is intended to automate patch creation and make it a less
error-prone process. It is useful for U-Boot and Linux work so far,
since it uses the checkpatch.pl script.

It is configured almost entirely by tags it finds in your commits.
This means that you can work on a number of different branches at
once, and keep the settings with each branch rather than having to
git format-patch, git send-email, etc. with the correct parameters
each time. So for example if you put:

Series-to: fred.bl...@napier.co.nz

in one of your commits, the series will be sent there.

In Linux and U-Boot this will also call get_maintainer.pl on each of your
patches automatically (unless you use -m to disable this).

I am submitting this to LKML to raise awareness, since those who are not
involved in U-Boot probably don't know about it. I could not find a
specific linux-tools list but may have missed something.


Simon Glass (3):
  Add patman patch automation script
  Add tests for patman
  Add documentation for patman

 MAINTAINERS |   5 +
 tools/patman/.gitignore |   1 +
 tools/patman/README | 475 
 tools/patman/checkpatch.py  | 173 
 tools/patman/command.py | 123 +
 tools/patman/commit.py  |  88 ++
 tools/patman/cros_subprocess.py | 397 +++
 tools/patman/get_maintainer.py  |  47 
 tools/patman/gitutil.py | 582 
 tools/patman/patchstream.py | 488 +
 tools/patman/patman |   1 +
 tools/patman/patman.py  | 167 
 tools/patman/project.py |  27 ++
 tools/patman/series.py  | 271 +++
 tools/patman/settings.py| 295 
 tools/patman/terminal.py| 158 +++
 tools/patman/test.py| 243 +
 17 files changed, 3541 insertions(+)
 create mode 100644 tools/patman/.gitignore
 create mode 100644 tools/patman/README
 create mode 100644 tools/patman/checkpatch.py
 create mode 100644 tools/patman/command.py
 create mode 100644 tools/patman/commit.py
 create mode 100644 tools/patman/cros_subprocess.py
 create mode 100644 tools/patman/get_maintainer.py
 create mode 100644 tools/patman/gitutil.py
 create mode 100644 tools/patman/patchstream.py
 create mode 12 tools/patman/patman
 create mode 100755 tools/patman/patman.py
 create mode 100644 tools/patman/project.py
 create mode 100644 tools/patman/series.py
 create mode 100644 tools/patman/settings.py
 create mode 100644 tools/patman/terminal.py
 create mode 100644 tools/patman/test.py

-- 
2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] Add patman patch automation script

[Simon Glass]

Hi Richard,

On 3 May 2015 at 13:16, Richard Weinberger richard.weinber...@gmail.com wrote:
 On Sun, May 3, 2015 at 8:29 PM, Simon Glass s...@chromium.org wrote:
 This tool is a Python script which:
 - Creates patch directly from your branch
 - Cleans them up by removing unwanted tags
 - Inserts a cover letter with change lists
 - Runs the patches through checkpatch.pl and its own checks
 - Optionally emails them out to selected people

 Don't get me wrong but is this really worth 3000+ lines of python?
 The tasks you describe can be done using a few lines bash.

#!/bin/bash
patman $@

I obviously failed in my attempt to briefly explain what it does.
Please check out the cover letter [1], README [2], or perhaps use it
on a series. With respect to the length, it could be slimmed down a
bit if that is important.

Regards,
Simon

[1] https://lkml.org/lkml/2015/5/3/105
[2] https://lkml.org/lkml/2015/5/3/95
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3] Add documentation for patman

[Simon Glass]

Add an entry to the MAINTAINERS file, plus a README which explains how to
use patman.

Signed-off-by: Simon Glass s...@chromium.org
---

 MAINTAINERS |   5 +
 tools/patman/README | 475 
 2 files changed, 480 insertions(+)
 create mode 100644 tools/patman/README

diff --git a/MAINTAINERS b/MAINTAINERS
index 1622775..cd415d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7404,6 +7404,11 @@ F:   drivers/video/fbdev/sti*
 F: drivers/video/console/sti*
 F: drivers/video/logo/logo_parisc*
 
+PATMAN PATCH AUTOMATION TOOL
+M: Simon Glass s...@chromium.org
+S: Maintained
+F: tools/patman/
+
 PC87360 HARDWARE MONITORING DRIVER
 M: Jim Cromie jim.cro...@gmail.com
 L: lm-sens...@lm-sensors.org
diff --git a/tools/patman/README b/tools/patman/README
new file mode 100644
index 000..27ec90a
--- /dev/null
+++ b/tools/patman/README
@@ -0,0 +1,475 @@
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+What is this?
+=
+
+This tool is a Python script which:
+- Creates patch directly from your branch
+- Cleans them up by removing unwanted tags
+- Inserts a cover letter with change lists
+- Runs the patches through checkpatch.pl and its own checks
+- Optionally emails them out to selected people
+
+It is intended to automate patch creation and make it a less
+error-prone process. It is useful for U-Boot and Linux work so far,
+since it uses the checkpatch.pl script.
+
+It is configured almost entirely by tags it finds in your commits.
+This means that you can work on a number of different branches at
+once, and keep the settings with each branch rather than having to
+git format-patch, git send-email, etc. with the correct parameters
+each time. So for example if you put:
+
+Series-to: fred.bl...@napier.co.nz
+
+in one of your commits, the series will be sent there.
+
+In Linux and U-Boot this will also call get_maintainer.pl on each of your
+patches automatically (unless you use -m to disable this).
+
+
+How to use this tool
+
+
+This tool requires a certain way of working:
+
+- Maintain a number of branches, one for each patch series you are
+working on
+- Add tags into the commits within each branch to indicate where the
+series should be sent, cover letter, version, etc. Most of these are
+normally in the top commit so it is easy to change them with 'git
+commit --amend'
+- Each branch tracks the upstream branch, so that this script can
+automatically determine the number of commits in it (optional)
+- Check out a branch, and run this script to create and send out your
+patches. Weeks later, change the patches and repeat, knowing that you
+will get a consistent result each time.
+
+
+How to configure it
+===
+
+For most cases of using patman for U-Boot development, patman can use the
+file 'doc/git-mailrc' in your U-Boot directory to supply the email aliases
+you need. To make this work, tell git where to find the file by typing
+this once:
+
+git config sendemail.aliasesfile doc/git-mailrc
+
+For both Linux and U-Boot the 'scripts/get_maintainer.pl' handles figuring
+out where to send patches pretty well.
+
+During the first run patman creates a config file for you by taking the default
+user name and email address from the global .gitconfig file.
+
+To add your own, create a file ~/.patman like this:
+
+
+# patman alias file
+
+[alias]
+me: Simon Glass s...@chromium.org
+
+u-boot: U-Boot Mailing List u-b...@lists.denx.de
+wolfgang: Wolfgang Denk w...@denx.de
+others: Mike Frysinger vap...@gentoo.org, Fred Bloggs f.blo...@napier.net
+
+
+
+Aliases are recursive.
+
+The checkpatch.pl in the U-Boot tools/ subdirectory will be located and
+used. Failing that you can put it into your path or ~/bin/checkpatch.pl
+
+
+If you want to change the defaults for patman's command-line arguments,
+you can add a [settings] section to your .patman file.  This can be used
+for any command line option by referring to the dest for the option in
+patman.py.  For reference, the useful ones (at the moment) shown below
+(all with the non-default setting):
+
+
+
+[settings]
+ignore_errors: True
+process_tags: False
+verbose: True
+
+
+
+
+If you want to adjust settings (or aliases) that affect just a single
+project you can add a section that looks like [project_settings] or
+[project_alias].  If you want to use tags for your linux work, you could
+do:
+
+
+
+[linux_settings]
+process_tags: True
+
+
+
+
+How to run it
+=
+
+First do a dry run:
+
+$ ./tools/patman/patman -n
+
+If it can't detect the upstream branch, try telling it how many patches
+there are in your series:
+
+$ ./tools/patman/patman -n -c5
+
+This will create patch files in your current directory and tell you who
+it is thinking of sending them to. Take a look at the patch files.
+
+$ ./tools/patman/patman -n -c5 -s1
+
+Similar to the above, but skip the first commit and take

[PATCH 1/3] Add patman patch automation script

[Simon Glass]

This tool is a Python script which:
- Creates patch directly from your branch
- Cleans them up by removing unwanted tags
- Inserts a cover letter with change lists
- Runs the patches through checkpatch.pl and its own checks
- Optionally emails them out to selected people

Add the main part of the code, excluding tests and documentation.

Signed-off-by: Simon Glass s...@chromium.org
---

 tools/patman/.gitignore |   1 +
 tools/patman/checkpatch.py  | 173 
 tools/patman/command.py | 123 +
 tools/patman/commit.py  |  88 ++
 tools/patman/cros_subprocess.py | 397 +++
 tools/patman/get_maintainer.py  |  47 
 tools/patman/gitutil.py | 582 
 tools/patman/patchstream.py | 488 +
 tools/patman/patman |   1 +
 tools/patman/patman.py  | 167 
 tools/patman/project.py |  27 ++
 tools/patman/series.py  | 271 +++
 tools/patman/settings.py| 295 
 tools/patman/terminal.py| 158 +++
 14 files changed, 2818 insertions(+)
 create mode 100644 tools/patman/.gitignore
 create mode 100644 tools/patman/checkpatch.py
 create mode 100644 tools/patman/command.py
 create mode 100644 tools/patman/commit.py
 create mode 100644 tools/patman/cros_subprocess.py
 create mode 100644 tools/patman/get_maintainer.py
 create mode 100644 tools/patman/gitutil.py
 create mode 100644 tools/patman/patchstream.py
 create mode 12 tools/patman/patman
 create mode 100755 tools/patman/patman.py
 create mode 100644 tools/patman/project.py
 create mode 100644 tools/patman/series.py
 create mode 100644 tools/patman/settings.py
 create mode 100644 tools/patman/terminal.py

diff --git a/tools/patman/.gitignore b/tools/patman/.gitignore
new file mode 100644
index 000..0d20b64
--- /dev/null
+++ b/tools/patman/.gitignore
@@ -0,0 +1 @@
+*.pyc
diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
new file mode 100644
index 000..34a3bd2
--- /dev/null
+++ b/tools/patman/checkpatch.py
@@ -0,0 +1,173 @@
+# Copyright (c) 2011 The Chromium OS Authors.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+import collections
+import command
+import gitutil
+import os
+import re
+import sys
+import terminal
+
+def FindCheckPatch():
+top_level = gitutil.GetTopLevel()
+try_list = [
+os.getcwd(),
+os.path.join(os.getcwd(), '..', '..'),
+os.path.join(top_level, 'tools'),
+os.path.join(top_level, 'scripts'),
+'%s/bin' % os.getenv('HOME'),
+]
+# Look in current dir
+for path in try_list:
+fname = os.path.join(path, 'checkpatch.pl')
+if os.path.isfile(fname):
+return fname
+
+# Look upwwards for a Chrome OS tree
+while not os.path.ismount(path):
+fname = os.path.join(path, 'src', 'third_party', 'kernel', 'files',
+'scripts', 'checkpatch.pl')
+if os.path.isfile(fname):
+return fname
+path = os.path.dirname(path)
+
+sys.exit('Cannot find checkpatch.pl - please put it in your ' +
+ '~/bin directory or use --no-check')
+
+def CheckPatch(fname, verbose=False):
+Run checkpatch.pl on a file.
+
+Returns:
+namedtuple containing:
+ok: False=failure, True=ok
+problems: List of problems, each a dict:
+'type'; error or warning
+'msg': text message
+'file' : filename
+'line': line number
+errors: Number of errors
+warnings: Number of warnings
+checks: Number of checks
+lines: Number of lines
+stdout: Full output of checkpatch
+
+fields = ['ok', 'problems', 'errors', 'warnings', 'checks', 'lines',
+  'stdout']
+result = collections.namedtuple('CheckPatchResult', fields)
+result.ok = False
+result.errors, result.warning, result.checks = 0, 0, 0
+result.lines = 0
+result.problems = []
+chk = FindCheckPatch()
+item = {}
+result.stdout = command.Output(chk, '--no-tree', fname)
+#pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+#stdout, stderr = pipe.communicate()
+
+# total: 0 errors, 0 warnings, 159 lines checked
+# or:
+# total: 0 errors, 2 warnings, 7 checks, 473 lines checked
+re_stats = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)')
+re_stats_full = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)'
+   ' checks, (\d+)')
+re_ok = re.compile('.*has no obvious style problems')
+re_bad = re.compile('.*has style problems, please review')
+re_error = re.compile('ERROR: (.*)')
+re_warning = re.compile('WARNING: (.*)')
+re_check = re.compile('CHECK: (.*)')
+re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):')
+
+for line

Re: [PATCH v5 09/11] drm/tegra: Reset the SOR on probe

[Simon Glass]

Hi,

On 2 March 2015 at 01:41, Alexandre Courbot  wrote:
>
> On Thu, Feb 12, 2015 at 5:51 PM, Tomeu Vizoso
>  wrote:
> > As there isn't a way for the firmware on the Nyan chromebooks to hand
> > over the display to the kernel.
>
> Could this have a side-effect on models for which the firmware *does*
> hand over the display to the kernel? E.g. temporary glitch or black
> screen?
>
> This is probably ok though, as such a handing over would need to be
> documented in the firmware/kernel command line, and could thus be
> caught to disable that code block if needed.

Is there a general way in which this hand-over is done, e.g. with a
device tree binding?

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 09/11] drm/tegra: Reset the SOR on probe

[Simon Glass]

Hi,

On 2 March 2015 at 01:41, Alexandre Courbot gnu...@gmail.com wrote:

 On Thu, Feb 12, 2015 at 5:51 PM, Tomeu Vizoso
 tomeu.viz...@collabora.com wrote:
  As there isn't a way for the firmware on the Nyan chromebooks to hand
  over the display to the kernel.

 Could this have a side-effect on models for which the firmware *does*
 hand over the display to the kernel? E.g. temporary glitch or black
 screen?

 This is probably ok though, as such a handing over would need to be
 documented in the firmware/kernel command line, and could thus be
 caught to disable that code block if needed.

Is there a general way in which this hand-over is done, e.g. with a
device tree binding?

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

[Simon Glass]

Hi Olof,

On 25 February 2015 at 17:59, Olof Johansson  wrote:
> On Tue, Feb 17, 2015 at 07:26:50PM -0700, Simon Glass wrote:
>> Hi,
>>
>> On 16 February 2015 at 01:19, Javier Martinez Canillas
>>  wrote:
>> > Hello Olof,
>> >
>> > On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
>> >> Hello,
>> >>
>> >> The mainline ChromeOS Embedded Controller (EC) driver is still missing 
>> >> some
>> >> features that are present in the downstream ChromiumOS tree. These are:
>> >>
>> >>   - Low Pin Count (LPC) interface
>> >>   - User-space device interface
>> >>   - Access to vboot context stored on a block device
>> >>   - Access to vboot context stored on EC's nvram
>> >>   - Power Delivery Device
>> >>   - Support for multiple EC in a system
>> >>
>> >> This is a fifth version of a series that adds support for the first two of
>> >> the missing features: the EC LPC and EC character device interfaces that
>> >> are used by user-space to access the ChromeOS EC. The support patches were
>> >> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
>> >> squashed to have a minimal patch-set.
>> >>
>> >
>> > Any comments on this series? The last version was posted a couple of weeks
>> > ago but the series have been in the list for months. Lee has already acked
>> > the mfd changes so you can merge all through your chrome-platform tree if
>> > you want.
>> >
>> > It wold be great if this series get in to have the EC user-space interface
>> > supported and to minimize the delta with the Chromemium OS kernel since it
>> > still has other features that needs to be upstreamed like multiple EC in a
>> > system and access to vboot context stored in block device or EC's nvram.
>>
>> Are you sure Olof is the right maintainer for this going to mainline?
>
> Yes.
>
>> I do feel for you trying to get all this in and have seen your many
>> attempts. It has been in U-Boot for 18 months...I hope you get there
>> in the end.
>
> Cool, u-boot has userspace interfaces now? I didn't know that.

Not the 'User-space device interface', the other stuff :-) Anyway it
will be good to see this series in.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

[Simon Glass]

Hi Olof,

On 25 February 2015 at 17:59, Olof Johansson o...@lixom.net wrote:
 On Tue, Feb 17, 2015 at 07:26:50PM -0700, Simon Glass wrote:
 Hi,

 On 16 February 2015 at 01:19, Javier Martinez Canillas
 javier.marti...@collabora.co.uk wrote:
  Hello Olof,
 
  On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
  Hello,
 
  The mainline ChromeOS Embedded Controller (EC) driver is still missing 
  some
  features that are present in the downstream ChromiumOS tree. These are:
 
- Low Pin Count (LPC) interface
- User-space device interface
- Access to vboot context stored on a block device
- Access to vboot context stored on EC's nvram
- Power Delivery Device
- Support for multiple EC in a system
 
  This is a fifth version of a series that adds support for the first two of
  the missing features: the EC LPC and EC character device interfaces that
  are used by user-space to access the ChromeOS EC. The support patches were
  taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
  squashed to have a minimal patch-set.
 
 
  Any comments on this series? The last version was posted a couple of weeks
  ago but the series have been in the list for months. Lee has already acked
  the mfd changes so you can merge all through your chrome-platform tree if
  you want.
 
  It wold be great if this series get in to have the EC user-space interface
  supported and to minimize the delta with the Chromemium OS kernel since it
  still has other features that needs to be upstreamed like multiple EC in a
  system and access to vboot context stored in block device or EC's nvram.

 Are you sure Olof is the right maintainer for this going to mainline?

 Yes.

 I do feel for you trying to get all this in and have seen your many
 attempts. It has been in U-Boot for 18 months...I hope you get there
 in the end.

 Cool, u-boot has userspace interfaces now? I didn't know that.

Not the 'User-space device interface', the other stuff :-) Anyway it
will be good to see this series in.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

[Simon Glass]

Hi,

On 16 February 2015 at 01:19, Javier Martinez Canillas
 wrote:
> Hello Olof,
>
> On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
>> Hello,
>>
>> The mainline ChromeOS Embedded Controller (EC) driver is still missing some
>> features that are present in the downstream ChromiumOS tree. These are:
>>
>>   - Low Pin Count (LPC) interface
>>   - User-space device interface
>>   - Access to vboot context stored on a block device
>>   - Access to vboot context stored on EC's nvram
>>   - Power Delivery Device
>>   - Support for multiple EC in a system
>>
>> This is a fifth version of a series that adds support for the first two of
>> the missing features: the EC LPC and EC character device interfaces that
>> are used by user-space to access the ChromeOS EC. The support patches were
>> taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
>> squashed to have a minimal patch-set.
>>
>
> Any comments on this series? The last version was posted a couple of weeks
> ago but the series have been in the list for months. Lee has already acked
> the mfd changes so you can merge all through your chrome-platform tree if
> you want.
>
> It wold be great if this series get in to have the EC user-space interface
> supported and to minimize the delta with the Chromemium OS kernel since it
> still has other features that needs to be upstreamed like multiple EC in a
> system and access to vboot context stored in block device or EC's nvram.

Are you sure Olof is the right maintainer for this going to mainline?

I do feel for you trying to get all this in and have seen your many
attempts. It has been in U-Boot for 18 months...I hope you get there
in the end.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v5 0/7] platform/chrome: Add user-space dev inferface support

[Simon Glass]

Hi,

On 16 February 2015 at 01:19, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
 Hello Olof,

 On 02/02/2015 12:26 PM, Javier Martinez Canillas wrote:
 Hello,

 The mainline ChromeOS Embedded Controller (EC) driver is still missing some
 features that are present in the downstream ChromiumOS tree. These are:

   - Low Pin Count (LPC) interface
   - User-space device interface
   - Access to vboot context stored on a block device
   - Access to vboot context stored on EC's nvram
   - Power Delivery Device
   - Support for multiple EC in a system

 This is a fifth version of a series that adds support for the first two of
 the missing features: the EC LPC and EC character device interfaces that
 are used by user-space to access the ChromeOS EC. The support patches were
 taken from the downstream ChromiumOS 3.14 tree with the fixes and cleanups
 squashed to have a minimal patch-set.


 Any comments on this series? The last version was posted a couple of weeks
 ago but the series have been in the list for months. Lee has already acked
 the mfd changes so you can merge all through your chrome-platform tree if
 you want.

 It wold be great if this series get in to have the EC user-space interface
 supported and to minimize the delta with the Chromemium OS kernel since it
 still has other features that needs to be upstreamed like multiple EC in a
 system and access to vboot context stored in block device or EC's nvram.

Are you sure Olof is the right maintainer for this going to mainline?

I do feel for you trying to get all this in and have seen your many
attempts. It has been in U-Boot for 18 months...I hope you get there
in the end.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result

[Simon Glass]

On 27 June 2014 12:56, Doug Anderson  wrote:
> We know how many bytes the EC should be sending us (which is also the
> number of bytes transferred) and also how many bytes the EC actually
> wanted to send to us.  When computing the checksum and copying back
> data let's make sure we take the lesser of the two of those.  We'll
> also complain if the EC tried to send us too many bytes.  The EC
> sending us too few bytes is legit for when we send the EC an invalid
> command.
>
> This is based on similar code in cros_ec_spi.
>
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result

[Simon Glass]

On 27 June 2014 12:56, Doug Anderson diand...@chromium.org wrote:
 We know how many bytes the EC should be sending us (which is also the
 number of bytes transferred) and also how many bytes the EC actually
 wanted to send to us.  When computing the checksum and copying back
 data let's make sure we take the lesser of the two of those.  We'll
 also complain if the EC tried to send us too many bytes.  The EC
 sending us too few bytes is legit for when we send the EC an invalid
 command.

 This is based on similar code in cros_ec_spi.

 Signed-off-by: Doug Anderson diand...@chromium.org

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

[Simon Glass]

On 23 June 2014 15:20, Doug Anderson  wrote:
> In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
> flag in the cros_ec_tunnel was useless.  It went into a 16-bit flags
> field but was defined at (1 << 16).
>
> Since we have no 10-bit i2c devices on the other side of the tunnel on
> any known devices this was never a problem.  Until we do it makes
> sense to remove this code.  On the EC side the code to handle this
> flag was removed in <https://chromium-review.googlesource.com/204162>.
>
> Reported-by: Dave Jones 
> Signed-off-by: Doug Anderson 

Funny. We certainly don't use it.

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

[Simon Glass]

On 23 June 2014 15:20, Doug Anderson diand...@chromium.org wrote:
 In https://lkml.org/lkml/2014/6/10/265 pointed out that the 10-bit
 flag in the cros_ec_tunnel was useless.  It went into a 16-bit flags
 field but was defined at (1  16).

 Since we have no 10-bit i2c devices on the other side of the tunnel on
 any known devices this was never a problem.  Until we do it makes
 sense to remove this code.  On the EC side the code to handle this
 flag was removed in https://chromium-review.googlesource.com/204162.

 Reported-by: Dave Jones da...@redhat.com
 Signed-off-by: Doug Anderson diand...@chromium.org

Funny. We certainly don't use it.

Reviewed-by: Simon Glass s...@chromium.org

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson  wrote:
> From: Andrew Bresticker 
>
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
>
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
>
> Signed-off-by: Andrew Bresticker 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 

We never needed an EC-level interrupt, and have shipped at least three
products now that use this code, so I think it is safe enough to
declare that we won't need it.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
>
> This change lets the EC report its errors separately.
>
> [dianders: Added common function to cros_ec.c]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

This is better.

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 05/10] mfd: cros_ec: Use struct cros_ec_command to communicate with the EC

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson  wrote:
> From: Bill Richardson 
>
> This is some internal structure reorganization / renaming to prepare
> for future patches that will add a userspace API to cros_ec.  There
> should be no visible changes.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> Acked-by: Lee Jones 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 05/10] mfd: cros_ec: Use struct cros_ec_command to communicate with the EC

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 This is some internal structure reorganization / renaming to prepare
 for future patches that will add a userspace API to cros_ec.  There
 should be no visible changes.

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 Acked-by: Lee Jones lee.jo...@linaro.org

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 Just because the host was able to talk to the EC doesn't mean that the EC
 was happy with what it was told. Errors in communincation are not the same
 as error messages from the EC itself.

 This change lets the EC report its errors separately.

 [dianders: Added common function to cros_ec.c]

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org

This is better.

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson diand...@chromium.org wrote:
 From: Andrew Bresticker abres...@chromium.org

 If we receive EC interrupts after the cros_ec driver has probed, but
 before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
 will not run the cros_ec_keyb notifier and the EC will leave the IRQ
 line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
 the resulting flood of interrupts causes the machine to hang.

 Since the EC interrupt is currently only used for the keyboard, move
 the setup and handling of the EC interrupt to the cros_ec_keyb driver.

 Signed-off-by: Andrew Bresticker abres...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org

Reviewed-by: Simon Glass s...@chromium.org

We never needed an EC-level interrupt, and have shipped at least three
products now that use this code, so I think it is safe enough to
declare that we won't need it.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 10/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi Doug,

On 17 June 2014 21:54, Doug Anderson  wrote:
> Simon,
>
> On Tue, Jun 17, 2014 at 8:46 PM, Simon Glass  wrote:
>> Hi,
>>
>> On 16 June 2014 14:40, Doug Anderson  wrote:
>>> From: Bill Richardson 
>>>
>>> When communicating with the EC, the cmd_xfer() function should return the
>>> number of bytes it received from the EC, or negative on error.
>>
>> This is just for the I2C tunnel feature, right? If so, I think this
>> should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
>> cmd_xfer().
>
> No, the tunnel feature is implemented just fine without this (and is
> already landed and working).  It looks like the (not yet upstreamed)
> ec_i2c_limited_xfer for spring returns this new value directly but I'm
> not convinced that's technicall correct.
>
> Bill can correct me if I'm wrong, but I think this is primarily
> interesting once we add in cros_ec_dev (the userspace API) which needs
> this info.  Until that happens this patch doesn't hurt and just
> returns some extra info.

Agreed.

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 17 June 2014 21:27, Doug Anderson  wrote:
> Simon,
>
> On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass  wrote:
>>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
>>> b/drivers/input/keyboard/cros_ec_keyb.c
>>> index 4083796..dc37b6b 100644
>>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>>> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>>>
>>>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
>>> *kb_state)
>>>  {
>>> -   return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>>> - kb_state, ckdev->cols);
>>> +   int ret;
>>> +   struct cros_ec_command msg = {
>>> +   .version = 0,
>>> +   .command = EC_CMD_MKBP_STATE,
>>> +   .outdata = NULL,
>>> +   .outsize = 0,
>>> +   .indata = kb_state,
>>> +   .insize = ckdev->cols,
>>> +   };
>>> +
>>> +   ret = ckdev->ec->cmd_xfer(ckdev->ec, );
>>
>> Do we need ret?
>
> No.  If you wish I will spin without it.  Let me know.
>
> Note that locally this code includes a comment between the above and the 
> return:
>   /* FIXME: This assumes msg.result == EC_RES_SUCCESS */

It's not important to me, and you've explained the other question.

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 17 June 2014 21:22, Doug Anderson  wrote:
>
> Simon,
>
> On Tue, Jun 17, 2014 at 8:39 PM, Simon Glass  wrote:
> > Hi Doug,
> >
> > On 16 June 2014 14:39, Doug Anderson  wrote:
> >> From: Bill Richardson 
> >>
> >> struct cros_ec_device has a superfluous "name" field. We can get all the
> >> debugging info we need from the existing ec_name and phys_name fields, so
> >> let's take out the extra field.
> >
> > Except that it no longer prints I2C/SPI - i.e. the transport that is
> > used. Is that not considered important?
>
> Before this change:
>   [1.895472] cros-ec-spi spi2.0: Chrome EC (SPI)
>
> After this change:
>   [1.910671] cros-ec-spi spi2.0: Chrome EC device registered
>
>
> I think having SPI in the name twice is probably enough.  ;)

Ah that helps! Could have been in the commit message.

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 10/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi,

On 16 June 2014 14:40, Doug Anderson  wrote:
> From: Bill Richardson 
>
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.

This is just for the I2C tunnel feature, right? If so, I think this
should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
cmd_xfer().

>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
>  drivers/mfd/cros_ec_i2c.c   | 2 +-
>  drivers/mfd/cros_ec_spi.c   | 2 +-
>  include/linux/mfd/cros_ec.h | 8 
>  4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
> b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index dd07818..05e033c 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> msg.insize = response_len;
>
> result = bus->ec->cmd_xfer(bus->ec, );
> -   if (result)
> +   if (result < 0)
> goto exit;
>
> result = ec_i2c_parse_response(response, i2c_msgs, );
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 2276096..dc0ba29 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -120,7 +120,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
> *ec_dev,
> goto done;
> }
>
> -   ret = 0;
> +   ret = i2c_msg[1].buf[1];
>   done:
> kfree(in_buf);
> kfree(out_buf);
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 4d34f1c..beba1bc 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -333,7 +333,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
> *ec_dev,
> goto exit;
> }
>
> -   ret = 0;
> +   ret = len;
>  exit:
> mutex_unlock(_spi->lock);
> return ret;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 60c0880..7b65a75 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -41,7 +41,7 @@ enum {
>   * @outdata: Outgoing data to EC
>   * @outsize: Outgoing length in bytes
>   * @indata: Where to put the incoming data from EC
> - * @insize: Incoming length in bytes (filled in by EC)
> + * @insize: Max number of bytes to accept from EC
>   * @result: EC's response to the command (separate from communication 
> failure)
>   */
>  struct cros_ec_command {
> @@ -64,9 +64,9 @@ struct cros_ec_command {
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
> - * Returns 0 if the communication succeeded, but that doesn't mean the EC
> - * was happy with the command it got. Caller should check msg.result for
> - * the EC's result code.
> + * Returns the number of bytes received if the communication succeeded, 
> but
> + * that doesn't mean the EC was happy with the command. The caller
> + * should check msg.result for the EC's result code.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use
> --
> 2.0.0.526.g5318336
>

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 09/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
>
> This change lets the EC report its errors separately.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec_i2c.c | 15 +++
>  drivers/mfd/cros_ec_spi.c | 26 ++
>  2 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 5bb32f5..2276096 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -92,11 +92,18 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
> *ec_dev,
> }
>
> /* check response error code */
> -   if (i2c_msg[1].buf[0]) {
> -   dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -msg->command, i2c_msg[1].buf[0]);
> -   ret = -EINVAL;
> +   msg->result = i2c_msg[1].buf[0];
> +   switch (msg->result) {
> +   case EC_RES_SUCCESS:
> +   break;
> +   case EC_RES_IN_PROGRESS:
> +   ret = -EAGAIN;
> +   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> +   msg->command);
> goto done;
> +   default:
> +   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
> +msg->command, msg->result);
> }
>
> /* copy response packet payload and compute checksum */
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 09ca789..4d34f1c 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -289,21 +289,23 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
> *ec_dev,
> goto exit;
> }
>
> -   /* check response error code */
> ptr = ec_dev->din;
> -   if (ptr[0]) {
> -   if (ptr[0] == EC_RES_IN_PROGRESS) {
> -   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> -   ec_msg->command);
> -   ret = -EAGAIN;
> -   goto exit;
> -   }
> -   dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -ec_msg->command, ptr[0]);
> -   debug_packet(ec_dev->dev, "in_err", ptr, len);
> -   ret = -EINVAL;
> +
> +   /* check response error code */
> +   ec_msg->result = ptr[0];
> +   switch (ec_msg->result) {
> +   case EC_RES_SUCCESS:
> +   break;
> +   case EC_RES_IN_PROGRESS:
> +   ret = -EAGAIN;
> +   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> +   ec_msg->command);
> goto exit;
> +   default:
> +   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
> +ec_msg->command, ec_msg->result);
> }

Since this code is the same as the above, should it go in a common
function in cros_ec?

> +
> len = ptr[1];
> sum = ptr[0] + ptr[1];
> if (len > ec_msg->insize) {
> --
> 2.0.0.526.g5318336
>

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
>
> This is internal code refactoring only. Nothing should change.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++
>  drivers/input/keyboard/cros_ec_keyb.c   | 14 --
>  drivers/mfd/cros_ec.c   | 32 
>  include/linux/mfd/cros_ec.h | 19 ++-
>  4 files changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
> b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 8e7a714..dd07818 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> u8 *request = NULL;
> u8 *response = NULL;
> int result;
> +   struct cros_ec_command msg;
>
> request_len = ec_i2c_count_message(i2c_msgs, num);
> if (request_len < 0) {
> @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> }
>
> ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> -   result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> -  request, request_len,
> -  response, response_len);
> +
> +   msg.version = 0;
> +   msg.command = EC_CMD_I2C_PASSTHRU;
> +   msg.outdata = request;
> +   msg.outsize = request_len;
> +   msg.indata = response;
> +   msg.insize = response_len;
> +
> +   result = bus->ec->cmd_xfer(bus->ec, );
> if (result)
> goto exit;
>
> @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
> u32 remote_bus;
> int err;
>
> -   if (!ec->command_sendrecv) {
> +   if (!ec->cmd_xfer) {
> dev_err(dev, "Missing sendrecv\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
> b/drivers/input/keyboard/cros_ec_keyb.c
> index 4083796..dc37b6b 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
> *kb_state)
>  {
> -   return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> - kb_state, ckdev->cols);
> +   int ret;
> +   struct cros_ec_command msg = {
> +   .version = 0,
> +   .command = EC_CMD_MKBP_STATE,
> +   .outdata = NULL,
> +   .outsize = 0,
> +   .indata = kb_state,
> +   .insize = ckdev->cols,
> +   };
> +
> +   ret = ckdev->ec->cmd_xfer(ckdev->ec, );

Do we need ret?

> +   return ret;
>  }
>
>  static int cros_ec_keyb_work(struct notifier_block *nb,
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index d242714..6dd91e9 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_prepare_tx);
>
> -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *out_buf, int out_len,
> -   void *in_buf, int in_len)
> -{
> -   struct cros_ec_command msg;
> -
> -   msg.version = cmd >> 8;
> -   msg.command = cmd & 0xff;
> -   msg.outdata = out_buf;
> -   msg.outsize = out_len;
> -   msg.indata = in_buf;
> -   msg.insize = in_len;
> -
> -   return ec_dev->cmd_xfer(ec_dev, );
> -}
> -
> -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *buf, int buf_len)
> -{
> -   return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
> -}
> -
> -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *buf, int buf_len)
> -{
> -   return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
> -}
> -
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
> struct cros_ec_device *ec_dev = data;
> @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>
> BLOCKING_INIT_NOTIFIER_HEAD(_dev->event_notifier);
>
> -   ec_dev->command_send = cros_ec_command_send;
> -   ec_dev->command_recv = cros_ec_command_recv;
> -   ec_dev->command_sendrecv = cros_ec_command_sendrecv;
> -
> if (ec_dev->din_size) {
> ec_dev->din = devm_kzalloc(dev, 

Re: [PATCH 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> struct cros_ec_device has a superfluous "name" field. We can get all the
> debugging info we need from the existing ec_name and phys_name fields, so
> let's take out the extra field.

Except that it no longer prints I2C/SPI - i.e. the transport that is
used. Is that not considered important?

Anyway:

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> The members of struct cros_ec_device were improperly commented, and
> intermixed the private and public sections. This is just cleanup to make it
> more obvious what goes with what.
>
> [dianders: left lock in the structure but gave it the name that will
> eventually be used.]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec.c   |  2 +-
>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>  drivers/mfd/cros_ec_spi.c   | 10 +++
>  include/linux/mfd/cros_ec.h | 65 
> -
>  4 files changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index bd6f936..a9eede5 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
> *ec_dev,
> msg.in_buf = in_buf;
> msg.in_len = in_len;
>
> -   return ec_dev->command_xfer(ec_dev, );
> +   return ec_dev->cmd_xfer(ec_dev, );

Why do this rename? It makes it different from the other members.

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
>
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 02/10] mfd: cros_ec: IRQs for cros_ec should be optional

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Preparing the way for the LPC device, which is just a plaform_device without
> interrupts.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 02/10] mfd: cros_ec: IRQs for cros_ec should be optional

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 Preparing the way for the LPC device, which is just a plaform_device without
 interrupts.

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 The lower-level driver may want to provide its own buffers. If so,
 there's no need to allocate new ones.  This already happens to work
 just fine (since we check for size of 0 and use devm allocation), but
 it's good to document it.

 [dianders: Resolved conflicts; documented that no code changes needed
 on mainline]

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 The members of struct cros_ec_device were improperly commented, and
 intermixed the private and public sections. This is just cleanup to make it
 more obvious what goes with what.

 [dianders: left lock in the structure but gave it the name that will
 eventually be used.]

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/mfd/cros_ec.c   |  2 +-
  drivers/mfd/cros_ec_i2c.c   |  4 +--
  drivers/mfd/cros_ec_spi.c   | 10 +++
  include/linux/mfd/cros_ec.h | 65 
 -
  4 files changed, 43 insertions(+), 38 deletions(-)

 diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
 index bd6f936..a9eede5 100644
 --- a/drivers/mfd/cros_ec.c
 +++ b/drivers/mfd/cros_ec.c
 @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
 *ec_dev,
 msg.in_buf = in_buf;
 msg.in_len = in_len;

 -   return ec_dev-command_xfer(ec_dev, msg);
 +   return ec_dev-cmd_xfer(ec_dev, msg);

Why do this rename? It makes it different from the other members.

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 struct cros_ec_device has a superfluous name field. We can get all the
 debugging info we need from the existing ec_name and phys_name fields, so
 let's take out the extra field.

Except that it no longer prints I2C/SPI - i.e. the transport that is
used. Is that not considered important?

Anyway:

Reviewed-by: Simon Glass s...@chromium.org

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 Remove the three wrapper functions that talk to the EC without passing all
 the desired arguments and just use the underlying communication function
 that passes everything in a struct intead.

 This is internal code refactoring only. Nothing should change.

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++
  drivers/input/keyboard/cros_ec_keyb.c   | 14 --
  drivers/mfd/cros_ec.c   | 32 
  include/linux/mfd/cros_ec.h | 19 ++-
  4 files changed, 29 insertions(+), 51 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
 b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 index 8e7a714..dd07818 100644
 --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
 i2c_msg i2c_msgs[],
 u8 *request = NULL;
 u8 *response = NULL;
 int result;
 +   struct cros_ec_command msg;

 request_len = ec_i2c_count_message(i2c_msgs, num);
 if (request_len  0) {
 @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
 i2c_msg i2c_msgs[],
 }

 ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
 -   result = bus-ec-command_sendrecv(bus-ec, EC_CMD_I2C_PASSTHRU,
 -  request, request_len,
 -  response, response_len);
 +
 +   msg.version = 0;
 +   msg.command = EC_CMD_I2C_PASSTHRU;
 +   msg.outdata = request;
 +   msg.outsize = request_len;
 +   msg.indata = response;
 +   msg.insize = response_len;
 +
 +   result = bus-ec-cmd_xfer(bus-ec, msg);
 if (result)
 goto exit;

 @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
 u32 remote_bus;
 int err;

 -   if (!ec-command_sendrecv) {
 +   if (!ec-cmd_xfer) {
 dev_err(dev, Missing sendrecv\n);
 return -EINVAL;
 }
 diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
 b/drivers/input/keyboard/cros_ec_keyb.c
 index 4083796..dc37b6b 100644
 --- a/drivers/input/keyboard/cros_ec_keyb.c
 +++ b/drivers/input/keyboard/cros_ec_keyb.c
 @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)

  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
 *kb_state)
  {
 -   return ckdev-ec-command_recv(ckdev-ec, EC_CMD_MKBP_STATE,
 - kb_state, ckdev-cols);
 +   int ret;
 +   struct cros_ec_command msg = {
 +   .version = 0,
 +   .command = EC_CMD_MKBP_STATE,
 +   .outdata = NULL,
 +   .outsize = 0,
 +   .indata = kb_state,
 +   .insize = ckdev-cols,
 +   };
 +
 +   ret = ckdev-ec-cmd_xfer(ckdev-ec, msg);

Do we need ret?

 +   return ret;
  }

  static int cros_ec_keyb_work(struct notifier_block *nb,
 diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
 index d242714..6dd91e9 100644
 --- a/drivers/mfd/cros_ec.c
 +++ b/drivers/mfd/cros_ec.c
 @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
  }
  EXPORT_SYMBOL(cros_ec_prepare_tx);

 -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
 -   uint16_t cmd, void *out_buf, int out_len,
 -   void *in_buf, int in_len)
 -{
 -   struct cros_ec_command msg;
 -
 -   msg.version = cmd  8;
 -   msg.command = cmd  0xff;
 -   msg.outdata = out_buf;
 -   msg.outsize = out_len;
 -   msg.indata = in_buf;
 -   msg.insize = in_len;
 -
 -   return ec_dev-cmd_xfer(ec_dev, msg);
 -}
 -
 -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
 -   uint16_t cmd, void *buf, int buf_len)
 -{
 -   return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
 -}
 -
 -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
 -   uint16_t cmd, void *buf, int buf_len)
 -{
 -   return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
 -}
 -
  static irqreturn_t ec_irq_thread(int irq, void *data)
  {
 struct cros_ec_device *ec_dev = data;
 @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)

 BLOCKING_INIT_NOTIFIER_HEAD(ec_dev-event_notifier);

 -   ec_dev-command_send = cros_ec_command_send;
 -   ec_dev-command_recv = cros_ec_command_recv;
 -   ec_dev-command_sendrecv = cros_ec_command_sendrecv;
 -
 if (ec_dev-din_size) {
 ec_dev-din = devm_kzalloc(dev, ec_dev-din_size, GFP_KERNEL);
 if (!ec_dev-din)
 diff --git 

Re: [PATCH 09/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 Just because the host was able to talk to the EC doesn't mean that the EC
 was happy with what it was told. Errors in communincation are not the same
 as error messages from the EC itself.

 This change lets the EC report its errors separately.

 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/mfd/cros_ec_i2c.c | 15 +++
  drivers/mfd/cros_ec_spi.c | 26 ++
  2 files changed, 25 insertions(+), 16 deletions(-)

 diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
 index 5bb32f5..2276096 100644
 --- a/drivers/mfd/cros_ec_i2c.c
 +++ b/drivers/mfd/cros_ec_i2c.c
 @@ -92,11 +92,18 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
 *ec_dev,
 }

 /* check response error code */
 -   if (i2c_msg[1].buf[0]) {
 -   dev_warn(ec_dev-dev, command 0x%02x returned an error %d\n,
 -msg-command, i2c_msg[1].buf[0]);
 -   ret = -EINVAL;
 +   msg-result = i2c_msg[1].buf[0];
 +   switch (msg-result) {
 +   case EC_RES_SUCCESS:
 +   break;
 +   case EC_RES_IN_PROGRESS:
 +   ret = -EAGAIN;
 +   dev_dbg(ec_dev-dev, command 0x%02x in progress\n,
 +   msg-command);
 goto done;
 +   default:
 +   dev_warn(ec_dev-dev, command 0x%02x returned %d\n,
 +msg-command, msg-result);
 }

 /* copy response packet payload and compute checksum */
 diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
 index 09ca789..4d34f1c 100644
 --- a/drivers/mfd/cros_ec_spi.c
 +++ b/drivers/mfd/cros_ec_spi.c
 @@ -289,21 +289,23 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
 *ec_dev,
 goto exit;
 }

 -   /* check response error code */
 ptr = ec_dev-din;
 -   if (ptr[0]) {
 -   if (ptr[0] == EC_RES_IN_PROGRESS) {
 -   dev_dbg(ec_dev-dev, command 0x%02x in progress\n,
 -   ec_msg-command);
 -   ret = -EAGAIN;
 -   goto exit;
 -   }
 -   dev_warn(ec_dev-dev, command 0x%02x returned an error %d\n,
 -ec_msg-command, ptr[0]);
 -   debug_packet(ec_dev-dev, in_err, ptr, len);
 -   ret = -EINVAL;
 +
 +   /* check response error code */
 +   ec_msg-result = ptr[0];
 +   switch (ec_msg-result) {
 +   case EC_RES_SUCCESS:
 +   break;
 +   case EC_RES_IN_PROGRESS:
 +   ret = -EAGAIN;
 +   dev_dbg(ec_dev-dev, command 0x%02x in progress\n,
 +   ec_msg-command);
 goto exit;
 +   default:
 +   dev_warn(ec_dev-dev, command 0x%02x returned %d\n,
 +ec_msg-command, ec_msg-result);
 }

Since this code is the same as the above, should it go in a common
function in cros_ec?

 +
 len = ptr[1];
 sum = ptr[0] + ptr[1];
 if (len  ec_msg-insize) {
 --
 2.0.0.526.g5318336


Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 10/10] mfd: cros_ec: ec_dev-cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi,

On 16 June 2014 14:40, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 When communicating with the EC, the cmd_xfer() function should return the
 number of bytes it received from the EC, or negative on error.

This is just for the I2C tunnel feature, right? If so, I think this
should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
cmd_xfer().


 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
  drivers/mfd/cros_ec_i2c.c   | 2 +-
  drivers/mfd/cros_ec_spi.c   | 2 +-
  include/linux/mfd/cros_ec.h | 8 
  4 files changed, 7 insertions(+), 7 deletions(-)

 diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
 b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 index dd07818..05e033c 100644
 --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 @@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
 i2c_msg i2c_msgs[],
 msg.insize = response_len;

 result = bus-ec-cmd_xfer(bus-ec, msg);
 -   if (result)
 +   if (result  0)
 goto exit;

 result = ec_i2c_parse_response(response, i2c_msgs, num);
 diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
 index 2276096..dc0ba29 100644
 --- a/drivers/mfd/cros_ec_i2c.c
 +++ b/drivers/mfd/cros_ec_i2c.c
 @@ -120,7 +120,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
 *ec_dev,
 goto done;
 }

 -   ret = 0;
 +   ret = i2c_msg[1].buf[1];
   done:
 kfree(in_buf);
 kfree(out_buf);
 diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
 index 4d34f1c..beba1bc 100644
 --- a/drivers/mfd/cros_ec_spi.c
 +++ b/drivers/mfd/cros_ec_spi.c
 @@ -333,7 +333,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
 *ec_dev,
 goto exit;
 }

 -   ret = 0;
 +   ret = len;
  exit:
 mutex_unlock(ec_spi-lock);
 return ret;
 diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
 index 60c0880..7b65a75 100644
 --- a/include/linux/mfd/cros_ec.h
 +++ b/include/linux/mfd/cros_ec.h
 @@ -41,7 +41,7 @@ enum {
   * @outdata: Outgoing data to EC
   * @outsize: Outgoing length in bytes
   * @indata: Where to put the incoming data from EC
 - * @insize: Incoming length in bytes (filled in by EC)
 + * @insize: Max number of bytes to accept from EC
   * @result: EC's response to the command (separate from communication 
 failure)
   */
  struct cros_ec_command {
 @@ -64,9 +64,9 @@ struct cros_ec_command {
   * sleep at the last suspend
   * @event_notifier: interrupt event notifier for transport devices
   * @cmd_xfer: send command to EC and get response
 - * Returns 0 if the communication succeeded, but that doesn't mean the EC
 - * was happy with the command it got. Caller should check msg.result for
 - * the EC's result code.
 + * Returns the number of bytes received if the communication succeeded, 
 but
 + * that doesn't mean the EC was happy with the command. The caller
 + * should check msg.result for the EC's result code.
   *
   * @priv: Private data
   * @irq: Interrupt to use
 --
 2.0.0.526.g5318336


Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 17 June 2014 21:22, Doug Anderson diand...@chromium.org wrote:

 Simon,

 On Tue, Jun 17, 2014 at 8:39 PM, Simon Glass s...@chromium.org wrote:
  Hi Doug,
 
  On 16 June 2014 14:39, Doug Anderson diand...@chromium.org wrote:
  From: Bill Richardson wfric...@chromium.org
 
  struct cros_ec_device has a superfluous name field. We can get all the
  debugging info we need from the existing ec_name and phys_name fields, so
  let's take out the extra field.
 
  Except that it no longer prints I2C/SPI - i.e. the transport that is
  used. Is that not considered important?

 Before this change:
   [1.895472] cros-ec-spi spi2.0: Chrome EC (SPI)

 After this change:
   [1.910671] cros-ec-spi spi2.0: Chrome EC device registered


 I think having SPI in the name twice is probably enough.  ;)

Ah that helps! Could have been in the commit message.

Reviewed-by: Simon Glass s...@chromium.org

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 17 June 2014 21:27, Doug Anderson diand...@chromium.org wrote:
 Simon,

 On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass s...@chromium.org wrote:
 diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
 b/drivers/input/keyboard/cros_ec_keyb.c
 index 4083796..dc37b6b 100644
 --- a/drivers/input/keyboard/cros_ec_keyb.c
 +++ b/drivers/input/keyboard/cros_ec_keyb.c
 @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)

  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
 *kb_state)
  {
 -   return ckdev-ec-command_recv(ckdev-ec, EC_CMD_MKBP_STATE,
 - kb_state, ckdev-cols);
 +   int ret;
 +   struct cros_ec_command msg = {
 +   .version = 0,
 +   .command = EC_CMD_MKBP_STATE,
 +   .outdata = NULL,
 +   .outsize = 0,
 +   .indata = kb_state,
 +   .insize = ckdev-cols,
 +   };
 +
 +   ret = ckdev-ec-cmd_xfer(ckdev-ec, msg);

 Do we need ret?

 No.  If you wish I will spin without it.  Let me know.

 Note that locally this code includes a comment between the above and the 
 return:
   /* FIXME: This assumes msg.result == EC_RES_SUCCESS */

It's not important to me, and you've explained the other question.

Reviewed-by: Simon Glass s...@chromium.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 10/10] mfd: cros_ec: ec_dev-cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi Doug,

On 17 June 2014 21:54, Doug Anderson diand...@chromium.org wrote:
 Simon,

 On Tue, Jun 17, 2014 at 8:46 PM, Simon Glass s...@chromium.org wrote:
 Hi,

 On 16 June 2014 14:40, Doug Anderson diand...@chromium.org wrote:
 From: Bill Richardson wfric...@chromium.org

 When communicating with the EC, the cmd_xfer() function should return the
 number of bytes it received from the EC, or negative on error.

 This is just for the I2C tunnel feature, right? If so, I think this
 should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
 cmd_xfer().

 No, the tunnel feature is implemented just fine without this (and is
 already landed and working).  It looks like the (not yet upstreamed)
 ec_i2c_limited_xfer for spring returns this new value directly but I'm
 not convinced that's technicall correct.

 Bill can correct me if I'm wrong, but I think this is primarily
 interesting once we add in cros_ec_dev (the userspace API) which needs
 this info.  Until that happens this patch doesn't hurt and just
 returns some extra info.

Agreed.

Reviewed-by: Simon Glass s...@chromium.org

Regards,
Simon
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Simon Glass]

Hi Doug,

On 18 April 2014 15:15, Doug Anderson  wrote:
>
> Simon,
>
> On Fri, Apr 18, 2014 at 10:28 AM, Simon Glass  wrote:
> > Hi Doug,
> >
> > On 17 April 2014 11:59, Doug Anderson  wrote:
> >> The main transfer function for cros_ec_spi can be called by more than
> >> one client at a time.  Make sure that those clients don't stomp on
> >> each other by locking the bus for the duration of the transfer
> >> function.
> >
> > Is there no lock at the cros_ec layer?
>
> Not with what's upstream.
>
> Locally in the Chromium OS tree:
>
> * You can see that Bill removed the dev_lock at
> <https://chromium-review.googlesource.com/#/c/57051/> since it wasn't
> used.
>
> * Andrew moved to a common locking scheme later in
> <https://chromium-review.googlesource.com/#/c/170747/> (thus adding
> roughly the same lock back and using it), but in order to do that
> we've got a dozen pathces in between, some of which are big
> reorganizations.  This includes at least:
>
> 6820b91 CHROMIUM: cros_ec: Tweak struct cros_ec_device for clarity
> 5e8e562 CHROMIUM: Use struct cros_ec_command to communicate with the EC
> 9e7db82 CHROMIUM: cleanup: remove unused fields from struct cros_ec_device
> 866e62d CHROMIUM: cleanup: Remove EC wrapper functions.
> 8a372b2 cros_ec: move locking into cros_ec_cmd_xfer
> 981c4aa cros_ec: stop calling ->cmd_xfer() directly
>
> I think we should take in some of those other changes but I'd rather
> get correctness in first (since people are wanting to use this driver
> in upstream kernel) and get cleanups posted after this lands.  I was
> also trying not to spam the list with a 20-patch series...

That explains it, thank you. I should have guessed that for myself.

Reviewed-by: Simon Glass 

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/7] mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms

[Simon Glass]

Hi Doug,

On 17 April 2014 11:59, Doug Anderson  wrote:
> We're adding i2c tunneling to the list of things that goes over
> cros_ec.  i2c tunneling can be slooow, so increase our deadline to
> 100ms to account for that.
>
> Signed-off-by: Doug Anderson 

I believe the EC protocol should be changed on future platforms to
poll for completion of I2C, but in the meantime this patch prevents
failure.

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/7] mfd: cros_ec: spi: calculate delay between transfers correctly

[Simon Glass]

On 17 April 2014 11:59, Doug Anderson  wrote:
> From: David Hendricks 
>
> To avoid spamming the EC we calculate the time between the previous
> transfer and the current transfer and force a delay if the time delta
> is too small.
>
> However, a small miscalculation causes the delay period to be
> far too short. Most noticably this impacts commands with a long
> turnaround time such as EC firmware reads and writes.
>
> Signed-off-by: David Hendricks 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Simon Glass]

On 17 April 2014 11:59, Doug Anderson  wrote:
> From: Bill Richardson 
>
> This just updates include/linux/mfd/cros_ec_commands.h to match the
> latest EC version (which is the One True Source for such things).  See
> <https://chromium.googlesource.com/chromiumos/platform/ec>
>
> [dianders: took today's ToT version from the Chromium OS EC; deleted
> references to cros_ec_dev and cros_ec_lpc since those aren't upstream
> yet]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Simon Glass]

Hi Doug,

On 17 April 2014 11:59, Doug Anderson  wrote:
> The main transfer function for cros_ec_spi can be called by more than
> one client at a time.  Make sure that those clients don't stomp on
> each other by locking the bus for the duration of the transfer
> function.

Is there no lock at the cros_ec layer?

Regards,
Simon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/7] mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable

[Simon Glass]

On 17 April 2014 11:59, Doug Anderson  wrote:
> The cros_ec_spi transfer had two problems with its timeout code:
>
> 1. It looked at the timeout even in the case that it found valid data.
> 2. If the cros_ec_spi code got switched out for a while, it's possible
>it could get a timeout after a single loop.  Let's be paranoid and
>make sure we do one last transfer after the timeout expires.
>
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/