Re: [PATCH v1 1/1] misc: bcm-vk: add kconfig for supporting viper chip

[Scott Branden <scott.branden@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]



On 2022-02-23 10:27, Greg Kroah-Hartman wrote:
> On Wed, Feb 23, 2022 at 09:59:48AM -0800, Scott Branden wrote:
>> Hi Greg,
>>
>> On 2022-02-22 23:26, Greg Kroah-Hartman wrote:
>>> On Tue, Feb 22, 2022 at 09:14:02PM -0800, Desmond yan wrote:
>>>>
>>>> Greg,
>>>>
>>>> Please see inlined.
>>>>
>>>> On 2/22/22 09:13, Greg Kroah-Hartman wrote:
>>>>> A: http://en.wikipedia.org/wiki/Top_post
>>>>> Q: Were do I find info about this thing called top-posting?
>>>>> A: Because it messes up the order in which people normally read text.
>>>>> Q: Why is top-posting such a bad thing?
>>>>> A: Top-posting.
>>>>> Q: What is the most annoying thing in e-mail?
>>>>>
>>>>> A: No.
>>>>> Q: Should I include quotations after my reply?
>>>>>
>>>>>
>>>>> http://daringfireball.net/2007/07/on_top
>>>>>
>>>>> On Tue, Feb 22, 2022 at 07:41:04AM -0800, Desmond Yan (APD) wrote:
>>>>>> Greg,
>>>>>>
>>>>>> This is to allow the customer to use their own driver, by default,
>>>>>> which is their preferred option.
>>>>>
>>>>> We do not care about out-of-tree code, and do not add Kconfig options
>>>>> just for that, sorry.
>>>>>
>>>>> Why not just add the out-of-tree driver here to the in-kernel code?
>>>>> What is preventing that from happening today?
>>>>
>>>> Using the bcm_vk driver for viper is only for proof-of-concept, and is not
>>>> aimed to be deployed for production.  In parallel, customer is developing
>>>> their own production-ready driver and would probably upstream it whenever
>>>> the appropriate time comes.
>>>
>>> Ok, so let us not add a new config option only for one person working on
>>> writing their own driver.  Let's just add the device id as there are
>>> more people than just that one person wanting to use their hardware with
>>> the existing drivers.
>> The bcm_vk driver is the official driver for Valkyrie.  For Viper, the
>> bcm_vk driver is only for proof of concept, testing, and demonstration
>> purposes. For production, the new driver in development will be used.
>>
>> To support such configuration, we need to have the Viper pci id configurable
>> and off by default in the bcm_vk driver to allow the new production driver
>> to be developed against the upstream kernel and drivers.
>>>
>>> Please resubmit this without the Kconfig change and I will be glad to
>>> take it.
>> I don't understand what there would be to submit?  The Viper pci id is
>> already upstreamed into the bcm_vk driver.  The sole purpose of this patch
>> is to add the Kconfig option and have it off by default.
> 
> Ah, I missed that it was disabling the id.  That's even worse.  No,
> sorry, not going to take this upstream.  Again, we do not add changes to
> the kernel (and especially not Kconfig options) for out-of-tree code.
Well, will you accept a patch to remove the Viper PCI id from upstream 
instead then?  bcm_vk driver should not support Viper PCI id in official 
systems.
> 
> You know this...
> 
> thanks,
> 
> greg k-h

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]