All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jim Quinlan <jim2101024@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com,
	linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Gregory Fong <gregory.0xf0@gmail.com>,
	Kevin Cernekee <cernekee@gmail.com>,
	Brian Norris <computersforpeace@gmail.com>
Subject: Re: [PATCH 1/9] SOC: brcmstb: add memory API
Date: Tue, 17 Oct 2017 01:24:13 -0700	[thread overview]
Message-ID: <20171017082413.GA10574@infradead.org> (raw)
In-Reply-To: <1507761269-7017-2-git-send-email-jim2101024@gmail.com>

> +/* Macros to help extract property data */
> +#define U8TOU32(b, offs) \
> +	((((u32)b[0 + offs] << 0)  & 0x000000ff) | \
> +	 (((u32)b[1 + offs] << 8)  & 0x0000ff00) | \
> +	 (((u32)b[2 + offs] << 16) & 0x00ff0000) | \
> +	 (((u32)b[3 + offs] << 24) & 0xff000000))

Please us helpers like get_unaligned_le32 instead opencoding them.

> +#define DT_PROP_DATA_TO_U32(b, offs) (fdt32_to_cpu(U8TOU32(b, offs)))

And together with this it looks really whacky.  So you're converting
from le to native first and then do another conversion from be to cpu?

Something doesn't work out here.

> +/* -------------------- Functions -------------------- */

Please remove pointless comments like this one.

> +
> +/*
> + * If the DT nodes are handy, determine which MEMC holds the specified
> + * physical address.
> + */
> +#ifdef CONFIG_ARCH_BRCMSTB
> +int __brcmstb_memory_phys_addr_to_memc(phys_addr_t pa, void __iomem *base)

Please move this into the arm arch code.

> +#elif defined(CONFIG_MIPS)

And this into the mips arch code.

> +EXPORT_SYMBOL(brcmstb_memory_phys_addr_to_memc);

> +EXPORT_SYMBOL(brcmstb_memory_memc_size);

Why is this exported?

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Jim Quinlan <jim2101024@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	linux-mips@linux-mips.org,
	Florian Fainelli <f.fainelli@gmail.com>,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	Kevin Cernekee <cernekee@gmail.com>,
	Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org, Ralf Baechle <ralf@linux-mips.org>,
	Rob Herring <robh+dt@kernel.org>,
	bcm-kernel-feedback-list@broadcom.com,
	Gregory Fong <gregory.0xf0@gmail.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Brian Norris <computersforpeace@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/9] SOC: brcmstb: add memory API
Date: Tue, 17 Oct 2017 01:24:13 -0700	[thread overview]
Message-ID: <20171017082413.GA10574@infradead.org> (raw)
In-Reply-To: <1507761269-7017-2-git-send-email-jim2101024@gmail.com>

> +/* Macros to help extract property data */
> +#define U8TOU32(b, offs) \
> +	((((u32)b[0 + offs] << 0)  & 0x000000ff) | \
> +	 (((u32)b[1 + offs] << 8)  & 0x0000ff00) | \
> +	 (((u32)b[2 + offs] << 16) & 0x00ff0000) | \
> +	 (((u32)b[3 + offs] << 24) & 0xff000000))

Please us helpers like get_unaligned_le32 instead opencoding them.

> +#define DT_PROP_DATA_TO_U32(b, offs) (fdt32_to_cpu(U8TOU32(b, offs)))

And together with this it looks really whacky.  So you're converting
from le to native first and then do another conversion from be to cpu?

Something doesn't work out here.

> +/* -------------------- Functions -------------------- */

Please remove pointless comments like this one.

> +
> +/*
> + * If the DT nodes are handy, determine which MEMC holds the specified
> + * physical address.
> + */
> +#ifdef CONFIG_ARCH_BRCMSTB
> +int __brcmstb_memory_phys_addr_to_memc(phys_addr_t pa, void __iomem *base)

Please move this into the arm arch code.

> +#elif defined(CONFIG_MIPS)

And this into the mips arch code.

> +EXPORT_SYMBOL(brcmstb_memory_phys_addr_to_memc);

> +EXPORT_SYMBOL(brcmstb_memory_memc_size);

Why is this exported?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: hch@infradead.org (Christoph Hellwig)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/9] SOC: brcmstb: add memory API
Date: Tue, 17 Oct 2017 01:24:13 -0700	[thread overview]
Message-ID: <20171017082413.GA10574@infradead.org> (raw)
In-Reply-To: <1507761269-7017-2-git-send-email-jim2101024@gmail.com>

> +/* Macros to help extract property data */
> +#define U8TOU32(b, offs) \
> +	((((u32)b[0 + offs] << 0)  & 0x000000ff) | \
> +	 (((u32)b[1 + offs] << 8)  & 0x0000ff00) | \
> +	 (((u32)b[2 + offs] << 16) & 0x00ff0000) | \
> +	 (((u32)b[3 + offs] << 24) & 0xff000000))

