Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: bENhyfamvJAALxABDCmkJ28Tl6aADDH08k6vGAbJMWRQSKrX7YNfyF81or0lUqM1s06Vf5kNKI
 +vvB72D1/oBA==
IronPort-SDR: hbfJtyZL8si0WExW5zdNVpNj2DyDYa6EOcoDY9ZAxiYGuIhE2+QY+gfaisjNBM/7oc1f1Q5KpK
 eA0NhXiIVZPw==
Date:   Fri, 18 Sep 2020 09:23:58 +0800
From:   Xu Yilun <yilun.xu@intel.com>
To:     Moritz Fischer <mdf@kernel.org>
Cc:     Tom Rix <trix@redhat.com>,
        Russ Weight <russell.h.weight@intel.com>,
        linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
        lgoncalv@redhat.com, hao.wu@intel.com, matthew.gerlach@intel.com,
        yilun.xu@intel.com
Subject: Re: [PATCH v2 1/1] fpga: dfl: afu: harden port enable logic
Message-ID: <20200918012358.GC13851@yilunxu-OptiPlex-7050>
References: <20200917183219.3603-1-russell.h.weight@intel.com>
 <7f181203-c164-4e6e-c710-1096b0aa13b8@redhat.com>
 <20200917213850.GA30570@archbook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200917213850.GA30570@archbook>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

> > >  /**
> > >   * __afu_port_enable - enable a port by clear reset
> > >   * @pdev: port platform device.
> > > @@ -32,7 +35,7 @@
> > >   *
> > >   * The caller needs to hold lock for protection.
> > >   */
> > > -void __afu_port_enable(struct platform_device *pdev)
> > > +int __afu_port_enable(struct platform_device *pdev)
> > >  {
> > >  	struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > >  	void __iomem *base;
> > > @@ -41,7 +44,7 @@ void __afu_port_enable(struct platform_device *pdev)
> > >  	WARN_ON(!pdata->disable_count);
> > >  
> > >  	if (--pdata->disable_count != 0)
> > > -		return;
> > > +		return 0;
> > Is this really a success ? Maybe -EBUSY ?
> Seems like if it's severe enough for a warning you'd probably want to
> return an error.

This code is to handle the port enable/disable request from multiple
users. This is a voting mechanism, the port would not be physically
enabled if there is still an disable vote. The --diable_count != 0 works
for this purpose. So I think it should be OK here since the voting
mechanism is working as expected.

Thanks,
Yilun