On Wed, Dec 20, 2017 at 4:29 PM, Alan Tull <atull@xxxxxxxxxx> wrote:
> On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao <hao.wu@xxxxxxxxx> wrote:
>
> Hi Hao,
>
>> +
>> +enum port_feature_id {
>> + PORT_FEATURE_ID_HEADER = 0x0,
>> + PORT_FEATURE_ID_ERROR = 0x1,
>> + PORT_FEATURE_ID_UMSG = 0x2,
>> + PORT_FEATURE_ID_PR = 0x3,
>> + PORT_FEATURE_ID_STP = 0x4,
>> + PORT_FEATURE_ID_UAFU = 0x5,
>> + PORT_FEATURE_ID_MAX = 0x6,
>> +};
>> +
>> +#define FME_FEATURE_NUM FME_FEATURE_ID_MAX
>> +#define PORT_FEATURE_NUM PORT_FEATURE_ID_MAX
>> +
>> +#define FPGA_FEATURE_DEV_FME "fpga-dfl-fme"
>> +#define FPGA_FEATURE_DEV_PORT "fpga-dfl-port"
>> +
>> +static inline int feature_platform_data_size(const int num)
>> +{
>> + return sizeof(struct feature_platform_data) +
>> + num * sizeof(struct feature);
>> +}
>> +
>> +int fpga_port_id(struct platform_device *pdev);
>> +
>> +static inline int fpga_port_check_id(struct platform_device *pdev,
>> + void *pport_id)
>> +{
>> + return fpga_port_id(pdev) == *(int *)pport_id;
>> +}
>> +
>> +void __fpga_port_enable(struct platform_device *pdev);
>> +int __fpga_port_disable(struct platform_device *pdev);
>> +
>> +static inline void fpga_port_enable(struct platform_device *pdev)
>> +{
>> + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> +
>> + mutex_lock(&pdata->lock);
>> + __fpga_port_enable(pdev);
>> + mutex_unlock(&pdata->lock);
>> +}
>> +
>> +static inline int fpga_port_disable(struct platform_device *pdev)
>> +{
>> + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> + int ret;
>> +
>> + mutex_lock(&pdata->lock);
>> + ret = __fpga_port_disable(pdev);
>> + mutex_unlock(&pdata->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static inline int __fpga_port_reset(struct platform_device *pdev)
>> +{
>> + int ret;
>> +
>> + ret = __fpga_port_disable(pdev);
>> + if (ret)
>> + return ret;
>> +
>> + __fpga_port_enable(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static inline int fpga_port_reset(struct platform_device *pdev)
>> +{
>> + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> + int ret;
>> +
>> + mutex_lock(&pdata->lock);
>> + ret = __fpga_port_reset(pdev);
>> + mutex_unlock(&pdata->lock);
>> +
>> + return ret;
>> +}
>
> I see that the port code is included as part of the enumeration code.
> This is not very future-proofed, if a different port needs to be
> supported.
>
> The port is a FPGA fabric based bridge with expanded functionality,
> right? So it is similar to the altera freeze bridge, but adds the
> ability to reset the fabric and some other features are promised in
> the future, IIUC. I still think that the port could be implemented in
> the bridge driver .c file instead of being here as part of the
> enumeration code. For that to happen, some APIs would need to be
> added to the bridge framework and the FPGA region framework. Then the
> reset can be requested through a new FPGA region API function.
>
> The advantage of this is that if this patchset evolves and there is
> some other v2 port driver needed, it can be a different driver if it
> needs to be.
>
> If the port reset is really a fabric reset,
Actually 'fabric reset' is probably not clear enough. It's resetting
the hardware in a partial reconfiguration region, not just resetting
the bridge. I'm trying to come up with a term that makes that clear
what is getting reset is the contents of the region.
> (correct me if I'm
> remembering wrongly) then it would be helpful to call it a
> fabric_reset. This would be the first bridge driver supporting fabric
> reset. I think it won't be the last.
>
> So what I'm proposing would be added/changed would be:
> * move all the bridge code to fpga-dfl-fme-br.c
> * add .fabric_reset to bridge ops
> * add fpga_bridges_reset to fpga-bridge.c (a new function that goes
> through a list of bridges and calls the reset ops if it exists,
> ignores the bridges where it doesn't exist)
> * add fpga_region_fabric_reset to fpga-region.c. This function gets
> the region, gets the bridges, calls fpga_bridges_reset (can steal code
> from fpga_region_program_fpga)
> * the rest of the patchset can use fpga_region_fabric_reset instead of
> fpga_port_reset
>
> Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
