On 3/2/2020 8:27 AM, Jiri Pirko wrote:
> Sat, Feb 15, 2020 at 12:21:59AM CET, jacob.e.keller@intel.com wrote:
>> This is a second revision of the previous RFC series I sent to enable two
>> new devlink region features.
>>
>> The original series can be viewed on the list archives at
>>
>> https://lore.kernel.org/netdev/20200130225913.1671982-1-jacob.e.keller@intel.com/
>>
>> Overall, this series can be broken into 5 phases:
>>
>> 1) implement basic devlink support in the ice driver, including .info_get
>> 2) convert regions to use the new devlink_region_ops structure
>> 3) implement support for DEVLINK_CMD_REGION_NEW
>> 4) implement support for directly reading from a region
>> 5) use these new features in the ice driver for the Shadow RAM region
> 
> Hmm. I think it is better to push this in multiple patchsets. For example,
> for 1) you don't really need RFC as it is only related to the ice driver
> implementing the existing API.
> 

Yes that's my plan for the next revision. I'm working on getting the ice
support ready to submit through IWL now. The other parts I will break
into 2 series.

> 
>>
>> (1) comprises 6 patches for the ice driver that add the devlink framework
>> and cleanup a few places in the code in preparation for the new region.
>>
>> (2) comprises 2 patches which convert regions to use the new
>> devlink_region_ops structure, and additionally move the snapshot destructor
>> to a region operation.
>>
>> (3) comprises 6 patches to enable supporting the DEVLINK_CMD_REGION_NEW
>> operation. This replaces what was previously the
>> DEVLINK_CMD_REGION_TAKE_SNAPSHOT, as per Jiri's suggestion. The new
>> operation supports specifying the requested id for the snapshot. To make
>> that possible, first snapshot id management is refactored to use an IDR.
>> Note that the extra complexity of the IDR is necessary in order to maintain
>> the ability for the snapshot IDs to be generated so that multiple regions
>> can use the same ID if triggered at the same time.
>>
>> (4) comprises 6 patches for modifying DEVLINK_CMD_REGION_READ so that it
>> accepts a request without a snapshot id. A new region operation is defined
>> for regions to optionally support the requests. The first few patches
>> refactor and simplify the functions used so that adding the new read method
>> reuses logic where possible.
>>
>> (5) finally comprises a single patch to implement a region for the ice
>> device hardware's Shadow RAM contents.
>>
>> Note that I plan to submit the ice patches through the Intel Wired LAN list,
>> but am sending the complete set here as an RFC in case there is further
>> feedback, and so that reviewers can have the correct context.
>>
>> I expect to get further feedback this RFC revision, and will hopefully send
>> the patches as non-RFC following this, if feedback looks good. Thank you for
>> the diligent review.
>>
>> Changes since v1:
> 
> Per-patch please. This is no good for review :/
> 

I've attached the git range-diff between the v1 and v2 series. I'll keep
in mind for future revision logs.

Thanks,
Jake