2017-12-01 07:54:54

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

Carlos,

On Thu, Nov 30, 2017 at 11:53 PM, James Hogan <[email protected]> wrote:
> On Tue, Nov 28, 2017 at 04:55:35PM -0800, David Daney wrote:
>> From: Carlos Munoz <[email protected]>
>>
>> Add a global resource manager to manage tagged pointers within
>> bootmem allocated memory. This is used by various functional
>> blocks in the Octeon core like the FPA, Ethernet nexus, etc.
>>
>> Signed-off-by: Carlos Munoz <[email protected]>
>> Signed-off-by: Steven J. Hill <[email protected]>
>> Signed-off-by: David Daney <[email protected]>
>> ---
>> arch/mips/cavium-octeon/Makefile | 3 +-
>> arch/mips/cavium-octeon/resource-mgr.c | 371 +++++++++++++++++++++++++++++++++
>> arch/mips/include/asm/octeon/octeon.h | 18 ++
>> 3 files changed, 391 insertions(+), 1 deletion(-)
>> create mode 100644 arch/mips/cavium-octeon/resource-mgr.c
>>
>> diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
>> index 7c02e542959a..0a299ab8719f 100644
>> --- a/arch/mips/cavium-octeon/Makefile
>> +++ b/arch/mips/cavium-octeon/Makefile
>> @@ -9,7 +9,8 @@
>> # Copyright (C) 2005-2009 Cavium Networks
>> #
>>
>> -obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
>> +obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \
>> + resource-mgr.o
>
> Maybe put that on a separate line like below.
>
>> obj-y += dma-octeon.o
>> obj-y += octeon-memcpy.o
>> obj-y += executive/
>> diff --git a/arch/mips/cavium-octeon/resource-mgr.c b/arch/mips/cavium-octeon/resource-mgr.c
>> new file mode 100644
>> index 000000000000..ca25fa953402
>> --- /dev/null
>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>> @@ -0,0 +1,371 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Resource manager for Octeon.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.
>> + *
>> + * Copyright (C) 2017 Cavium, Inc.
>> + */

Since you nicely included an SPDX id, you would not need the
boilerplate anymore. e.g. these can go alright?

>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.

--
Cordially
Philippe Ombredanne


2017-12-01 17:42:58

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
> Carlos,
>
> On Thu, Nov 30, 2017 at 11:53 PM, James Hogan <[email protected]> wrote:
>> On Tue, Nov 28, 2017 at 04:55:35PM -0800, David Daney wrote:
>>> From: Carlos Munoz <[email protected]>
>>>
>>> Add a global resource manager to manage tagged pointers within
>>> bootmem allocated memory. This is used by various functional
>>> blocks in the Octeon core like the FPA, Ethernet nexus, etc.
>>>
>>> Signed-off-by: Carlos Munoz <[email protected]>
>>> Signed-off-by: Steven J. Hill <[email protected]>
>>> Signed-off-by: David Daney <[email protected]>
>>> ---
>>> arch/mips/cavium-octeon/Makefile | 3 +-
>>> arch/mips/cavium-octeon/resource-mgr.c | 371 +++++++++++++++++++++++++++++++++
>>> arch/mips/include/asm/octeon/octeon.h | 18 ++
>>> 3 files changed, 391 insertions(+), 1 deletion(-)
>>> create mode 100644 arch/mips/cavium-octeon/resource-mgr.c
>>>
>>> diff --git a/arch/mips/cavium-octeon/Makefile b/arch/mips/cavium-octeon/Makefile
>>> index 7c02e542959a..0a299ab8719f 100644
>>> --- a/arch/mips/cavium-octeon/Makefile
>>> +++ b/arch/mips/cavium-octeon/Makefile
>>> @@ -9,7 +9,8 @@
>>> # Copyright (C) 2005-2009 Cavium Networks
>>> #
>>>
>>> -obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o
>>> +obj-y := cpu.o setup.o octeon-platform.o octeon-irq.o csrc-octeon.o \
>>> + resource-mgr.o
>>
>> Maybe put that on a separate line like below.
>>
>>> obj-y += dma-octeon.o
>>> obj-y += octeon-memcpy.o
>>> obj-y += executive/
>>> diff --git a/arch/mips/cavium-octeon/resource-mgr.c b/arch/mips/cavium-octeon/resource-mgr.c
>>> new file mode 100644
>>> index 000000000000..ca25fa953402
>>> --- /dev/null
>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>> @@ -0,0 +1,371 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Resource manager for Octeon.
>>> + *
>>> + * This file is subject to the terms and conditions of the GNU General Public
>>> + * License. See the file "COPYING" in the main directory of this archive
>>> + * for more details.
>>> + *
>>> + * Copyright (C) 2017 Cavium, Inc.
>>> + */
>
> Since you nicely included an SPDX id, you would not need the
> boilerplate anymore. e.g. these can go alright?

They may not be strictly speaking necessary, but I don't think they hurt
anything. Unless there is a requirement to strip out the license text,
we would stick with it as is.

>
>>> + * This file is subject to the terms and conditions of the GNU General Public
>>> + * License. See the file "COPYING" in the main directory of this archive
>>> + * for more details.
>

2017-12-01 19:50:15

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

David, Greg,

On Fri, Dec 1, 2017 at 6:42 PM, David Daney <[email protected]> wrote:
> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
[...]
>>>> --- /dev/null
>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>>> @@ -0,0 +1,371 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Resource manager for Octeon.
>>>> + *
>>>> + * This file is subject to the terms and conditions of the GNU General
>>>> Public
>>>> + * License. See the file "COPYING" in the main directory of this
>>>> archive
>>>> + * for more details.
>>>> + *
>>>> + * Copyright (C) 2017 Cavium, Inc.
>>>> + */
>>
>>
>> Since you nicely included an SPDX id, you would not need the
>> boilerplate anymore. e.g. these can go alright?
>
>
> They may not be strictly speaking necessary, but I don't think they hurt
> anything. Unless there is a requirement to strip out the license text, we
> would stick with it as is.

I think the requirement is there and that would be much better for
everyone: keeping both is redundant and does not bring any value, does
it? Instead it kinda removes the benefits of having the SPDX id in the
first place IMHO.

Furthermore, as there have been already ~12K+ files cleaned up and
still over 60K files to go, it would really nice if new files could
adopt the new style: this way we will not have to revisit and repatch
them in the future.

--
Cordially
Philippe Ombredanne

2017-12-01 20:01:14

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

On 12/01/2017 11:49 AM, Philippe Ombredanne wrote:
> David, Greg,
>
> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <[email protected]> wrote:
>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
> [...]
>>>>> --- /dev/null
>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>>>> @@ -0,0 +1,371 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * Resource manager for Octeon.
>>>>> + *
>>>>> + * This file is subject to the terms and conditions of the GNU General
>>>>> Public
>>>>> + * License. See the file "COPYING" in the main directory of this
>>>>> archive
>>>>> + * for more details.
>>>>> + *
>>>>> + * Copyright (C) 2017 Cavium, Inc.
>>>>> + */
>>>
>>>
>>> Since you nicely included an SPDX id, you would not need the
>>> boilerplate anymore. e.g. these can go alright?
>>
>>
>> They may not be strictly speaking necessary, but I don't think they hurt
>> anything. Unless there is a requirement to strip out the license text, we
>> would stick with it as is.
>
> I think the requirement is there and that would be much better for
> everyone: keeping both is redundant and does not bring any value, does
> it? Instead it kinda removes the benefits of having the SPDX id in the
> first place IMHO.
>
> Furthermore, as there have been already ~12K+ files cleaned up and
> still over 60K files to go, it would really nice if new files could
> adopt the new style: this way we will not have to revisit and repatch
> them in the future.
>

I am happy to follow any style Greg would suggest. There doesn't seem
to be much documentation about how this should be done yet.

David Daney

2017-12-01 20:41:50

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

David,

