RE: [RFC 2/8] cxgb4: setup pcie memory window 4 and create p2pmem region

2017-04-05 Thread Steve Wise 
> 
> 
> > +static void setup_memwin_p2pmem(struct adapter *adap)
> > +{
> > +   unsigned int mem_base = t4_read_reg(adap,
> CIM_EXTMEM2_BASE_ADDR_A);
> > +   unsigned int mem_size = t4_read_reg(adap,
> CIM_EXTMEM2_ADDR_SIZE_A);
> > +
> > +   if (!use_p2pmem)
> > +   return;
> 
> This is weird, why even call this if !use_p2pmem?
> 

The use_p2pmem was added after the original change.  I'll update as you
suggest.

> > +static int init_p2pmem(struct adapter *adapter)
> > +{
> > +   unsigned int mem_size = t4_read_reg(adapter,
> CIM_EXTMEM2_ADDR_SIZE_A);
> > +   struct p2pmem_dev *p;
> > +   int rc;
> > +   struct resource res;
> > +
> > +   if (!mem_size || !use_p2pmem)
> > +   return 0;
> 
> Again, weird...

Yup.

Re: [RFC 2/8] cxgb4: setup pcie memory window 4 and create p2pmem region

2017-04-04 Thread Logan Gunthorpe 


On 04/04/17 04:42 AM, Sagi Grimberg wrote:
> This is weird, why even call this if !use_p2pmem?

I personally find it cleaner than:

if (use_p2pmem)
setup_memwin_p2pmem(...)

I'm not sure why that's so weird.

Logan

Re: [RFC 2/8] cxgb4: setup pcie memory window 4 and create p2pmem region

2017-04-04 Thread Sagi Grimberg 



+static void setup_memwin_p2pmem(struct adapter *adap)
+{
+   unsigned int mem_base = t4_read_reg(adap, CIM_EXTMEM2_BASE_ADDR_A);
+   unsigned int mem_size = t4_read_reg(adap, CIM_EXTMEM2_ADDR_SIZE_A);
+
+   if (!use_p2pmem)
+   return;


This is weird, why even call this if !use_p2pmem?


+static int init_p2pmem(struct adapter *adapter)
+{
+   unsigned int mem_size = t4_read_reg(adapter, CIM_EXTMEM2_ADDR_SIZE_A);
+   struct p2pmem_dev *p;
+   int rc;
+   struct resource res;
+
+   if (!mem_size || !use_p2pmem)
+   return 0;


Again, weird...