Please us helpers like get_unaligned_le32 instead opencoding them.

> +#define DT_PROP_DATA_TO_U32(b, offs) (fdt32_to_cpu(U8TOU32(b, offs)))

And together with this it looks really whacky.  So you're converting
from le to native first and then do another conversion from be to cpu?

Something doesn't work out here.

> +/* -------------------- Functions -------------------- */

Please remove pointless comments like this one.

> +
> +/*
> + * If the DT nodes are handy, determine which MEMC holds the specified
> + * physical address.
> + */
> +#ifdef CONFIG_ARCH_BRCMSTB
> +int __brcmstb_memory_phys_addr_to_memc(phys_addr_t pa, void __iomem *base)

Please move this into the arm arch code.

> +#elif defined(CONFIG_MIPS)

And this into the mips arch code.

> +EXPORT_SYMBOL(brcmstb_memory_phys_addr_to_memc);

> +EXPORT_SYMBOL(brcmstb_memory_memc_size);

Why is this exported?

  parent reply	other threads:[~2017-10-17  8:24 UTC|newest]

Thread overview: 138+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-11 22:34 PCI: brcmstb: Add Broadcom Settopbox PCIe support Jim Quinlan
2017-10-11 22:34 ` Jim Quinlan
2017-10-11 22:34 ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 1/9] SOC: brcmstb: add memory API Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-12 14:41   ` Julien Thierry
2017-10-12 14:41     ` Julien Thierry
2017-10-12 14:41     ` Julien Thierry
2017-10-12 16:53     ` Florian Fainelli
2017-10-12 16:53       ` Florian Fainelli
2017-10-12 16:53       ` Florian Fainelli
2017-10-17  8:24   ` Christoph Hellwig [this message]
2017-10-17  8:24     ` Christoph Hellwig
2017-10-17  8:24     ` Christoph Hellwig
2017-10-17 16:12     ` Florian Fainelli
2017-10-17 16:12       ` Florian Fainelli
2017-10-17 16:12       ` Florian Fainelli
2017-10-18  6:46       ` Christoph Hellwig
2017-10-18  6:46         ` Christoph Hellwig
2017-10-18  6:46         ` Christoph Hellwig
2017-10-18 16:47         ` Florian Fainelli
2017-10-18 16:47           ` Florian Fainelli
2017-10-11 22:34 ` [PATCH 2/9] PCI: host: brcmstb: add DT docs for Brcmstb PCIe device Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-12  0:55   ` Brian Norris
2017-10-12  0:55     ` Brian Norris
2017-10-12  0:55     ` Brian Norris
2017-10-12  0:55     ` Brian Norris
2017-10-12 17:52     ` Jim Quinlan
2017-10-12 17:52       ` Jim Quinlan
2017-10-12 17:52       ` Jim Quinlan
2017-10-17 20:24   ` Rob Herring
2017-10-17 20:24     ` Rob Herring
2017-10-17 20:24     ` Rob Herring
2017-10-17 22:42     ` Jim Quinlan
2017-10-17 22:42       ` Jim Quinlan
2017-10-17 22:42       ` Jim Quinlan
2017-10-19 21:49       ` Rob Herring
2017-10-19 21:49         ` Rob Herring
2017-10-19 21:49         ` Rob Herring
2017-10-19 21:58         ` Florian Fainelli
2017-10-19 21:58           ` Florian Fainelli
2017-10-19 21:58           ` Florian Fainelli
2017-10-20 17:27           ` Brian Norris
2017-10-20 17:27             ` Brian Norris
2017-10-20 17:27             ` Brian Norris
2017-10-20 17:27             ` Brian Norris
2017-10-20 21:39             ` Rob Herring
2017-10-20 21:39               ` Rob Herring
2017-10-20 21:39               ` Rob Herring
2017-10-19 23:04         ` Jim Quinlan
2017-10-19 23:04           ` Jim Quinlan
2017-10-19 23:04           ` Jim Quinlan
2017-10-19 23:04           ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 3/9] PCI: host: brcmstb: Broadcom PCIe Host Controller Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 4/9] arm64: dma-mapping: export symbol arch_setup_dma_ops Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-12 17:06   ` Robin Murphy
2017-10-12 17:06     ` Robin Murphy
2017-10-12 17:06     ` Robin Murphy
2017-10-12 18:15     ` Jim Quinlan
2017-10-12 18:15       ` Jim Quinlan
2017-10-12 18:15       ` Jim Quinlan
2017-10-12 18:15       ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 5/9] PCI: host: brcmstb: add dma-ranges for inbound traffic Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
     [not found]   ` <1507761269-7017-6-git-send-email-jim2101024-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-12 18:04     ` Robin Murphy
2017-10-12 18:04       ` Robin Murphy
2017-10-12 18:04       ` Robin Murphy
2017-10-12 18:04       ` Robin Murphy
2017-10-12 21:43       ` Jim Quinlan
2017-10-12 21:43         ` Jim Quinlan
2017-10-12 21:43         ` Jim Quinlan
2017-10-17  8:14       ` Christoph Hellwig
2017-10-17  8:14         ` Christoph Hellwig
2017-10-17  8:14         ` Christoph Hellwig
2017-10-17 16:11         ` Jim Quinlan
2017-10-17 16:11           ` Jim Quinlan
2017-10-17 16:11           ` Jim Quinlan
     [not found]           ` <CANCKTBsCB+x2XgrND9AhRtxPkCXfps1nA+YymkZjKHOUZfjSHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-18  6:53             ` Christoph Hellwig
2017-10-18  6:53               ` Christoph Hellwig
2017-10-18  6:53               ` Christoph Hellwig
2017-10-18  6:53               ` Christoph Hellwig
     [not found]               ` <20171018065316.GA11183-jcswGhMUV9g@public.gmane.org>
2017-10-18 14:41                 ` Jim Quinlan
2017-10-18 14:41                   ` Jim Quinlan
2017-10-18 14:41                   ` Jim Quinlan
     [not found]                   ` <CANCKTBv+yiCNsrnx3m+W9wPqC4NdKPZ2p=zLtSa8fX6v1rPcYQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-19  9:16                     ` Christoph Hellwig
2017-10-19  9:16                       ` Christoph Hellwig
2017-10-19  9:16                       ` Christoph Hellwig
2017-10-19  9:16                       ` Christoph Hellwig
2017-10-19 22:47                       ` Jim Quinlan
2017-10-19 22:47                         ` Jim Quinlan
2017-10-19 22:47                         ` Jim Quinlan
     [not found]                         ` <CANCKTBuaTD29My77QfOeUmtZfLAmmJXUYe6QvBW+uoH2Kb+tAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-20  7:37                           ` Christoph Hellwig
2017-10-20  7:37                             ` Christoph Hellwig
2017-10-20  7:37                             ` Christoph Hellwig
2017-10-20  7:37                             ` Christoph Hellwig
     [not found]                             ` <20171020073730.GA12937-jcswGhMUV9g@public.gmane.org>
2017-10-20 14:41                               ` Jim Quinlan
2017-10-20 14:41                                 ` Jim Quinlan
2017-10-20 14:41                                 ` Jim Quinlan
2017-10-20 14:41                                 ` Jim Quinlan
2017-10-20 14:57                                 ` Christoph Hellwig
2017-10-20 14:57                                   ` Christoph Hellwig
2017-10-20 14:57                                   ` Christoph Hellwig
2017-10-20 14:57                                   ` Christoph Hellwig
2017-10-20 15:27                                   ` Jim Quinlan
2017-10-20 15:27                                     ` Jim Quinlan
2017-10-20 15:27                                     ` Jim Quinlan
2017-10-20 16:17                                     ` Christoph Hellwig
2017-10-20 16:17                                       ` Christoph Hellwig
2017-10-20 16:17                                       ` Christoph Hellwig
2017-10-20 16:17                                       ` Christoph Hellwig
     [not found]                                     ` <CANCKTBtxp9gSdndKAZ7xGA+VozQsn2PX_-9P8A22_5Matbb7-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-10-23  9:06                                       ` David Laight
2017-10-23  9:06                                         ` David Laight
2017-10-23  9:06                                         ` David Laight
2017-10-23  9:06                                         ` David Laight
     [not found]                                         ` <063D6719AE5E284EB5DD2968C1650D6DD009F05A-VkEWCZq2GCInGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2017-10-24 18:08                                           ` Jim Quinlan
2017-10-24 18:08                                             ` Jim Quinlan
2017-10-24 18:08                                             ` Jim Quinlan
2017-10-24 18:08                                             ` Jim Quinlan
2017-10-25  9:36                                             ` David Laight
2017-10-25  9:36                                               ` David Laight
2017-10-25  9:36                                               ` David Laight
2017-10-25  9:36                                               ` David Laight
2017-10-11 22:34 ` [PATCH 6/9] PCI/MSI: Enable PCI_MSI_IRQ_DOMAIN support for MIPS Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 7/9] PCI: host: brcmstb: add MSI capability Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 8/9] MIPS: BMIPS: add PCI bindings for 7425, 7435 Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan
2017-10-11 22:34 ` [PATCH 9/9] MIPS: BMIPS: enable PCI Jim Quinlan
2017-10-11 22:34   ` Jim Quinlan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171017082413.GA10574@infradead.org \
    --to=hch@infradead.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=cernekee@gmail.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=gregory.0xf0@gmail.com \
    --cc=jim2101024@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ralf@linux-mips.org \
    --cc=robh+dt@kernel.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.