On Fri, Dec 1, 2017 at 9:01 PM, David Daney <[email protected]> wrote:
> On 12/01/2017 11:49 AM, Philippe Ombredanne wrote:
>>
>> David, Greg,
>>
>> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <[email protected]>
>> wrote:
>>>
>>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
>>
>> [...]
>>>>>>
>>>>>> --- /dev/null
>>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>>>>> @@ -0,0 +1,371 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>> +/*
>>>>>> + * Resource manager for Octeon.
>>>>>> + *
>>>>>> + * This file is subject to the terms and conditions of the GNU
>>>>>> General
>>>>>> Public
>>>>>> + * License. See the file "COPYING" in the main directory of this
>>>>>> archive
>>>>>> + * for more details.
>>>>>> + *
>>>>>> + * Copyright (C) 2017 Cavium, Inc.
>>>>>> + */
>>>>
>>>>
>>>>
>>>> Since you nicely included an SPDX id, you would not need the
>>>> boilerplate anymore. e.g. these can go alright?
>>>
>>>
>>>
>>> They may not be strictly speaking necessary, but I don't think they hurt
>>> anything. Unless there is a requirement to strip out the license text,
>>> we
>>> would stick with it as is.
>>
>>
>> I think the requirement is there and that would be much better for
>> everyone: keeping both is redundant and does not bring any value, does
>> it? Instead it kinda removes the benefits of having the SPDX id in the
>> first place IMHO.
>>
>> Furthermore, as there have been already ~12K+ files cleaned up and
>> still over 60K files to go, it would really nice if new files could
>> adopt the new style: this way we will not have to revisit and repatch
>> them in the future.
>>
>
> I am happy to follow any style Greg would suggest. There doesn't seem to be
> much documentation about how this should be done yet.

Thomas (tglx) has already submitted a first series of doc patches a
few weeks ago. And AFAIK he might be working on posting the updates
soon, whenever his real time clock yields a few cycles away from real
time coding work ;)

See also these discussions with Linus [1][2][3], Thomas[4] and Greg[5]
on this and mostly related topics

[1] https://lkml.org/lkml/2017/11/2/715
[2] https://lkml.org/lkml/2017/11/25/125
[3] https://lkml.org/lkml/2017/11/25/133
[4] https://lkml.org/lkml/2017/11/2/805
[5] https://lkml.org/lkml/2017/10/19/165

--
Cordially
Philippe Ombredanne

2017-12-01 20:56:45

by David Daney

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

On 12/01/2017 12:41 PM, Philippe Ombredanne wrote:
> David,
>
> On Fri, Dec 1, 2017 at 9:01 PM, David Daney <[email protected]> wrote:
>> On 12/01/2017 11:49 AM, Philippe Ombredanne wrote:
>>>
>>> David, Greg,
>>>
>>> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <[email protected]>
>>> wrote:
>>>>
>>>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
>>>
>>> [...]
>>>>>>>
>>>>>>> --- /dev/null
>>>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>>>>>> @@ -0,0 +1,371 @@
>>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>>> +/*
>>>>>>> + * Resource manager for Octeon.
>>>>>>> + *
>>>>>>> + * This file is subject to the terms and conditions of the GNU
>>>>>>> General
>>>>>>> Public
>>>>>>> + * License. See the file "COPYING" in the main directory of this
>>>>>>> archive
>>>>>>> + * for more details.
>>>>>>> + *
>>>>>>> + * Copyright (C) 2017 Cavium, Inc.
>>>>>>> + */
>>>>>
>>>>>
>>>>>
>>>>> Since you nicely included an SPDX id, you would not need the
>>>>> boilerplate anymore. e.g. these can go alright?
>>>>
>>>>
>>>>
>>>> They may not be strictly speaking necessary, but I don't think they hurt
>>>> anything. Unless there is a requirement to strip out the license text,
>>>> we
>>>> would stick with it as is.
>>>
>>>
>>> I think the requirement is there and that would be much better for
>>> everyone: keeping both is redundant and does not bring any value, does
>>> it? Instead it kinda removes the benefits of having the SPDX id in the
>>> first place IMHO.
>>>
>>> Furthermore, as there have been already ~12K+ files cleaned up and
>>> still over 60K files to go, it would really nice if new files could
>>> adopt the new style: this way we will not have to revisit and repatch
>>> them in the future.
>>>
>>
>> I am happy to follow any style Greg would suggest. There doesn't seem to be
>> much documentation about how this should be done yet.
>
> Thomas (tglx) has already submitted a first series of doc patches a
> few weeks ago. And AFAIK he might be working on posting the updates
> soon, whenever his real time clock yields a few cycles away from real
> time coding work ;)
>
> See also these discussions with Linus [1][2][3], Thomas[4] and Greg[5]
> on this and mostly related topics
>
> [1] https://lkml.org/lkml/2017/11/2/715
> [2] https://lkml.org/lkml/2017/11/25/125
> [3] https://lkml.org/lkml/2017/11/25/133
> [4] https://lkml.org/lkml/2017/11/2/805
> [5] https://lkml.org/lkml/2017/10/19/165
>

OK, you convinced me.

Thanks,
David

2017-12-01 23:34:15

by Philippe Ombredanne

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] MIPS: Octeon: Add a global resource manager.

On Fri, Dec 1, 2017 at 9:56 PM, David Daney <[email protected]> wrote:
> On 12/01/2017 12:41 PM, Philippe Ombredanne wrote:
>>
>> David,
>>
>> On Fri, Dec 1, 2017 at 9:01 PM, David Daney <[email protected]>
>> wrote:
>>>
>>> On 12/01/2017 11:49 AM, Philippe Ombredanne wrote:
>>>>
>>>>
>>>> David, Greg,
>>>>
>>>> On Fri, Dec 1, 2017 at 6:42 PM, David Daney <[email protected]>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 11/30/2017 11:53 PM, Philippe Ombredanne wrote:
>>>>
>>>>
>>>> [...]
>>>>>>>>
>>>>>>>>
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/arch/mips/cavium-octeon/resource-mgr.c
>>>>>>>> @@ -0,0 +1,371 @@
>>>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>>>> +/*
>>>>>>>> + * Resource manager for Octeon.
>>>>>>>> + *
>>>>>>>> + * This file is subject to the terms and conditions of the GNU
>>>>>>>> General
>>>>>>>> Public
>>>>>>>> + * License. See the file "COPYING" in the main directory of this
>>>>>>>> archive
>>>>>>>> + * for more details.
>>>>>>>> + *
>>>>>>>> + * Copyright (C) 2017 Cavium, Inc.
>>>>>>>> + */
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Since you nicely included an SPDX id, you would not need the
>>>>>> boilerplate anymore. e.g. these can go alright?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> They may not be strictly speaking necessary, but I don't think they
>>>>> hurt
>>>>> anything. Unless there is a requirement to strip out the license text,
>>>>> we
>>>>> would stick with it as is.
>>>>
>>>>
>>>>
>>>> I think the requirement is there and that would be much better for
>>>> everyone: keeping both is redundant and does not bring any value, does
>>>> it? Instead it kinda removes the benefits of having the SPDX id in the
>>>> first place IMHO.
>>>>
>>>> Furthermore, as there have been already ~12K+ files cleaned up and
>>>> still over 60K files to go, it would really nice if new files could
>>>> adopt the new style: this way we will not have to revisit and repatch
>>>> them in the future.
>>>>
>>>
>>> I am happy to follow any style Greg would suggest. There doesn't seem to
>>> be
>>> much documentation about how this should be done yet.
>>
>>
>> Thomas (tglx) has already submitted a first series of doc patches a
>> few weeks ago. And AFAIK he might be working on posting the updates
>> soon, whenever his real time clock yields a few cycles away from real
>> time coding work ;)
>>
>> See also these discussions with Linus [1][2][3], Thomas[4] and Greg[5]
>> on this and mostly related topics
>>
>> [1] https://lkml.org/lkml/2017/11/2/715
>> [2] https://lkml.org/lkml/2017/11/25/125
>> [3] https://lkml.org/lkml/2017/11/25/133
>> [4] https://lkml.org/lkml/2017/11/2/805
>> [5] https://lkml.org/lkml/2017/10/19/165
>>
>
> OK, you convinced me.
>
> Thanks,
> David
>

No! Thank you to you: For doing real work on the kernel that makes my
servers and laptops run, while I am nitpicking you on comments.

--
Cordially
Philippe Ombredanne