* Re: [PATCH v4 3/17] video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
From: Ingo Molnar @ 2015-05-29 6:41 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: tomi.valkeinen, linux-kernel, linux-fbdev, Luis R. Rodriguez,
Thomas Gleixner, Juergen Gross, Jingoo Han, Daniel Vetter,
Masanari Iida, Suresh Siddha, Ingo Molnar, Andy Lutomirski,
Dave Airlie, Antonino Daplas, Jean-Christophe Plagniol-Villard
In-Reply-To: <1432859434-17821-3-git-send-email-mcgrof@do-not-panic.com>
* Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> + gbe_mem = devm_ioremap_wc(&p_dev->dev, gbe_mem_phys,
> + gbe_mem_size);
> + gbe_mem = dma_alloc_writecombine(NULL, gbe_mem_size,
> + &gbe_dma_addr, GFP_KERNEL);
> + par->wc_cookie = arch_phys_wc_add(gbe_mem_phys, gbe_mem_size);
> + arch_phys_wc_del(par->wc_cookie);
> + arch_phys_wc_del(par->wc_cookie);
Could we _please_ rename dma_*_writecombine() to dma_*_wc(), so that the naming is
coherent across the various APIs?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Kishon Vijay Abraham I @ 2015-05-29 11:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150526183955.GW26599@saruman.tx.rr.com>
Hi Felipe,
On Wednesday 27 May 2015 12:09 AM, Felipe Balbi wrote:
> On Tue, May 26, 2015 at 11:37:17AM -0700, Arun Ramamurthy wrote:
>> Hi
>>
>> On 15-05-26 07:19 AM, Felipe Balbi wrote:
>>> HI,
>>>
>>> On Mon, May 25, 2015 at 02:19:58PM -0700, Arun Ramamurthy wrote:
>>>>
>>>>
>>>> On 15-05-14 05:52 PM, Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
>>>>>> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
>>>>>> is only recommended when the config is not visible, GENERIC_PHY is changed
>>>>>> an invisible option. To maintain consistency, all phy providers are changed
>>>>>> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
>>>>>> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
>>>>>> dependency, so it is left as "select".
>>>>>>
>>>>>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>>>>>> ---
>>>>>> drivers/ata/Kconfig | 1 -
>>>>>> drivers/media/platform/exynos4-is/Kconfig | 2 +-
>>>>>> drivers/phy/Kconfig | 4 ++--
>>>>>> drivers/usb/host/Kconfig | 4 ++--
>>>>>> drivers/video/fbdev/exynos/Kconfig | 2 +-
>>>>>> 5 files changed, 6 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>>>>>> index 5f60155..6d2e881 100644
>>>>>> --- a/drivers/ata/Kconfig
>>>>>> +++ b/drivers/ata/Kconfig
>>>>>> @@ -301,7 +301,6 @@ config SATA_MV
>>>>>> tristate "Marvell SATA support"
>>>>>> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
>>>>>> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
>>>>>> - select GENERIC_PHY
>>>>>> help
>>>>>> This option enables support for the Marvell Serial ATA family.
>>>>>> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
>>>>>> diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
>>>>>> index b7b2e47..b6f3eaa 100644
>>>>>> --- a/drivers/media/platform/exynos4-is/Kconfig
>>>>>> +++ b/drivers/media/platform/exynos4-is/Kconfig
>>>>>> @@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
>>>>>> config VIDEO_S5P_MIPI_CSIS
>>>>>> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
>>>>>> depends on REGULATOR
>>>>>> - select GENERIC_PHY
>>>>>> + depends on GENERIC_PHY
>>>>>> help
>>>>>> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
>>>>>> receiver (MIPI-CSIS) devices.
>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>> index 2962de2..edecdb1 100644
>>>>>> --- a/drivers/phy/Kconfig
>>>>>> +++ b/drivers/phy/Kconfig
>>>>>> @@ -5,7 +5,7 @@
>>>>>> menu "PHY Subsystem"
>>>>>>
>>>>>> config GENERIC_PHY
>>>>>> - bool "PHY Core"
>>>>>> + bool
>>>>>> help
>>>>>> Generic PHY support.
>>>>>>
>>>>>> @@ -72,7 +72,7 @@ config PHY_MIPHY365X
>>>>>> config PHY_RCAR_GEN2
>>>>>> tristate "Renesas R-Car generation 2 USB PHY driver"
>>>>>> depends on ARCH_SHMOBILE
>>>>>> - depends on GENERIC_PHY
>>>>>> + select GENERIC_PHY
>>>>>
>>>>> so some you changed from depends to select...
>>>>>
>>>>>> help
>>>>>> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>>>>>>
>>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>>> index 5ad60e4..e2197e2 100644
>>>>>> --- a/drivers/usb/host/Kconfig
>>>>>> +++ b/drivers/usb/host/Kconfig
>>>>>> @@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
>>>>>> config USB_EHCI_HCD_STI
>>>>>> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
>>>>>> depends on ARCH_STI && OF
>>>>>> - select GENERIC_PHY
>>>>>> + depends on GENERIC_PHY
>>>>>
>>>>> while others you changed from select to depends.
>>>>>
>>>>> NAK.
>>>>>
>>>> Felipe, I dont understand your concern, could you please explain it more
>>>> detail? The logic behind the changes is that in cases where there was an
>>>> explicit dependency, I changed it to "depends on" and in other cases I
>>>> changed it to "selects". Thanks
>>>
>>> Since GENERIC_PHY is visible from Kconfig, it would be much nicer to
>>> avoid select altogether.
>>>
>> Felipe, after discussion with the maintainers, I have made GENERIC_PHY an
>> invisible option as part of this change. Thanks
>
> Then, if the option is invisible, how can you "depend" on it ? It can
> never be selected by poking around in Kconfig. IMO, it's
> counterintuitive that you need to enable a PHY driver before you can see
> your EHCI/OHCI/whatever controller listed in Kconfig.
If the controller requires PHY for it to be functional, it is okay to make the
controller depend on PHY IMHO. We want to try and minimize the usage of
'select' wherever possible or else 'select' is the most intuitive way. The
other option is just to leave the 'depends on' and let the user select PHY.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Kishon Vijay Abraham I @ 2015-05-29 12:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1429743853-10254-2-git-send-email-arun.ramamurthy@broadcom.com>
Tejun, Maxime, Sylwester, Kyungmin
On Thursday 23 April 2015 04:34 AM, Arun Ramamurthy wrote:
> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> is only recommended when the config is not visible, GENERIC_PHY is changed
> an invisible option. To maintain consistency, all phy providers are changed
> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> dependency, so it is left as "select".
>
> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
Need your ACK for this patch.
Thanks
Kishon
> ---
> drivers/ata/Kconfig | 1 -
> drivers/media/platform/exynos4-is/Kconfig | 2 +-
> drivers/phy/Kconfig | 4 ++--
> drivers/usb/host/Kconfig | 4 ++--
> drivers/video/fbdev/exynos/Kconfig | 2 +-
> 5 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 5f60155..6d2e881 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -301,7 +301,6 @@ config SATA_MV
> tristate "Marvell SATA support"
> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> - select GENERIC_PHY
> help
> This option enables support for the Marvell Serial ATA family.
> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> index b7b2e47..b6f3eaa 100644
> --- a/drivers/media/platform/exynos4-is/Kconfig
> +++ b/drivers/media/platform/exynos4-is/Kconfig
> @@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> config VIDEO_S5P_MIPI_CSIS
> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> depends on REGULATOR
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> help
> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> receiver (MIPI-CSIS) devices.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2962de2..edecdb1 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -5,7 +5,7 @@
> menu "PHY Subsystem"
>
> config GENERIC_PHY
> - bool "PHY Core"
> + bool
> help
> Generic PHY support.
>
> @@ -72,7 +72,7 @@ config PHY_MIPHY365X
> config PHY_RCAR_GEN2
> tristate "Renesas R-Car generation 2 USB PHY driver"
> depends on ARCH_SHMOBILE
> - depends on GENERIC_PHY
> + select GENERIC_PHY
> help
> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 5ad60e4..e2197e2 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> config USB_EHCI_HCD_STI
> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> depends on ARCH_STI && OF
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> select USB_EHCI_HCD_PLATFORM
> help
> Enable support for the on-chip EHCI controller found on
> @@ -409,7 +409,7 @@ config USB_OHCI_HCD_SPEAR
> config USB_OHCI_HCD_STI
> tristate "Support for ST STiHxxx on-chip OHCI USB controller"
> depends on ARCH_STI && OF
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> select USB_OHCI_HCD_PLATFORM
> help
> Enable support for the on-chip OHCI controller found on
> diff --git a/drivers/video/fbdev/exynos/Kconfig b/drivers/video/fbdev/exynos/Kconfig
> index 1f16b46..6c53894 100644
> --- a/drivers/video/fbdev/exynos/Kconfig
> +++ b/drivers/video/fbdev/exynos/Kconfig
> @@ -16,7 +16,7 @@ if EXYNOS_VIDEO
>
> config EXYNOS_MIPI_DSI
> bool "EXYNOS MIPI DSI driver support."
> - select GENERIC_PHY
> + depends on GENERIC_PHY
> help
> This enables support for MIPI-DSI device.
>
>
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Maxime Coquelin @ 2015-05-29 12:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55685D7E.9000700@ti.com>
Hi Kishon, Arun,
On 05/29/2015 02:37 PM, Kishon Vijay Abraham I wrote:
> Tejun, Maxime, Sylwester, Kyungmin
>
> On Thursday 23 April 2015 04:34 AM, Arun Ramamurthy wrote:
>> Most of the phy providers use "select" to enable GENERIC_PHY. Since
>> select
>> is only recommended when the config is not visible, GENERIC_PHY is
>> changed
>> an invisible option. To maintain consistency, all phy providers are
>> changed
>> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when
>> the
>> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
>> dependency, so it is left as "select".
>>
>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>
> Need your ACK for this patch.
>
For the STi part, you can add my:
Acked-by: Maxime Coquelin <maxime.coquelin@st.com>
Regards,
Maxime
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Tejun Heo @ 2015-05-29 13:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55685D7E.9000700@ti.com>
On Fri, May 29, 2015 at 06:07:18PM +0530, Kishon Vijay Abraham I wrote:
> Tejun, Maxime, Sylwester, Kyungmin
>
> On Thursday 23 April 2015 04:34 AM, Arun Ramamurthy wrote:
> >Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> >is only recommended when the config is not visible, GENERIC_PHY is changed
> >an invisible option. To maintain consistency, all phy providers are changed
> >to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> >phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> >dependency, so it is left as "select".
> >
> >Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>
> Need your ACK for this patch.
For ATA part,
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Sylwester Nawrocki @ 2015-05-29 13:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55685D7E.9000700@ti.com>
On 29/05/15 14:37, Kishon Vijay Abraham I wrote:
> Tejun, Maxime, Sylwester, Kyungmin
>
> On Thursday 23 April 2015 04:34 AM, Arun Ramamurthy wrote:
>> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
>> is only recommended when the config is not visible, GENERIC_PHY is changed
>> an invisible option. To maintain consistency, all phy providers are changed
>> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
>> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
>> dependency, so it is left as "select".
>>
>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>
> Need your ACK for this patch.
For
drivers/media/platform/exynos4-is/Kconfig
drivers/video/fbdev/exynos/Kconfig
Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
--
Thanks,
Sylwester
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Felipe Balbi @ 2015-05-29 15:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <55684ECE.9060003@ti.com>
[-- Attachment #1: Type: text/plain, Size: 5275 bytes --]
Hi,
On Fri, May 29, 2015 at 05:04:38PM +0530, Kishon Vijay Abraham I wrote:
> Hi Felipe,
>
> On Wednesday 27 May 2015 12:09 AM, Felipe Balbi wrote:
> >On Tue, May 26, 2015 at 11:37:17AM -0700, Arun Ramamurthy wrote:
> >>Hi
> >>
> >>On 15-05-26 07:19 AM, Felipe Balbi wrote:
> >>>HI,
> >>>
> >>>On Mon, May 25, 2015 at 02:19:58PM -0700, Arun Ramamurthy wrote:
> >>>>
> >>>>
> >>>>On 15-05-14 05:52 PM, Felipe Balbi wrote:
> >>>>>Hi,
> >>>>>
> >>>>>On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
> >>>>>>Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> >>>>>>is only recommended when the config is not visible, GENERIC_PHY is changed
> >>>>>>an invisible option. To maintain consistency, all phy providers are changed
> >>>>>>to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> >>>>>>phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> >>>>>>dependency, so it is left as "select".
> >>>>>>
> >>>>>>Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
> >>>>>>---
> >>>>>> drivers/ata/Kconfig | 1 -
> >>>>>> drivers/media/platform/exynos4-is/Kconfig | 2 +-
> >>>>>> drivers/phy/Kconfig | 4 ++--
> >>>>>> drivers/usb/host/Kconfig | 4 ++--
> >>>>>> drivers/video/fbdev/exynos/Kconfig | 2 +-
> >>>>>> 5 files changed, 6 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>>diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> >>>>>>index 5f60155..6d2e881 100644
> >>>>>>--- a/drivers/ata/Kconfig
> >>>>>>+++ b/drivers/ata/Kconfig
> >>>>>>@@ -301,7 +301,6 @@ config SATA_MV
> >>>>>> tristate "Marvell SATA support"
> >>>>>> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> >>>>>> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> >>>>>>- select GENERIC_PHY
> >>>>>> help
> >>>>>> This option enables support for the Marvell Serial ATA family.
> >>>>>> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> >>>>>>diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>index b7b2e47..b6f3eaa 100644
> >>>>>>--- a/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>+++ b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>@@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> >>>>>> config VIDEO_S5P_MIPI_CSIS
> >>>>>> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> >>>>>> depends on REGULATOR
> >>>>>>- select GENERIC_PHY
> >>>>>>+ depends on GENERIC_PHY
> >>>>>> help
> >>>>>> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> >>>>>> receiver (MIPI-CSIS) devices.
> >>>>>>diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> >>>>>>index 2962de2..edecdb1 100644
> >>>>>>--- a/drivers/phy/Kconfig
> >>>>>>+++ b/drivers/phy/Kconfig
> >>>>>>@@ -5,7 +5,7 @@
> >>>>>> menu "PHY Subsystem"
> >>>>>>
> >>>>>> config GENERIC_PHY
> >>>>>>- bool "PHY Core"
> >>>>>>+ bool
> >>>>>> help
> >>>>>> Generic PHY support.
> >>>>>>
> >>>>>>@@ -72,7 +72,7 @@ config PHY_MIPHY365X
> >>>>>> config PHY_RCAR_GEN2
> >>>>>> tristate "Renesas R-Car generation 2 USB PHY driver"
> >>>>>> depends on ARCH_SHMOBILE
> >>>>>>- depends on GENERIC_PHY
> >>>>>>+ select GENERIC_PHY
> >>>>>
> >>>>>so some you changed from depends to select...
> >>>>>
> >>>>>> help
> >>>>>> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
> >>>>>>
> >>>>>>diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> >>>>>>index 5ad60e4..e2197e2 100644
> >>>>>>--- a/drivers/usb/host/Kconfig
> >>>>>>+++ b/drivers/usb/host/Kconfig
> >>>>>>@@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> >>>>>> config USB_EHCI_HCD_STI
> >>>>>> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> >>>>>> depends on ARCH_STI && OF
> >>>>>>- select GENERIC_PHY
> >>>>>>+ depends on GENERIC_PHY
> >>>>>
> >>>>>while others you changed from select to depends.
> >>>>>
> >>>>>NAK.
> >>>>>
> >>>>Felipe, I dont understand your concern, could you please explain it more
> >>>>detail? The logic behind the changes is that in cases where there was an
> >>>>explicit dependency, I changed it to "depends on" and in other cases I
> >>>>changed it to "selects". Thanks
> >>>
> >>>Since GENERIC_PHY is visible from Kconfig, it would be much nicer to
> >>>avoid select altogether.
> >>>
> >>Felipe, after discussion with the maintainers, I have made GENERIC_PHY an
> >>invisible option as part of this change. Thanks
> >
> >Then, if the option is invisible, how can you "depend" on it ? It can
> >never be selected by poking around in Kconfig. IMO, it's
> >counterintuitive that you need to enable a PHY driver before you can see
> >your EHCI/OHCI/whatever controller listed in Kconfig.
>
> If the controller requires PHY for it to be functional, it is okay to make
> the controller depend on PHY IMHO. We want to try and minimize the usage of
> 'select' wherever possible or else 'select' is the most intuitive way. The
> other option is just to leave the 'depends on' and let the user select PHY.
How can you 'depend' on something that the user can't select by
navigating through Kconfig ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-05-29 19:24 UTC (permalink / raw)
To: Tomi Valkeinen, H. Peter Anvin, Andy Lutomirski, Juergen Gross,
Toshi Kani, Suresh Siddha, Thomas Gleixner, Borislav Petkov,
Ingo Molnar, X86 ML
Cc: Bjorn Helgaas, Dave Airlie, linux-fbdev,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
xen-devel@lists.xensource.com, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Dave Hansen,
Arnd Bergmann, Michael S. Tsirkin, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso, Konrad Rzeszutek Wilk, Ville Syrjälä,
David Vrabel, Jan Beulich, Roger Pau Monné
In-Reply-To: <5567FFB6.5050108@ti.com>
On Thu, May 28, 2015 at 10:57 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
>
> On 29/05/15 03:36, Luis R. Rodriguez wrote:
>> On Wed, May 27, 2015 at 1:04 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
>>> On Tue, May 26, 2015 at 12:40:08PM -0500, Bjorn Helgaas wrote:
>>>> On Fri, May 22, 2015 at 02:23:41AM +0200, Luis R. Rodriguez wrote:
>>>>> On Thu, May 21, 2015 at 05:33:21PM -0500, Bjorn Helgaas wrote:
>>>>>>
>>>>>> I tentatively put this (and the rest of the series) on a pci/resource
>>>>>> branch. I'm hoping you'll propose some clarification about
>>>>>> EXPORT_SYMBOL_GPL().
>>>>>
>>>>> EXPORT_SYMBOL_GPL() also serves to ensure only GPL modules can
>>>>> only run that code. So for instance although we have "Dual BSD/GPL"
>>>>> tags for modules pure "BSD" tags do not exist for module tags and
>>>>> cannot run EXPORT_SYMBOL_GPL() code [0]. Also there is some folks
>>>>> who do believe tha at run time all kernel modules are GPL [1] [2].
>>>>> And to be precise even though the FSF may claim a list of licenses
>>>>> are GPL-compatible we cannot rely on this list alone for our own
>>>>> goals and if folks want to use our EXPORT_SYMBOL_GPL()s they must
>>>>> discuss this on lkml [2].
>>>>
>>>> By "propose some clarification," I meant that I hoped you would propose a
>>>> patch to Documentation/ that would give maintainers some guidance.
>>>
>>> I *really really* would hate to do so but only because you insist, I'll look
>>> into this...
>>
>> OK done. Please let me know if there is anything else I can do to
>> help. Also as per review with Tomi, the framebuffer maintainer, he
>> would prefer for only the required symbols to go through your tree.
>> We'd then wait for the next merge window for them to perculate to
>> Linus' tree and once there I'd send him a pull request for the
>> framebuffer device driver changes alone. So this does mean we'll have
>> no users of the symbols for a full release, but again, this is as per
>> Tomi's preference. This strategy is also the preference then for the
>> pci_iomap_wc() series as well. With that in mind, perhaps the lib
>> patch can go in as we'd have no users but we do have a few future
>> possible expected users.
>
> I don't have any issue with fbdev changes going through other trees, but
> I'd rather do that only if there are good reasons to go that way.
OK, either way I prefer to go with maintainer's preferences. Most
changes are for framebuffer drivers as that's where MTRR is used most
these days.
> These changes to fbdev drivers look like cleanups, so they are not
> critical, right?
I'll let you make the call, I'll just provide information to you. I
trust your judgement and what you prefer.
This and other series which change use of MTRR to arch_phys enable use
of PAT when available, we want to bury out MTRR from further usage so
all these arch_phys changes will help with that. MTRR, although
supported, should be seen as a first step temporary architectural
evolution to what PAT became. There are known architectural issue with
MTRR, let me list the issues for you to review and consider and
evaluate:
* MTRR acts on physical addresses and requires power-of-two
alignment, on both the base used and size, this limits the flexibility
of MTRR use
* MTRR is known to be unreliable, it can at times not work even on
modern systems
* MTRRs are limited, if using a large number of devices MTRRs will
run out fast, its why Andy ended up adding the arch_phys APIs
* PAT has been available for quite a long time, since Pentium III
(circa 1999) and newer, but having PAT enabled does not restrict use
of MTRR and because of this some systems may end up then combining
MTRR and PAT. I do not believe this wasn't an original highly expected
wide use situation, it technically should work to combine both but
there might be issues with interactions between both, exactly what
issues can exist or have existed remains quite unclear as MTRR in and
of itself has been known to be unreliable anyway. If possible its best
to just be binary about this and only use MTRR if and only if
necessary because of the other issues known with MTRR.
With all these changes being merged the only use case for MTRR then
would be through the arch_phys APIs, which would just enable use of
MTRR when PAT is not available or a system / driver is known to
require MTRR -- those systems are expected to low numbered, in the end
after all these series Linux will will end up with only two device
drivers which will require MTRR to be enabled always:
* ipath: this device driver is old, powers the old HTX bus cards
that only work in AMD systems, while the newer IB/qib device driver
powers all PCI-e cards. The ipath device driver is obsolete, hardware
hard to find. In fact the maintainers of this driver have recently
even seriously discussed removing the driver from upstream altogether.
* ivtv: the hardware is really rare these days, its expected only
some lost souls in some third world country are expected to be using
the feature which requires MTRR. The way this driver uses MTRR is also
quite questionable, but still has been present in the driver for a
long time.
These two device drivers can get PAT disabled by just using the kernel
parameter to disable PAT upon boot. The kernel log will tell them what
to do exactly.
Another use case for still using MTRR are cases where PAT is known to
have errata. We actually want to learn about those users who have
issues with PAT though. Last I heard about this was that the required
errata folks are aware of can be worked around by having PAT entry 0
and entry 4 both map to WB. hpa / andy might be able to elaborate if
needed, but we might even be able to preemptively already deal with
the errata issues for users.
Now let me explain the gains with all these changes being merged:
* Although there are known PAT errata, we want to learn about those
issues and fix the issues
* We have a long term goal to change the default behavior of
ioremap_nocache() and pci_mmap_page_range() to use PAT strong UC,
right now we cannot do this, but after all drivers are converted (all
these series I've been posting) we expect to be able to make the
change. Making a change to strong UC on these two calls can only
happen after a period of time of having Linux bake with all these
changes merged and in place. How many kernels we will want Linux baked
with all these transformations to arch_phys before making a change to
ioremap_nocache() and pci_mmap_page_range() is up to x86 folks. There
are other gains possible with this but I welcome others to chime in
here with what gains we can expect from this
* Xen lacks support for MTRR upstream, but has full PAT support.
These changes will help enable use of the devices which exclusively
used MTRR to work on Xen with write combining moving forward
> Does delaying the fbdev changes until the dependencies are in prevent some other development?
Just the later work on putting the nail on the MTRR coffin, and
delaying the change of default ioremap_nocache() and
pci_mmap_page_range() from UC- to strong UC. We ideally should want a
few kernel release with all the fbdev changes in place, not sure how
many we'd want, but the sooner we get folks testing kernels with that
the sooner we'd be able to evaluate doing a flip form UC- to strong
UC.
Luis
^ permalink raw reply
* Re: [PATCH v4 0/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-05-29 21:04 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: Luis R. Rodriguez, linux-kernel, linux-fbdev
In-Reply-To: <5567FDBE.8010701@ti.com>
On Fri, May 29, 2015 at 08:48:46AM +0300, Tomi Valkeinen wrote:
>
>
> On 29/05/15 03:30, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > Tomi,
> >
> > Upon integration onto your tree of the series, "[PATCH v3 00/17] framebuffer:
> > simple conversions to arch_phys_wc_add()" the 0 day build bot found a
> > compilation issue on the gbefb driver. I had test compiled drivers with
> > allyesconfig and allmodconfig but failed to test compile against MIPS. This
> > driver is enabled *only for MIPS*. For the life me I could not get a MIPS
> > cross compiler even on debian, so what I did to test this was incorporate into
> > my private tree a temporary patch [0] which enables this driver to compile on
> > x86 and go test compile with that as a temporary patch. The compilation was
> > failing since I used the info struct instead of the actual private data
> > structure. This fixes that and moves its assignment early. Sorry about that.
> >
> > The rest of the series does not require changes for integration after these
> > two patch replacements. Let me know if you'd like me to respin the entire
> > series though, but I didn't since I figured its pointless as the patches remain
> > intact. For your convenience however I've rebased all these 17 patches onto
> > your latest tree on the for-next branch, you can pull the changes with the
> > details provided below. This v4 iteration only carries the two patches which
> > required updates. The details of the full pull request go below this.
>
> Thanks, I've updated the two patches, and pushed the series to my for-next.
And yet another corner case, which compilation would not have picked up but
only grammar would. Best handled now before it being merged. The same gbefb
MIPS patch had a missing change from dma_free_coherent() to
dma_free_writecombine(), this is needed since the gbefb is changed to use
dma_alloc_writecombine(). The change required is illustrated below. Terribly
sorry about that... I'll send a v5 pull request unless you want that to
go separately. Meanwhile I've verified the other series I have for MTRR
and none of them use these APIs so this is the only one with this
inconsistancy.
diff --git a/drivers/video/fbdev/gbefb.c b/drivers/video/fbdev/gbefb.c
index d2601808..b63d55f 100644
--- a/drivers/video/fbdev/gbefb.c
+++ b/drivers/video/fbdev/gbefb.c
@@ -1238,7 +1238,7 @@ static int gbefb_probe(struct platform_device *p_dev)
out_gbe_unmap:
arch_phys_wc_del(par->wc_cookie);
if (gbe_dma_addr)
- dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
+ dma_free_writecombine(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
out_tiles_free:
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
@@ -1259,7 +1259,7 @@ static int gbefb_remove(struct platform_device* p_dev)
gbe_turn_off();
arch_phys_wc_del(par->wc_cookie);
if (gbe_dma_addr)
- dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
+ dma_free_writecombine(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
Luis
^ permalink raw reply related
* [PATCH v5 0/17] framebuffer: simple conversions to arch_phys_wc_add()
From: Luis R. Rodriguez @ 2015-05-29 22:07 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-kernel, linux-fbdev, Luis R. Rodriguez
From: "Luis R. Rodriguez" <mcgrof@suse.com>
Tomi,
this v5 series goes with the patch 3/17 titled "video: fbdev: gbefb:
use arch_phys_wc_add() and devm_ioremap_wc()" with a change from
dma_free_coherent() to dma_free_writecombine() for the area now allocated
with dma_alloc_writecombine(). The remaining patches remain the same.
As for the pull request all I did was reset my tree to yours, and
then reset it to the same old commit 36520841a443d5ee966f9632c417fcc8a25e07e3,
then applied the new series. The only delta from the v4 seires is the
patch 3/17. The new branch changes from simple-arch_phys-20150528 to
simple-arch_phys-20150529. Pull request below for your convenience.
Luis R. Rodriguez (1):
video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
drivers/video/fbdev/gbefb.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
The following changes since commit 36520841a443d5ee966f9632c417fcc8a25e07e3:
video/console: use swap() in newport_bmove() (2015-05-27 12:54:49 +0300)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-framebuffer.git simple-arch_phys-20150529
for you to fetch changes up to d8328b5f2175f1ae2f9dc4db6140af67c9196989:
video: fbdev: geode gxfb: use ioremap_wc() for framebuffer (2015-05-29 14:57:49 -0700)
----------------------------------------------------------------
Luis R. Rodriguez (17):
video: fbdev: radeonfb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: gbefb: add missing mtrr_del() calls
video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
video: fbdev: intelfb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: neofb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: nvidia: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: savagefb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: sisfb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: aty: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: i810: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: pm2fb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: pm3fb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: rivafb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: tdfxfb: use arch_phys_wc_add() and ioremap_wc()
video: fbdev: atmel_lcdfb: use ioremap_wc() for framebuffer
video: fbdev: geode gxfb: use ioremap_wc() for framebuffer
drivers/video/fbdev/atmel_lcdfb.c | 3 +-
drivers/video/fbdev/aty/aty128fb.c | 36 ++++--------------------
drivers/video/fbdev/aty/radeon_base.c | 29 ++++----------------
drivers/video/fbdev/aty/radeonfb.h | 2 +-
drivers/video/fbdev/gbefb.c | 25 ++++++++---------
drivers/video/fbdev/geode/gxfb_core.c | 3 +-
drivers/video/fbdev/i810/i810.h | 3 +-
drivers/video/fbdev/i810/i810_main.c | 11 +++++---
drivers/video/fbdev/i810/i810_main.h | 26 ------------------
drivers/video/fbdev/intelfb/intelfb.h | 4 +--
drivers/video/fbdev/intelfb/intelfbdrv.c | 38 +++-----------------------
drivers/video/fbdev/matrox/matroxfb_base.c | 36 +++++++++---------------
drivers/video/fbdev/matrox/matroxfb_base.h | 27 +-----------------
drivers/video/fbdev/neofb.c | 26 +++++-------------
drivers/video/fbdev/nvidia/nv_type.h | 7 +----
drivers/video/fbdev/nvidia/nvidia.c | 37 ++++---------------------
drivers/video/fbdev/pm2fb.c | 31 ++++-----------------
drivers/video/fbdev/pm3fb.c | 30 ++++----------------
drivers/video/fbdev/riva/fbdev.c | 39 +++++---------------------
drivers/video/fbdev/riva/rivafb.h | 4 +--
drivers/video/fbdev/savage/savagefb.h | 4 +--
drivers/video/fbdev/savage/savagefb_driver.c | 17 ++----------
drivers/video/fbdev/sis/sis.h | 2 +-
drivers/video/fbdev/sis/sis_main.c | 27 ++++--------------
drivers/video/fbdev/tdfxfb.c | 41 ++++------------------------
include/video/neomagic.h | 5 +---
include/video/tdfx.h | 2 +-
27 files changed, 108 insertions(+), 407 deletions(-)
^ permalink raw reply
* [PATCH v5 3/17] video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-29 22:07 UTC (permalink / raw)
To: tomi.valkeinen
Cc: linux-kernel, linux-fbdev, Luis R. Rodriguez, Thomas Gleixner,
Juergen Gross, Jingoo Han, Daniel Vetter, Masanari Iida,
Suresh Siddha, Ingo Molnar, Andy Lutomirski, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard
In-Reply-To: <1432937236-9327-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
Convert the driver from using the x86 specific MTRR code to
the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
will avoid MTRR if write-combining is available, in order to
take advantage of that also ensure the ioremap'd area is requested
as write-combining.
There are a few motivations for this:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
The conversion done is expressed by the following Coccinelle
SmPL patch, it additionally required manual intervention to
address all the #ifdery and removal of redundant things which
arch_phys_wc_add() already addresses such as verbose message
about when MTRR fails and doing nothing when we didn't get
an MTRR.
@ mtrr_found @
expression index, base, size;
@@
-index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
+index = arch_phys_wc_add(base, size);
@ mtrr_rm depends on mtrr_found @
expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
@@
-mtrr_del(index, base, size);
+arch_phys_wc_del(index);
@ mtrr_rm_zero_arg depends on mtrr_found @
expression mtrr_found.index;
@@
-mtrr_del(index, 0, 0);
+arch_phys_wc_del(index);
@ mtrr_rm_fb_info depends on mtrr_found @
struct fb_info *info;
expression mtrr_found.index;
@@
-mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
+arch_phys_wc_del(index);
@ ioremap_replace_nocache depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap_nocache(base, size);
+info->screen_base = ioremap_wc(base, size);
@ ioremap_replace_default depends on mtrr_found @
struct fb_info *info;
expression base, size;
@@
-info->screen_base = ioremap(base, size);
+info->screen_base = ioremap_wc(base, size);
Generated-by: Coccinelle SmPL
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Masanari Iida <standby24x7@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/video/fbdev/gbefb.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/video/fbdev/gbefb.c b/drivers/video/fbdev/gbefb.c
index 4e54faa06..b63d55f 100644
--- a/drivers/video/fbdev/gbefb.c
+++ b/drivers/video/fbdev/gbefb.c
@@ -22,9 +22,6 @@
#include <linux/module.h>
#include <linux/io.h>
-#ifdef CONFIG_X86
-#include <asm/mtrr.h>
-#endif
#ifdef CONFIG_MIPS
#include <asm/addrspace.h>
#endif
@@ -1176,8 +1173,8 @@ static int gbefb_probe(struct platform_device *p_dev)
if (gbe_mem_phys) {
/* memory was allocated at boot time */
- gbe_mem = devm_ioremap_nocache(&p_dev->dev, gbe_mem_phys,
- gbe_mem_size);
+ gbe_mem = devm_ioremap_wc(&p_dev->dev, gbe_mem_phys,
+ gbe_mem_size);
if (!gbe_mem) {
printk(KERN_ERR "gbefb: couldn't map framebuffer\n");
ret = -ENOMEM;
@@ -1188,8 +1185,8 @@ static int gbefb_probe(struct platform_device *p_dev)
} else {
/* try to allocate memory with the classical allocator
* this has high chance to fail on low memory machines */
- gbe_mem = dma_alloc_coherent(NULL, gbe_mem_size, &gbe_dma_addr,
- GFP_KERNEL);
+ gbe_mem = dma_alloc_writecombine(NULL, gbe_mem_size,
+ &gbe_dma_addr, GFP_KERNEL);
if (!gbe_mem) {
printk(KERN_ERR "gbefb: couldn't allocate framebuffer memory\n");
ret = -ENOMEM;
@@ -1200,10 +1197,7 @@ static int gbefb_probe(struct platform_device *p_dev)
}
par = info->par;
-#ifdef CONFIG_X86
- par->wc_cookie = mtrr_add(gbe_mem_phys, gbe_mem_size,
- MTRR_TYPE_WRCOMB, 1);
-#endif
+ par->wc_cookie = arch_phys_wc_add(gbe_mem_phys, gbe_mem_size);
/* map framebuffer memory into tiles table */
for (i = 0; i < (gbe_mem_size >> TILE_SHIFT); i++)
@@ -1242,12 +1236,9 @@ static int gbefb_probe(struct platform_device *p_dev)
return 0;
out_gbe_unmap:
-#ifdef CONFIG_MTRR
- if (par->wc_cookie >= 0)
- mtrr_del(par->wc_cookie, 0, 0);
-#endif
+ arch_phys_wc_del(par->wc_cookie);
if (gbe_dma_addr)
- dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
+ dma_free_writecombine(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
out_tiles_free:
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
@@ -1266,12 +1257,9 @@ static int gbefb_remove(struct platform_device* p_dev)
unregister_framebuffer(info);
gbe_turn_off();
-#ifdef CONFIG_MTRR
- if (par->wc_cookie >= 0)
- mtrr_del(par->wc_cookie, 0, 0);
-#endif
+ arch_phys_wc_del(par->wc_cookie);
if (gbe_dma_addr)
- dma_free_coherent(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
+ dma_free_writecombine(NULL, gbe_mem_size, gbe_mem, gbe_mem_phys);
dma_free_coherent(NULL, GBE_TLB_SIZE * sizeof(uint16_t),
(void *)gbe_tiles.cpu, gbe_tiles.dma);
release_mem_region(GBE_BASE, sizeof(struct sgi_gbe));
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v4 3/17] video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
From: Luis R. Rodriguez @ 2015-05-30 0:32 UTC (permalink / raw)
To: Ingo Molnar, tomi.valkeinen, Dave Airlie
Cc: Luis R. Rodriguez, linux-kernel, linux-fbdev, Thomas Gleixner,
Juergen Gross, Jingoo Han, Daniel Vetter, Masanari Iida,
Suresh Siddha, Ingo Molnar, Andy Lutomirski, Antonino Daplas,
Jean-Christophe Plagniol-Villard
In-Reply-To: <20150529064144.GA22749@gmail.com>
On Fri, May 29, 2015 at 08:41:44AM +0200, Ingo Molnar wrote:
>
> * Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>
> > + gbe_mem = devm_ioremap_wc(&p_dev->dev, gbe_mem_phys,
> > + gbe_mem_size);
>
> > + gbe_mem = dma_alloc_writecombine(NULL, gbe_mem_size,
> > + &gbe_dma_addr, GFP_KERNEL);
>
> > + par->wc_cookie = arch_phys_wc_add(gbe_mem_phys, gbe_mem_size);
>
> > + arch_phys_wc_del(par->wc_cookie);
>
> > + arch_phys_wc_del(par->wc_cookie);
>
> Could we _please_ rename dma_*_writecombine() to dma_*_wc(), so that the naming is
> coherent across the various APIs?
Sure, OK done. Will send once this series is merged and visible on linux-next.
But who's tree should this go through? Since it depends on this series which
goes through Tomi's tree should it go through there?
Luis
^ permalink raw reply
* [RFC 13/24] powerpc: Cleanup nvram includes
From: Finn Thain @ 2015-05-31 1:01 UTC (permalink / raw)
To: linux-kernel, linux-m68k, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Arnd Bergmann,
Greg Kroah-Hartman, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <20150531010132.289674310@telegraphics.com.au>
The nvram_read_byte() and nvram_write_byte() definitions in asm/nvram.h
duplicate those in linux/nvram.h. Get rid of the former to prepare for
adoption of struct arch_nvram_ops (which is defined in linux/nvram.h for
general use).
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
arch/powerpc/include/asm/nvram.h | 3 ---
arch/powerpc/kernel/setup_32.c | 1 +
drivers/char/generic_nvram.c | 4 +++-
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
Index: linux/arch/powerpc/include/asm/nvram.h
=================================--- linux.orig/arch/powerpc/include/asm/nvram.h 2015-05-31 11:00:59.000000000 +1000
+++ linux/arch/powerpc/include/asm/nvram.h 2015-05-31 11:01:13.000000000 +1000
@@ -101,7 +101,4 @@ extern int nvram_write_os_partition(stru
/* Determine NVRAM size */
extern ssize_t nvram_get_size(void);
-/* Normal access to NVRAM */
-extern unsigned char nvram_read_byte(int i);
-extern void nvram_write_byte(unsigned char c, int i);
#endif /* _ASM_POWERPC_NVRAM_H */
Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c 2015-05-31 11:01:13.000000000 +1000
@@ -16,6 +16,7 @@
#include <linux/cpu.h>
#include <linux/console.h>
#include <linux/memblock.h>
+#include <linux/nvram.h>
#include <asm/io.h>
#include <asm/prom.h>
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/char/generic_nvram.c 2015-05-31 11:01:13.000000000 +1000
@@ -20,9 +20,11 @@
#include <linux/fcntl.h>
#include <linux/init.h>
#include <linux/mutex.h>
+#include <linux/nvram.h>
#include <asm/uaccess.h>
-#include <asm/nvram.h>
+
#ifdef CONFIG_PPC_PMAC
+#include <asm/nvram.h>
#include <asm/machdep.h>
#endif
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c 2015-05-31 11:01:13.000000000 +1000
@@ -111,12 +111,12 @@
#include "matroxfb_g450.h"
#include <linux/matroxfb.h>
#include <linux/interrupt.h>
+#include <linux/nvram.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
#ifdef CONFIG_PPC_PMAC
#include <asm/machdep.h>
-unsigned char nvram_read_byte(int);
static int default_vmode = VMODE_NVRAM;
static int default_cmode = CMODE_NVRAM;
#endif
^ permalink raw reply
* [RFC 17/24] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_
From: Finn Thain @ 2015-05-31 1:01 UTC (permalink / raw)
To: linux-kernel, linux-m68k, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Arnd Bergmann,
Greg Kroah-Hartman, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <20150531010132.289674310@telegraphics.com.au>
Make use of arch_nvram_ops in device drivers so that the nvram_*
function exports can be removed.
Since they are no longer global symbols, rename the PPC32 nvram_* functions
appropriately.
Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
arch/powerpc/kernel/setup_32.c | 8 ++++----
drivers/char/generic_nvram.c | 4 ++--
drivers/video/fbdev/controlfb.c | 4 ++--
drivers/video/fbdev/imsttfb.c | 7 +++----
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
drivers/video/fbdev/platinumfb.c | 4 ++--
drivers/video/fbdev/valkyriefb.c | 4 ++--
7 files changed, 16 insertions(+), 17 deletions(-)
Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c 2015-05-31 11:01:17.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c 2015-05-31 11:01:19.000000000 +1000
@@ -170,20 +170,18 @@ __setup("l3cr=", ppc_setup_l3cr);
#ifdef CONFIG_GENERIC_NVRAM
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
{
if (ppc_md.nvram_read_val)
return ppc_md.nvram_read_val(addr);
return 0xff;
}
-EXPORT_SYMBOL(nvram_read_byte);
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
{
if (ppc_md.nvram_write_val)
ppc_md.nvram_write_val(addr, val);
}
-EXPORT_SYMBOL(nvram_write_byte);
static ssize_t ppc_nvram_get_size(void)
{
@@ -200,6 +198,8 @@ static long ppc_nvram_sync(void)
}
const struct nvram_ops arch_nvram_ops = {
+ .read_byte = ppc_nvram_read_byte,
+ .write_byte = ppc_nvram_write_byte,
.get_size = ppc_nvram_get_size,
.sync = ppc_nvram_sync,
};
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c 2015-05-31 11:01:17.000000000 +1000
+++ linux/drivers/char/generic_nvram.c 2015-05-31 11:01:19.000000000 +1000
@@ -64,7 +64,7 @@ static ssize_t read_nvram(struct file *f
if (*ppos >= nvram_len)
return 0;
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
- if (__put_user(nvram_read_byte(i), p))
+ if (__put_user(arch_nvram_ops.read_byte(i), p))
return -EFAULT;
*ppos = i;
return p - buf;
@@ -84,7 +84,7 @@ static ssize_t write_nvram(struct file *
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
if (__get_user(c, p))
return -EFAULT;
- nvram_write_byte(c, i);
+ arch_nvram_ops.write_byte(c, i);
}
*ppos = i;
return p - buf;
Index: linux/drivers/video/fbdev/controlfb.c
=================================--- linux.orig/drivers/video/fbdev/controlfb.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/video/fbdev/controlfb.c 2015-05-31 11:01:19.000000000 +1000
@@ -415,7 +415,7 @@ static int __init init_control(struct fb
/* Try to pick a video mode out of NVRAM if we have one. */
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if(cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
} else
@@ -423,7 +423,7 @@ static int __init init_control(struct fb
cmodeÞfault_cmode;
#ifdef CONFIG_NVRAM
if (default_vmode = VMODE_NVRAM) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode < 1 || vmode > VMODE_MAX ||
control_mac_modes[vmode - 1].m[full] < cmode) {
sense = read_control_sense(p);
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c 2015-05-31 11:01:13.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c 2015-05-31 11:01:19.000000000 +1000
@@ -1888,7 +1888,7 @@ static int initMatrox2(struct matrox_fb_
default_vmode = VMODE_640_480_60;
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/platinumfb.c
=================================--- linux.orig/drivers/video/fbdev/platinumfb.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/video/fbdev/platinumfb.c 2015-05-31 11:01:19.000000000 +1000
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_in
printk(KERN_INFO "platinumfb: Monitor sense value = 0x%x, ", sense);
if (default_vmode = VMODE_NVRAM) {
#ifdef CONFIG_NVRAM
- default_vmode = nvram_read_byte(NV_VMODE);
+ default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (default_vmode <= 0 || default_vmode > VMODE_MAX ||
!platinum_reg_init[default_vmode-1])
#endif
@@ -362,7 +362,7 @@ static int platinum_init_fb(struct fb_in
default_vmode = VMODE_640_480_60;
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/valkyriefb.c
=================================--- linux.orig/drivers/video/fbdev/valkyriefb.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c 2015-05-31 11:01:19.000000000 +1000
@@ -287,7 +287,7 @@ static void __init valkyrie_choose_mode(
/* Try to pick a video mode out of NVRAM if we have one. */
#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
if (default_vmode = VMODE_NVRAM) {
- default_vmode = nvram_read_byte(NV_VMODE);
+ default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (default_vmode <= 0
|| default_vmode > VMODE_MAX
|| !valkyrie_reg_init[default_vmode - 1])
@@ -300,7 +300,7 @@ static void __init valkyrie_choose_mode(
default_vmode = VMODE_640_480_67;
#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
/*
Index: linux/drivers/video/fbdev/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c 2015-05-31 11:00:59.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c 2015-05-31 11:01:19.000000000 +1000
@@ -328,7 +328,6 @@ enum {
TVP = 1
};
-#define USE_NV_MODES 1
#define INIT_BPP 8
#define INIT_XRES 640
#define INIT_YRES 480
@@ -1391,17 +1390,17 @@ static void init_imstt(struct fb_info *i
}
}
-#if USE_NV_MODES && defined(CONFIG_PPC32)
+#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
{
int vmode = init_vmode, cmode = init_cmode;
if (vmode = -1) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode <= 0 || vmode > VMODE_MAX)
vmode = VMODE_640_480_67;
}
if (cmode = -1) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if (cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
}
^ permalink raw reply
* Re: [PATCH v4 3/17] video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
From: Ingo Molnar @ 2015-06-01 8:53 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: tomi.valkeinen, Dave Airlie, Luis R. Rodriguez, linux-kernel,
linux-fbdev, Thomas Gleixner, Juergen Gross, Jingoo Han,
Daniel Vetter, Masanari Iida, Suresh Siddha, Ingo Molnar,
Andy Lutomirski, Antonino Daplas,
Jean-Christophe Plagniol-Villard
In-Reply-To: <20150530003256.GG23057@wotan.suse.de>
* Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Fri, May 29, 2015 at 08:41:44AM +0200, Ingo Molnar wrote:
> >
> > * Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
> >
> > > + gbe_mem = devm_ioremap_wc(&p_dev->dev, gbe_mem_phys,
> > > + gbe_mem_size);
> >
> > > + gbe_mem = dma_alloc_writecombine(NULL, gbe_mem_size,
> > > + &gbe_dma_addr, GFP_KERNEL);
> >
> > > + par->wc_cookie = arch_phys_wc_add(gbe_mem_phys, gbe_mem_size);
> >
> > > + arch_phys_wc_del(par->wc_cookie);
> >
> > > + arch_phys_wc_del(par->wc_cookie);
> >
> > Could we _please_ rename dma_*_writecombine() to dma_*_wc(), so that the naming is
> > coherent across the various APIs?
>
> Sure, OK done. Will send once this series is merged and visible on linux-next.
> But who's tree should this go through? Since it depends on this series which
> goes through Tomi's tree should it go through there?
So we could do the rename near the end of the merge window, when all the patches
have drained from maintainer trees into Linus's tree. Mind keeping it in your tree
until then?
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Kishon Vijay Abraham I @ 2015-06-01 12:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20150529150413.GB2026@saruman.tx.rr.com>
Hi,
On Friday 29 May 2015 08:34 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, May 29, 2015 at 05:04:38PM +0530, Kishon Vijay Abraham I wrote:
>> Hi Felipe,
>>
>> On Wednesday 27 May 2015 12:09 AM, Felipe Balbi wrote:
>>> On Tue, May 26, 2015 at 11:37:17AM -0700, Arun Ramamurthy wrote:
>>>> Hi
>>>>
>>>> On 15-05-26 07:19 AM, Felipe Balbi wrote:
>>>>> HI,
>>>>>
>>>>> On Mon, May 25, 2015 at 02:19:58PM -0700, Arun Ramamurthy wrote:
>>>>>>
>>>>>>
>>>>>> On 15-05-14 05:52 PM, Felipe Balbi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
>>>>>>>> Most of the phy providers use "select" to enable GENERIC_PHY. Since select
>>>>>>>> is only recommended when the config is not visible, GENERIC_PHY is changed
>>>>>>>> an invisible option. To maintain consistency, all phy providers are changed
>>>>>>>> to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
>>>>>>>> phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
>>>>>>>> dependency, so it is left as "select".
>>>>>>>>
>>>>>>>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
>>>>>>>> ---
>>>>>>>> drivers/ata/Kconfig | 1 -
>>>>>>>> drivers/media/platform/exynos4-is/Kconfig | 2 +-
>>>>>>>> drivers/phy/Kconfig | 4 ++--
>>>>>>>> drivers/usb/host/Kconfig | 4 ++--
>>>>>>>> drivers/video/fbdev/exynos/Kconfig | 2 +-
>>>>>>>> 5 files changed, 6 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
>>>>>>>> index 5f60155..6d2e881 100644
>>>>>>>> --- a/drivers/ata/Kconfig
>>>>>>>> +++ b/drivers/ata/Kconfig
>>>>>>>> @@ -301,7 +301,6 @@ config SATA_MV
>>>>>>>> tristate "Marvell SATA support"
>>>>>>>> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
>>>>>>>> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
>>>>>>>> - select GENERIC_PHY
>>>>>>>> help
>>>>>>>> This option enables support for the Marvell Serial ATA family.
>>>>>>>> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
>>>>>>>> diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
>>>>>>>> index b7b2e47..b6f3eaa 100644
>>>>>>>> --- a/drivers/media/platform/exynos4-is/Kconfig
>>>>>>>> +++ b/drivers/media/platform/exynos4-is/Kconfig
>>>>>>>> @@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
>>>>>>>> config VIDEO_S5P_MIPI_CSIS
>>>>>>>> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
>>>>>>>> depends on REGULATOR
>>>>>>>> - select GENERIC_PHY
>>>>>>>> + depends on GENERIC_PHY
>>>>>>>> help
>>>>>>>> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
>>>>>>>> receiver (MIPI-CSIS) devices.
>>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>>>> index 2962de2..edecdb1 100644
>>>>>>>> --- a/drivers/phy/Kconfig
>>>>>>>> +++ b/drivers/phy/Kconfig
>>>>>>>> @@ -5,7 +5,7 @@
>>>>>>>> menu "PHY Subsystem"
>>>>>>>>
>>>>>>>> config GENERIC_PHY
>>>>>>>> - bool "PHY Core"
>>>>>>>> + bool
>>>>>>>> help
>>>>>>>> Generic PHY support.
>>>>>>>>
>>>>>>>> @@ -72,7 +72,7 @@ config PHY_MIPHY365X
>>>>>>>> config PHY_RCAR_GEN2
>>>>>>>> tristate "Renesas R-Car generation 2 USB PHY driver"
>>>>>>>> depends on ARCH_SHMOBILE
>>>>>>>> - depends on GENERIC_PHY
>>>>>>>> + select GENERIC_PHY
>>>>>>>
>>>>>>> so some you changed from depends to select...
>>>>>>>
>>>>>>>> help
>>>>>>>> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
>>>>>>>>
>>>>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>>>>> index 5ad60e4..e2197e2 100644
>>>>>>>> --- a/drivers/usb/host/Kconfig
>>>>>>>> +++ b/drivers/usb/host/Kconfig
>>>>>>>> @@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
>>>>>>>> config USB_EHCI_HCD_STI
>>>>>>>> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
>>>>>>>> depends on ARCH_STI && OF
>>>>>>>> - select GENERIC_PHY
>>>>>>>> + depends on GENERIC_PHY
>>>>>>>
>>>>>>> while others you changed from select to depends.
>>>>>>>
>>>>>>> NAK.
>>>>>>>
>>>>>> Felipe, I dont understand your concern, could you please explain it more
>>>>>> detail? The logic behind the changes is that in cases where there was an
>>>>>> explicit dependency, I changed it to "depends on" and in other cases I
>>>>>> changed it to "selects". Thanks
>>>>>
>>>>> Since GENERIC_PHY is visible from Kconfig, it would be much nicer to
>>>>> avoid select altogether.
>>>>>
>>>> Felipe, after discussion with the maintainers, I have made GENERIC_PHY an
>>>> invisible option as part of this change. Thanks
>>>
>>> Then, if the option is invisible, how can you "depend" on it ? It can
>>> never be selected by poking around in Kconfig. IMO, it's
>>> counterintuitive that you need to enable a PHY driver before you can see
>>> your EHCI/OHCI/whatever controller listed in Kconfig.
>>
>> If the controller requires PHY for it to be functional, it is okay to make
>> the controller depend on PHY IMHO. We want to try and minimize the usage of
>> 'select' wherever possible or else 'select' is the most intuitive way. The
>> other option is just to leave the 'depends on' and let the user select PHY.
>
> How can you 'depend' on something that the user can't select by
> navigating through Kconfig ?
hmm... Actually it's selected when the user selects the PHY driver. Maybe we
should directly depend on the PHY driver instead of Generic PHY?
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH v4 3/17] video: fbdev: gbefb: use arch_phys_wc_add() and devm_ioremap_wc()
From: Luis R. Rodriguez @ 2015-06-01 15:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: Tomi Valkeinen, Dave Airlie, linux-kernel@vger.kernel.org,
linux-fbdev, Thomas Gleixner, Juergen Gross, Jingoo Han,
Daniel Vetter, Masanari Iida, Suresh Siddha, Ingo Molnar,
Andy Lutomirski, Antonino Daplas,
Jean-Christophe Plagniol-Villard
In-Reply-To: <20150601085332.GA14549@gmail.com>
On Mon, Jun 1, 2015 at 1:53 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Luis R. Rodriguez <mcgrof@suse.com> wrote:
>
>> On Fri, May 29, 2015 at 08:41:44AM +0200, Ingo Molnar wrote:
>> >
>> > * Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote:
>> >
>> > > + gbe_mem = devm_ioremap_wc(&p_dev->dev, gbe_mem_phys,
>> > > + gbe_mem_size);
>> >
>> > > + gbe_mem = dma_alloc_writecombine(NULL, gbe_mem_size,
>> > > + &gbe_dma_addr, GFP_KERNEL);
>> >
>> > > + par->wc_cookie = arch_phys_wc_add(gbe_mem_phys, gbe_mem_size);
>> >
>> > > + arch_phys_wc_del(par->wc_cookie);
>> >
>> > > + arch_phys_wc_del(par->wc_cookie);
>> >
>> > Could we _please_ rename dma_*_writecombine() to dma_*_wc(), so that the naming is
>> > coherent across the various APIs?
>>
>> Sure, OK done. Will send once this series is merged and visible on linux-next.
>> But who's tree should this go through? Since it depends on this series which
>> goes through Tomi's tree should it go through there?
>
> So we could do the rename near the end of the merge window, when all the patches
> have drained from maintainer trees into Linus's tree.
OK.
> Mind keeping it in your tree until then?
You got it.
Luis
^ permalink raw reply
* Re: [PATCHv3 1/4] phy: phy-core: Make GENERIC_PHY an invisible option
From: Felipe Balbi @ 2015-06-01 18:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <556C5599.9020808@ti.com>
[-- Attachment #1: Type: text/plain, Size: 5984 bytes --]
On Mon, Jun 01, 2015 at 06:22:41PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Friday 29 May 2015 08:34 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Fri, May 29, 2015 at 05:04:38PM +0530, Kishon Vijay Abraham I wrote:
> >>Hi Felipe,
> >>
> >>On Wednesday 27 May 2015 12:09 AM, Felipe Balbi wrote:
> >>>On Tue, May 26, 2015 at 11:37:17AM -0700, Arun Ramamurthy wrote:
> >>>>Hi
> >>>>
> >>>>On 15-05-26 07:19 AM, Felipe Balbi wrote:
> >>>>>HI,
> >>>>>
> >>>>>On Mon, May 25, 2015 at 02:19:58PM -0700, Arun Ramamurthy wrote:
> >>>>>>
> >>>>>>
> >>>>>>On 15-05-14 05:52 PM, Felipe Balbi wrote:
> >>>>>>>Hi,
> >>>>>>>
> >>>>>>>On Wed, Apr 22, 2015 at 04:04:10PM -0700, Arun Ramamurthy wrote:
> >>>>>>>>Most of the phy providers use "select" to enable GENERIC_PHY. Since select
> >>>>>>>>is only recommended when the config is not visible, GENERIC_PHY is changed
> >>>>>>>>an invisible option. To maintain consistency, all phy providers are changed
> >>>>>>>>to "select" GENERIC_PHY and all non-phy drivers use "depends on" when the
> >>>>>>>>phy framework is explicity required. USB_MUSB_OMAP2PLUS has a cyclic
> >>>>>>>>dependency, so it is left as "select".
> >>>>>>>>
> >>>>>>>>Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
> >>>>>>>>---
> >>>>>>>> drivers/ata/Kconfig | 1 -
> >>>>>>>> drivers/media/platform/exynos4-is/Kconfig | 2 +-
> >>>>>>>> drivers/phy/Kconfig | 4 ++--
> >>>>>>>> drivers/usb/host/Kconfig | 4 ++--
> >>>>>>>> drivers/video/fbdev/exynos/Kconfig | 2 +-
> >>>>>>>> 5 files changed, 6 insertions(+), 7 deletions(-)
> >>>>>>>>
> >>>>>>>>diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> >>>>>>>>index 5f60155..6d2e881 100644
> >>>>>>>>--- a/drivers/ata/Kconfig
> >>>>>>>>+++ b/drivers/ata/Kconfig
> >>>>>>>>@@ -301,7 +301,6 @@ config SATA_MV
> >>>>>>>> tristate "Marvell SATA support"
> >>>>>>>> depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
> >>>>>>>> ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
> >>>>>>>>- select GENERIC_PHY
> >>>>>>>> help
> >>>>>>>> This option enables support for the Marvell Serial ATA family.
> >>>>>>>> Currently supports 88SX[56]0[48][01] PCI(-X) chips,
> >>>>>>>>diff --git a/drivers/media/platform/exynos4-is/Kconfig b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>>>index b7b2e47..b6f3eaa 100644
> >>>>>>>>--- a/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>>>+++ b/drivers/media/platform/exynos4-is/Kconfig
> >>>>>>>>@@ -31,7 +31,7 @@ config VIDEO_S5P_FIMC
> >>>>>>>> config VIDEO_S5P_MIPI_CSIS
> >>>>>>>> tristate "S5P/EXYNOS MIPI-CSI2 receiver (MIPI-CSIS) driver"
> >>>>>>>> depends on REGULATOR
> >>>>>>>>- select GENERIC_PHY
> >>>>>>>>+ depends on GENERIC_PHY
> >>>>>>>> help
> >>>>>>>> This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC MIPI-CSI2
> >>>>>>>> receiver (MIPI-CSIS) devices.
> >>>>>>>>diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> >>>>>>>>index 2962de2..edecdb1 100644
> >>>>>>>>--- a/drivers/phy/Kconfig
> >>>>>>>>+++ b/drivers/phy/Kconfig
> >>>>>>>>@@ -5,7 +5,7 @@
> >>>>>>>> menu "PHY Subsystem"
> >>>>>>>>
> >>>>>>>> config GENERIC_PHY
> >>>>>>>>- bool "PHY Core"
> >>>>>>>>+ bool
> >>>>>>>> help
> >>>>>>>> Generic PHY support.
> >>>>>>>>
> >>>>>>>>@@ -72,7 +72,7 @@ config PHY_MIPHY365X
> >>>>>>>> config PHY_RCAR_GEN2
> >>>>>>>> tristate "Renesas R-Car generation 2 USB PHY driver"
> >>>>>>>> depends on ARCH_SHMOBILE
> >>>>>>>>- depends on GENERIC_PHY
> >>>>>>>>+ select GENERIC_PHY
> >>>>>>>
> >>>>>>>so some you changed from depends to select...
> >>>>>>>
> >>>>>>>> help
> >>>>>>>> Support for USB PHY found on Renesas R-Car generation 2 SoCs.
> >>>>>>>>
> >>>>>>>>diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> >>>>>>>>index 5ad60e4..e2197e2 100644
> >>>>>>>>--- a/drivers/usb/host/Kconfig
> >>>>>>>>+++ b/drivers/usb/host/Kconfig
> >>>>>>>>@@ -182,7 +182,7 @@ config USB_EHCI_HCD_SPEAR
> >>>>>>>> config USB_EHCI_HCD_STI
> >>>>>>>> tristate "Support for ST STiHxxx on-chip EHCI USB controller"
> >>>>>>>> depends on ARCH_STI && OF
> >>>>>>>>- select GENERIC_PHY
> >>>>>>>>+ depends on GENERIC_PHY
> >>>>>>>
> >>>>>>>while others you changed from select to depends.
> >>>>>>>
> >>>>>>>NAK.
> >>>>>>>
> >>>>>>Felipe, I dont understand your concern, could you please explain it more
> >>>>>>detail? The logic behind the changes is that in cases where there was an
> >>>>>>explicit dependency, I changed it to "depends on" and in other cases I
> >>>>>>changed it to "selects". Thanks
> >>>>>
> >>>>>Since GENERIC_PHY is visible from Kconfig, it would be much nicer to
> >>>>>avoid select altogether.
> >>>>>
> >>>>Felipe, after discussion with the maintainers, I have made GENERIC_PHY an
> >>>>invisible option as part of this change. Thanks
> >>>
> >>>Then, if the option is invisible, how can you "depend" on it ? It can
> >>>never be selected by poking around in Kconfig. IMO, it's
> >>>counterintuitive that you need to enable a PHY driver before you can see
> >>>your EHCI/OHCI/whatever controller listed in Kconfig.
> >>
> >>If the controller requires PHY for it to be functional, it is okay to make
> >>the controller depend on PHY IMHO. We want to try and minimize the usage of
> >>'select' wherever possible or else 'select' is the most intuitive way. The
> >>other option is just to leave the 'depends on' and let the user select PHY.
> >
> >How can you 'depend' on something that the user can't select by
> >navigating through Kconfig ?
>
> hmm... Actually it's selected when the user selects the PHY driver.
that's my point, don't you think it's a little counter-intuitive ?
> Maybe we should directly depend on the PHY driver instead of Generic
> PHY?
maybe... But then what do you do when you have different boards using
different PHYs ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 02/12] Staging: sm750fb: sm750_help.h: Insert spaces after commas.
From: Sudip Mukherjee @ 2015-06-02 6:27 UTC (permalink / raw)
To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <1433134873-9869-3-git-send-email-isaac.a.travers@gmail.com>
On Sun, May 31, 2015 at 10:01:03PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the
> following checkpatch errors in sm750_help.h:
> ERROR: space required after that ','
>
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
> drivers/staging/sm750fb/sm750_help.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750_help.h b/drivers/staging/sm750fb/sm750_help.h
> index e0128d2..3940359 100644
> --- a/drivers/staging/sm750fb/sm750_help.h
> +++ b/drivers/staging/sm750fb/sm750_help.h
> @@ -10,11 +10,11 @@
>
> #define RAW_MASK(f) (0xFFFFFFFF >> (32 - _COUNT(f)))
> #define GET_MASK(f) (RAW_MASK(f) << _LSB(f))
> -#define GET_FIELD(d,f) (((d) >> _LSB(f)) & RAW_MASK(f))
> -#define TEST_FIELD(d,f,v) (GET_FIELD(d,f) = f ## _ ## v)
> -#define SET_FIELD(d,f,v) (((d) & ~GET_MASK(f)) | \
> +#define GET_FIELD(d, f) (((d) >> _LSB(f)) & RAW_MASK(f))
> +#define TEST_FIELD(d, f, v) (GET_FIELD(d,f) = f ## _ ## v)
^^^
i guess you missed this one.
regards
sudip
^ permalink raw reply
* Re: [PATCH 03/12] Staging: sm750fb: sm750.h: Insert spaces after commas.
From: Sudip Mukherjee @ 2015-06-02 6:37 UTC (permalink / raw)
To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <1433134873-9869-4-git-send-email-isaac.a.travers@gmail.com>
On Sun, May 31, 2015 at 10:01:04PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the
> following checkpatch errors in sm750.h:
> ERROR: space required after that ','
>
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
> drivers/staging/sm750fb/sm750.h | 35 ++++++++++++++++++-----------------
> 1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/sm750.h b/drivers/staging/sm750fb/sm750.h
> index 5528912..5f86077 100644
> --- a/drivers/staging/sm750fb/sm750.h
> +++ b/drivers/staging/sm750fb/sm750.h
> @@ -12,7 +12,7 @@
> #define MB(x) ((x)<<20)
> #define MHZ(x) ((x) * 1000000)
> /* align should be 2,4,8,16 */
> -#define PADDING(align,data) (((data)+(align)-1)&(~((align) -1)))
> +#define PADDING(align, data) (((data)+(align)-1)&(~((align) -1)))
> extern int smi_indent;
>
>
> @@ -27,15 +27,16 @@ struct lynx_accel{
>
>
> - int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> - u32,u32,u32,u32,u32,u32,u32,u32,u32);
> + int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
^^^^^
oops, trailing whitespace errors :(
> + u32, u32, u32, u32,
^^^^^
here also
please test your patch with checkpatch before sending
regards
sudip
^ permalink raw reply
* Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.
From: Sudip Mukherjee @ 2015-06-02 6:48 UTC (permalink / raw)
To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <1433134873-9869-13-git-send-email-isaac.a.travers@gmail.com>
On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the following checkpatch
> errors in ddk750_mode.c, sm750_accel.c and sm750_help.h:
> ERROR: space required after that ','
>
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
> drivers/staging/sm750fb/ddk750_mode.c | 46 +++++++++++++++++------------------
> drivers/staging/sm750fb/sm750_accel.c | 2 +-
> drivers/staging/sm750fb/sm750_help.h | 2 +-
> 3 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
> index 4e252fb..a054747 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -101,29 +101,29 @@ static int programModeRegisters(mode_parameter_t * pModeParam, pll_value_t * pll
> | FIELD_VALUE(0, CRT_VERTICAL_SYNC, START, pModeParam->vertical_sync_start - 1));
>
>
> - ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> + ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL, VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> - FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
> - FIELD_SET(0,CRT_DISPLAY_CTRL,PLANE,ENABLE);
> + FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> + FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
I am getting confused now.
This part you have modified in patch 10/12 and i just replied that
you have missed two modifications.
regards
sudip
^ permalink raw reply
* Re: [PATCH 10/12] Staging: sm750fb: Insert spaces after commas in two files.
From: Sudip Mukherjee @ 2015-06-02 6:53 UTC (permalink / raw)
To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <1433134873-9869-11-git-send-email-isaac.a.travers@gmail.com>
On Sun, May 31, 2015 at 10:01:11PM -0700, Isaac Assegai wrote:
> Insert Spaces after commas to rectify the following
> checkpatch errors in ddk750_help.c and ddk750_mode.c:
> ERROR: space required after that ','
>
> Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> ---
>
<snip>
>
> - ulTmpValue = FIELD_VALUE(0,CRT_DISPLAY_CTRL,VSYNC_PHASE,pModeParam->vertical_sync_polarity)|
> - FIELD_VALUE(0,CRT_DISPLAY_CTRL,HSYNC_PHASE,pModeParam->horizontal_sync_polarity)|
> - FIELD_SET(0,CRT_DISPLAY_CTRL,TIMING,ENABLE)|
> + ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
^^^^^^^^
missed this
> + FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> + FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
^^^^^
this also
regards
sudip
^ permalink raw reply
* Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.
From: Isaac Assegai @ 2015-06-02 7:55 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150602064806.GF6382@sudip-PC>
On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> > Insert Spaces after commas to rectify the following checkpatch
> > errors in ddk750_mode.c, sm750_accel.c and sm750_help.h:
> > ERROR: space required after that ','
> >
> > Signed-off-by: Isaac Assegai <isaac.a.travers@gmail.com>
> > ---
> > drivers/staging/sm750fb/ddk750_mode.c | 46 +++++++++++++++++------------------
> > drivers/staging/sm750fb/sm750_accel.c | 2 +-
> > drivers/staging/sm750fb/sm750_help.h | 2 +-
> > 3 files changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
> > index 4e252fb..a054747 100644
> > --- a/drivers/staging/sm750fb/ddk750_mode.c
> > +++ b/drivers/staging/sm750fb/ddk750_mode.c
> > @@ -101,29 +101,29 @@ static int programModeRegisters(mode_parameter_t * pModeParam, pll_value_t * pll
> > | FIELD_VALUE(0, CRT_VERTICAL_SYNC, START, pModeParam->vertical_sync_start - 1));
> >
> >
> > - ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL,VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> > + ulTmpValue = FIELD_VALUE(0, CRT_DISPLAY_CTRL, VSYNC_PHASE, pModeParam->vertical_sync_polarity)|
> > FIELD_VALUE(0, CRT_DISPLAY_CTRL, HSYNC_PHASE, pModeParam->horizontal_sync_polarity)|
> > - FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING,ENABLE)|
> > - FIELD_SET(0,CRT_DISPLAY_CTRL,PLANE,ENABLE);
> > + FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > + FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> I am getting confused now.
> This part you have modified in patch 10/12 and i just replied that
> you have missed two modifications.
>
> regards
> sudip
Thanks for your feedback Sudip, I have a few questions:
1. After applying the first 11 patches I ran checkpatch again and noticed that
I missed three warnings so I ran a 12th patch fixing them. That's why you
see the 12th patch modifying a file that another patch already touched.
What should be done in this type of situation?
2. On Patch 3 you pointed out two trailing whitespace errors.
I ran checkpatch on all the patches before I sent them to
make sure I didn't introduce any new errors, however
there are *many* warnings and errors showing up from
the poor styling already present and I missed these
whitespace errors in the mess.
Can I make checkpatch suppress any errors that might have already
been present in the code and only show me those issues introduced
by the patch itself? If not, how did you identify it?
Thanks,
Isaac
^ permalink raw reply
* Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.
From: Isaac Assegai @ 2015-06-02 8:30 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150602082555.GA4436@sudip-PC>
On Tue, Jun 02, 2015 at 01:55:56PM +0530, Sudip Mukherjee wrote:
> On Tue, Jun 02, 2015 at 12:55:22AM -0700, Isaac Assegai wrote:
> > On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> > > On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
> <snip>
> > > > + FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > > > + FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> > > I am getting confused now.
> > > This part you have modified in patch 10/12 and i just replied that
> > > you have missed two modifications.
> > >
> > Thanks for your feedback Sudip, I have a few questions:
> > 1. After applying the first 11 patches I ran checkpatch again and noticed that
> > I missed three warnings so I ran a 12th patch fixing them. That's why you
> > see the 12th patch modifying a file that another patch already touched.
> > What should be done in this type of situation?
> you have to start from the beginning again. :(
> I have faced such similar situation also, but starting for the beginnig
> is the only way.
> >
> > 2. On Patch 3 you pointed out two trailing whitespace errors.
> > I ran checkpatch on all the patches before I sent them to
> > make sure I didn't introduce any new errors,
> but it looks like they were introduced in your patch.
> > - int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> > - u32,u32,u32,u32,u32,u32,u32,u32,u32);
> > + int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
> the original code was having three u32, i think when you moved one more
> u32 from the following line you forgot to remove the space after that.
>
> >however
> > there are *many* warnings and errors showing up from
> > the poor styling already present and I missed these
> > whitespace errors in the mess.
> > Can I make checkpatch suppress any errors that might have already
> > been present in the code and only show me those issues introduced
> > by the patch itself? If not, how did you identify it?
> i dont think checkpatch can work like that way.
> identification was easy. i saw checkpatch giving error and noticed you
> have modified the code. So i checked the original code if trailing
> whitespace was there or not.
>
> regards
> sudip
Okay,
I'll re-run this and send a new version in the morning.
Thanks,
Isaac
^ permalink raw reply
* Re: [PATCH 12/12] Staging: sm750fb: Insert spaces after commas in three files.
From: Sudip Mukherjee @ 2015-06-02 8:37 UTC (permalink / raw)
To: Isaac Assegai; +Cc: gregkh, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <20150602075522.GA10942@ubuntu-desktop>
On Tue, Jun 02, 2015 at 12:55:22AM -0700, Isaac Assegai wrote:
> On Tue, Jun 02, 2015 at 12:18:06PM +0530, Sudip Mukherjee wrote:
> > On Sun, May 31, 2015 at 10:01:13PM -0700, Isaac Assegai wrote:
<snip>
> > > + FIELD_SET(0, CRT_DISPLAY_CTRL, TIMING, ENABLE)|
> > > + FIELD_SET(0, CRT_DISPLAY_CTRL, PLANE, ENABLE);
> > I am getting confused now.
> > This part you have modified in patch 10/12 and i just replied that
> > you have missed two modifications.
> >
> Thanks for your feedback Sudip, I have a few questions:
> 1. After applying the first 11 patches I ran checkpatch again and noticed that
> I missed three warnings so I ran a 12th patch fixing them. That's why you
> see the 12th patch modifying a file that another patch already touched.
> What should be done in this type of situation?
you have to start from the beginning again. :(
I have faced such similar situation also, but starting for the beginnig
is the only way.
>
> 2. On Patch 3 you pointed out two trailing whitespace errors.
> I ran checkpatch on all the patches before I sent them to
> make sure I didn't introduce any new errors,
but it looks like they were introduced in your patch.
> - int (*de_imageblit)(struct lynx_accel *,const char *,u32,u32,u32,
> - u32,u32,u32,u32,u32,u32,u32,u32,u32);
> + int (*de_imageblit)(struct lynx_accel *, const char *, u32, u32, u32, u32,
the original code was having three u32, i think when you moved one more
u32 from the following line you forgot to remove the space after that.
>however
> there are *many* warnings and errors showing up from
> the poor styling already present and I missed these
> whitespace errors in the mess.
> Can I make checkpatch suppress any errors that might have already
> been present in the code and only show me those issues introduced
> by the patch itself? If not, how did you identify it?
i dont think checkpatch can work like that way.
identification was easy. i saw checkpatch giving error and noticed you
have modified the code. So i checked the original code if trailing
whitespace was there or not.
regards
sudip
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox