LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [2.6.19 PATCH 1/7] ehea: interface to network stack
From: Alexey Dobriyan @ 2006-08-18 14:44 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181329.02042.ossthema@de.ibm.com>

On Fri, Aug 18, 2006 at 01:29:01PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_main.c
> +++ kernel/drivers/net/ehea/ehea_main.c

> +static struct net_device_stats *ehea_get_stats(struct net_device *dev)
> +{
> +	int i;
> +	u64 hret = H_HARDWARE;

Useless assignment here and everywhere.

> +	u64 rx_packets = 0;
> +	struct ehea_port *port = netdev_priv(dev);
> +	struct hcp_query_ehea_port_cb_2 *cb2 = NULL;
> +	struct net_device_stats *stats = &port->stats;
> +
> +	cb2 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> +	if (!cb2) {
> +		ehea_error("no mem for cb2");
> +		goto get_stat_exit;
> +	}
> +
> +	hret = ehea_h_query_ehea_port(port->adapter->handle,
> +				      port->logical_port_id,
> +				      H_PORT_CB2,
> +				      H_PORT_CB2_ALL,
> +				      cb2);

> +static inline int ehea_refill_rq1(struct ehea_port_res *pr, int index,
> +				  int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct sk_buff **skb_arr_rq1 = pr->skb_arr_rq1;
> +	int max_index_mask = pr->skb_arr_rq1_len - 1;
> +
> +	if (!nr_of_wqes)
> +		return 0;
> +
> +	for (i = 0; i < nr_of_wqes; i++) {
> +		if (!skb_arr_rq1[index]) {
> +			skb_arr_rq1[index] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +
> +			if (!skb_arr_rq1[index]) {
> +				ehea_error("no mem for skb/%d wqes filled", i);
> +				ret = -ENOMEM;
> +				break;
> +			}
> +		}
> +		index--;
> +		index &= max_index_mask;
> +	}
> +	/* Ring doorbell */
> +	ehea_update_rq1a(pr->qp, i);
> +
> +	return ret;
> +}
> +
> +static int ehea_init_fill_rq1(struct ehea_port_res *pr, int nr_rq1a)
> +{
> +	int i;
> +	int ret = 0;
> +	struct sk_buff **skb_arr_rq1 = pr->skb_arr_rq1;
> +
> +	for (i = 0; i < pr->skb_arr_rq1_len; i++) {
> +		skb_arr_rq1[i] = dev_alloc_skb(EHEA_LL_PKT_SIZE);
> +		if (!skb_arr_rq1[i]) {
> +			ehea_error("no mem for skb/%d skbs filled.", i);
> +			ret = -ENOMEM;
> +			goto nomem;
> +		}
> +	}
> +	/* Ring doorbell */
> +	ehea_update_rq1a(pr->qp, nr_rq1a);
> +nomem:
> +	return ret;
> +}
> +
> +static inline int ehea_refill_rq2_def(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp;
> +	struct ehea_rwqe *rwqe;
> +	struct sk_buff **skb_arr_rq2 = pr->skb_arr_rq2;
> +	int index, max_index_mask;
> +
> +	if (!nr_of_wqes)
> +		return 0;
> +
> +	qp = pr->qp;
> +
> +	index = pr->skb_rq2_index;
> +	max_index_mask = pr->skb_arr_rq2_len - 1;
> +	for (i = 0; i < nr_of_wqes; i++) {
> +		struct sk_buff *skb = dev_alloc_skb(EHEA_RQ2_PKT_SIZE
> +						    + NET_IP_ALIGN);
> +		if (!skb) {
> +			ehea_error("no mem for skb/%d wqes filled", i);
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		skb_reserve(skb, NET_IP_ALIGN);
> +
> +		skb_arr_rq2[index] = skb;
> +
> +		rwqe = ehea_get_next_rwqe(qp, 2);
> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE2_TYPE)
> +		            | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
> +		rwqe->sg_list[0].l_key = pr->recv_mr.lkey;
> +		rwqe->sg_list[0].vaddr = (u64)skb->data;
> +		rwqe->sg_list[0].len = EHEA_RQ2_PKT_SIZE;
> +		rwqe->data_segments = 1;
> +
> +		index++;
> +		index &= max_index_mask;
> +	}
> +	pr->skb_rq2_index = index;
> +
> +	/* Ring doorbell */
> +	iosync();
> +	ehea_update_rq2a(qp, i);
> +	return ret;
> +}
> +
> +
> +static inline int ehea_refill_rq2(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	return ehea_refill_rq2_def(pr, nr_of_wqes);
> +}
> +
> +static inline int ehea_refill_rq3_def(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	int i;
> +	int ret = 0;
> +	struct ehea_qp *qp = pr->qp;
> +	struct ehea_rwqe *rwqe;
> +	int skb_arr_rq3_len = pr->skb_arr_rq3_len;
> +	struct sk_buff **skb_arr_rq3 = pr->skb_arr_rq3;
> +	int index, max_index_mask;
> +
> +	if (nr_of_wqes == 0)
> +		return -EINVAL;
> +        
> +	index = pr->skb_rq3_index;
> +	max_index_mask = skb_arr_rq3_len - 1;
> +	for (i = 0; i < nr_of_wqes; i++) {
> +		struct sk_buff *skb = dev_alloc_skb(EHEA_MAX_PACKET_SIZE
> +						    + NET_IP_ALIGN);
> +		if (!skb) {
> +			ehea_error("no mem for skb/%d wqes filled", i);
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		skb_reserve(skb, NET_IP_ALIGN);
> +
> +		rwqe = ehea_get_next_rwqe(qp, 3);
> +
> +		skb_arr_rq3[index] = skb;
> +
> +		rwqe->wr_id = EHEA_BMASK_SET(EHEA_WR_ID_TYPE, EHEA_RWQE3_TYPE)
> +			    | EHEA_BMASK_SET(EHEA_WR_ID_INDEX, index);
> +		rwqe->sg_list[0].l_key = pr->recv_mr.lkey;
> +		rwqe->sg_list[0].vaddr = (u64)skb->data;
> +		rwqe->sg_list[0].len = EHEA_MAX_PACKET_SIZE;
> +		rwqe->data_segments = 1;
> +
> +		index++;
> +		index &= max_index_mask;
> +	}
> +	pr->skb_rq3_index = index;
> +
> +	/* Ring doorbell */
> +	iosync();
> +	ehea_update_rq3a(qp, i);
> +	return ret;
> +}

This one looks too big to be inlined as well as other similalry named
functions.

> +static inline int ehea_refill_rq3(struct ehea_port_res *pr, int nr_of_wqes)
> +{
> +	return ehea_refill_rq3_def(pr, nr_of_wqes);
> +}
> +
> +static inline int ehea_check_cqe(struct ehea_cqe *cqe, int *rq_num)
> +{
> +	*rq_num = (cqe->type & EHEA_CQE_TYPE_RQ) >> 5;
> +	if ((cqe->status & EHEA_CQE_STAT_ERR_MASK) == 0)
> +		return 0;
> +	if (((cqe->status & EHEA_CQE_STAT_ERR_TCP) != 0)
> +	    && (cqe->header_length == 0))
> +		return 0;
> +	return -EINVAL;
> +}
> +
> +static inline void ehea_fill_skb(struct net_device *dev,
> +				 struct sk_buff *skb, struct ehea_cqe *cqe)
> +{
> +	int length = cqe->num_bytes_transfered - 4;	/*remove CRC */
> +	skb_put(skb, length);
> +	skb->dev = dev;
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> +	skb->protocol = eth_type_trans(skb, dev);
> +}

> +static int ehea_init_port_res(struct ehea_port *port, struct ehea_port_res *pr,
> +			      struct port_res_cfg *pr_cfg, int queue_token)
> +{
> +	int ret = -EINVAL;
> +	int max_rq_entries = 0;
> +	enum ehea_eq_type eq_type = EHEA_EQ;
> +	struct ehea_qp_init_attr *init_attr = NULL;
> +	struct ehea_adapter *adapter = port->adapter;
> +
> +	memset(pr, 0, sizeof(struct ehea_port_res));
> +
> +	pr->skb_arr_rq3 = NULL;
> +	pr->skb_arr_rq2 = NULL;
> +	pr->skb_arr_rq1 = NULL;
> +	pr->skb_arr_sq = NULL;
> +	pr->qp = NULL;
> +	pr->send_cq = NULL;
> +	pr->recv_cq = NULL;
> +	pr->send_eq = NULL;
> +	pr->recv_eq = NULL;

After memset unneeded. ;-)

> +static int ehea_clean_port_res(struct ehea_port *port, struct ehea_port_res *pr)

Freeing/deallocating/cleaning functions usually return void. The fact
that you always return -EINVAL only reaffirmes my belief. ;-)

> +{
> +	int i;
> +	int ret = -EINVAL;
> +
> +	ehea_destroy_qp(pr->qp);
> +	ehea_destroy_cq(pr->send_cq);
> +	ehea_destroy_cq(pr->recv_cq);
> +	ehea_destroy_eq(pr->send_eq);
> +	ehea_destroy_eq(pr->recv_eq);
> +
> +	for (i = 0; i < pr->skb_arr_rq1_len; i++) {
> +		if (pr->skb_arr_rq1[i])
> +			dev_kfree_skb(pr->skb_arr_rq1[i]);
> +	}
> +
> +	for (i = 0; i < pr->skb_arr_rq2_len; i++)
> +		if (pr->skb_arr_rq2[i])
> +			dev_kfree_skb(pr->skb_arr_rq2[i]);
> +
> +	for (i = 0; i < pr->skb_arr_rq3_len; i++)
> +		if (pr->skb_arr_rq3[i])
> +			dev_kfree_skb(pr->skb_arr_rq3[i]);
> +
> +	for (i = 0; i < pr->skb_arr_sq_len; i++)
> +		if (pr->skb_arr_sq[i])
> +			dev_kfree_skb(pr->skb_arr_sq[i]);
> +
> +	vfree(pr->skb_arr_sq);
> +	vfree(pr->skb_arr_rq1);
> +	vfree(pr->skb_arr_rq2);
> +	vfree(pr->skb_arr_rq3);
> +
> +	ehea_rem_smrs(pr);
> +	return ret;
> +}

> +static inline void write_swqe2_data(struct sk_buff *skb,
> +				    struct net_device *dev,
> +				    struct ehea_swqe *swqe,
> +				    u32 lkey)
> +{

Way too big.

> +	int skb_data_size, nfrags, headersize, i, sg1entry_contains_frag_data;
> +	struct ehea_vsgentry *sg_list;
> +	struct ehea_vsgentry *sg1entry;
> +	struct ehea_vsgentry *sgentry;
> +	u8 *imm_data;
> +	u64 tmp_addr;
> +	skb_frag_t *frag;
> +
> +	skb_data_size = skb->len - skb->data_len;
> +	nfrags = skb_shinfo(skb)->nr_frags;
> +	sg1entry = &swqe->u.immdata_desc.sg_entry;
> +	sg_list = (struct ehea_vsgentry*)&swqe->u.immdata_desc.sg_list;
> +	imm_data = &swqe->u.immdata_desc.immediate_data[0];
> +	swqe->descriptors = 0;
> +	sg1entry_contains_frag_data = 0;
> +
> +	if ((dev->features & NETIF_F_TSO) && skb_shinfo(skb)->gso_size) {
> +		/* Packet is TCP with TSO enabled */
> +		swqe->tx_control |= EHEA_SWQE_TSO;
> +		swqe->mss = skb_shinfo(skb)->gso_size;
> +		/* copy only eth/ip/tcp headers to immediate data and
> +		 * the rest of skb->data to sg1entry
> +		 */
> +		headersize = ETH_HLEN + (skb->nh.iph->ihl * 4)
> +				      + (skb->h.th->doff * 4);
> +
> +		skb_data_size = skb->len - skb->data_len;
> +
> +		if (skb_data_size >= headersize) {
> +			/* copy immediate data */
> +			memcpy(imm_data, skb->data, headersize);
> +			swqe->immediate_data_length = headersize;
> +
> +			if (skb_data_size > headersize) {
> +				/* set sg1entry data */
> +				sg1entry->l_key = lkey;
> +				sg1entry->len = skb_data_size - headersize;
> +
> +				tmp_addr = (u64)(skb->data + headersize);
> +				sg1entry->vaddr = tmp_addr;
> +				swqe->descriptors++;
> +			}
> +		} else
> +			ehea_error("cannot handle fragmented headers");
> +	} else {
> +		/* Packet is any nonTSO type
> +		 *
> +		 * Copy as much as possible skb->data to immediate data and
> +		 * the rest to sg1entry
> +		 */
> +		if (skb_data_size >= SWQE2_MAX_IMM) {
> +			/* copy immediate data */
> +			memcpy(imm_data, skb->data, SWQE2_MAX_IMM);
> +
> +			swqe->immediate_data_length = SWQE2_MAX_IMM;
> +
> +			if (skb_data_size > SWQE2_MAX_IMM) {
> +				/* copy sg1entry data */
> +				sg1entry->l_key = lkey;
> +				sg1entry->len = skb_data_size - SWQE2_MAX_IMM;
> +				tmp_addr = (u64)(skb->data + SWQE2_MAX_IMM);
> +				sg1entry->vaddr = tmp_addr;
> +				swqe->descriptors++;
> +			}
> +		} else {
> +			memcpy(imm_data, skb->data, skb_data_size);
> +			swqe->immediate_data_length = skb_data_size;
> +		}
> +	}
> +
> +	/* write descriptors */
> +	if (nfrags > 0) {
> +		if (swqe->descriptors == 0) {
> +			/* sg1entry not yet used */
> +			frag = &skb_shinfo(skb)->frags[0];
> +
> +			/* copy sg1entry data */
> +			sg1entry->l_key = lkey;
> +			sg1entry->len = frag->size;
> +			tmp_addr =  (u64)(page_address(frag->page)
> +					  + frag->page_offset);
> +			sg1entry->vaddr = tmp_addr;
> +			swqe->descriptors++;
> +			sg1entry_contains_frag_data = 1;
> +		}
> +
> +		for (i = sg1entry_contains_frag_data; i < nfrags; i++) {
> +
> +			frag = &skb_shinfo(skb)->frags[i];
> +			sgentry = &sg_list[i - sg1entry_contains_frag_data];
> +
> +			sgentry->l_key = lkey;
> +			sgentry->len = frag->size;
> +
> +			tmp_addr = (u64)(page_address(frag->page)
> +					 + frag->page_offset);
> +			sgentry->vaddr = tmp_addr;
> +		}
> +	}
> +}

Got it?

> +static inline void ehea_xmit2(struct sk_buff *skb,
> +			      struct net_device *dev, struct ehea_swqe *swqe,
> +			      u32 lkey)
> +{
> +	int nfrags;
> +	nfrags = skb_shinfo(skb)->nr_frags;
> +
> +	if (skb->protocol == htons(ETH_P_IP)) {
> +		/* IPv4 */
> +		swqe->tx_control |= EHEA_SWQE_CRC
> +				 | EHEA_SWQE_IP_CHECKSUM
> +				 | EHEA_SWQE_TCP_CHECKSUM
> +				 | EHEA_SWQE_IMM_DATA_PRESENT
> +				 | EHEA_SWQE_DESCRIPTORS_PRESENT;
> +
> +		write_ip_start_end(swqe, skb);
> +
> +		if (skb->nh.iph->protocol == IPPROTO_UDP) {
> +			if ((skb->nh.iph->frag_off & IP_MF)
> +			    || (skb->nh.iph->frag_off & IP_OFFSET))
> +				/* IP fragment, so don't change cs */
> +				swqe->tx_control &= ~EHEA_SWQE_TCP_CHECKSUM;
> +			else
> +				write_udp_offset_end(swqe, skb);
> +
> +		} else if (skb->nh.iph->protocol == IPPROTO_TCP) {
> +			write_tcp_offset_end(swqe, skb);
> +		}
> +
> +		/* icmp (big data) and ip segmentation packets (all other ip
> +		   packets) do not require any special handling */
> +
> +	} else {
> +		/* Other Ethernet Protocol */
> +		swqe->tx_control |= EHEA_SWQE_CRC
> +				 | EHEA_SWQE_IMM_DATA_PRESENT
> +				 | EHEA_SWQE_DESCRIPTORS_PRESENT;
> +	}
> +
> +	write_swqe2_data(skb, dev, swqe, lkey);
> +}
> +
> +static inline void ehea_xmit3(struct sk_buff *skb,
> +			      struct net_device *dev, struct ehea_swqe *swqe)
> +{
> +	int i;
> +	skb_frag_t *frag;
> +	int nfrags = skb_shinfo(skb)->nr_frags;
> +	u8 *imm_data = &swqe->u.immdata_nodesc.immediate_data[0];
> +
> +	if (likely(skb->protocol == htons(ETH_P_IP))) {
> +		/* IPv4 */
> +		write_ip_start_end(swqe, skb);
> +
> +		if (skb->nh.iph->protocol == IPPROTO_TCP) {
> +			swqe->tx_control |= EHEA_SWQE_CRC
> +					 | EHEA_SWQE_IP_CHECKSUM
> +					 | EHEA_SWQE_TCP_CHECKSUM
> +					 | EHEA_SWQE_IMM_DATA_PRESENT;
> +
> +			write_tcp_offset_end(swqe, skb);
> +
> +		} else if (skb->nh.iph->protocol == IPPROTO_UDP) {
> +			if ((skb->nh.iph->frag_off & IP_MF)
> +			    || (skb->nh.iph->frag_off & IP_OFFSET))
> +				/* IP fragment, so don't change cs */
> +				swqe->tx_control |= EHEA_SWQE_CRC
> +						 | EHEA_SWQE_IMM_DATA_PRESENT;
> +			else {
> +				swqe->tx_control |= EHEA_SWQE_CRC
> +						 | EHEA_SWQE_IP_CHECKSUM
> +						 | EHEA_SWQE_TCP_CHECKSUM
> +						 | EHEA_SWQE_IMM_DATA_PRESENT;
> +
> +				write_udp_offset_end(swqe, skb);
> +			}
> +		} else {
> +			/* icmp (big data) and
> +			   ip segmentation packets (all other ip packets) */
> +			swqe->tx_control |= EHEA_SWQE_CRC
> +					 | EHEA_SWQE_IP_CHECKSUM
> +					 | EHEA_SWQE_IMM_DATA_PRESENT;
> +		}
> +	} else {
> +		/* Other Ethernet Protocol */
> +		swqe->tx_control |= EHEA_SWQE_CRC | EHEA_SWQE_IMM_DATA_PRESENT;
> +	}
> +	/* copy (immediate) data */
> +	if (nfrags == 0) {
> +		/* data is in a single piece */
> +		memcpy(imm_data, skb->data, skb->len);
> +	} else {
> +		/* first copy data from the skb->data buffer ... */
> +		memcpy(imm_data, skb->data, skb->len - skb->data_len);
> +		imm_data += skb->len - skb->data_len;
> +
> +		/* ... then copy data from the fragments */
> +		for (i = 0; i < nfrags; i++) {
> +			frag = &skb_shinfo(skb)->frags[i];
> +			memcpy(imm_data,
> +			       page_address(frag->page) + frag->page_offset,
> +			       frag->size);
> +			imm_data += frag->size;
> +		}
> +	}
> +	swqe->immediate_data_length = skb->len;
> +	dev_kfree_skb(skb);
> +}

Ditto.

> +static void ehea_vlan_rx_register(struct net_device *dev,
> +				  struct vlan_group *grp)
> +{
> +	u64 hret = H_HARDWARE;
> +	struct hcp_query_ehea_port_cb_1 *cb1 = NULL;
> +	struct ehea_port *port = netdev_priv(dev);
> +	struct ehea_adapter *adapter = port->adapter;
> +
> +	port->vgrp = grp;
> +
> +	cb1 = kzalloc(H_CB_ALIGNMENT, GFP_KERNEL);
> +	if (!cb1) {
> +		ehea_error("no mem for cb1");
> +		goto vlan_reg_exit;
> +	}
> +
> +	if (grp)
> +		memset(cb1->vlan_filter, 0, sizeof(cb1->vlan_filter));
> +	else
> +		memset(cb1->vlan_filter, 1, sizeof(cb1->vlan_filter));

Just to be sure, this should be 1 not 0xff?

> +	hret = ehea_h_modify_ehea_port(adapter->handle,
> +				       port->logical_port_id,
> +				       H_PORT_CB1,
> +				       H_PORT_CB1_ALL,
> +				       cb1);
> +	if (hret != H_SUCCESS)
> +		ehea_error("modify_ehea_port failed");
> +
> +	kfree(cb1);
> +
> +vlan_reg_exit:
> +	return;
> +}

> +void ehea_clean_all_port_res(struct ehea_port *port)
> +{
> +	int ret;
> +	int i;
> +	for(i = 0; i < port->num_def_qps + port->num_tx_qps; i++)
> +		ehea_clean_port_res(port, &port->port_res[i]);
> +
> +	ret = ehea_destroy_eq(port->qp_eq);
> +}

ret is entirely useless.

> +int __init ehea_module_init(void)
static

> +{
> +	int ret = -EINVAL;
> +
> +	printk("IBM eHEA Ethernet Device Driver (Release %s)\n", DRV_VERSION);
> +
> +	ret = ibmebus_register_driver(&ehea_driver);
> +	if (ret) {
> +		ehea_error("failed registering eHEA device driver on ebus");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

Pass ret to upper layer. Simplest way is:

	static int __init ehea_module_init(void)
	{
		return ibmebus_register_driver(&ehea_driver);
	}

^ permalink raw reply

* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Alexey Dobriyan @ 2006-08-18 15:06 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181330.21507.ossthema@de.ibm.com>

On Fri, Aug 18, 2006 at 01:30:21PM +0200, Jan-Bernd Themann wrote:
> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea_phyp.c
> +++ kernel/drivers/net/ehea/ehea_phyp.c

> +	hret = ehea_hcall_9arg_9ret(H_QUERY_HEA_QP,
> +				    hcp_adapter_handle,	        /* R4 */
> +				    qp_category,	        /* R5 */
> +				    qp_handle,	                /* R6 */
> +				    sel_mask,	                /* R7 */
> +				    virt_to_abs(cb_addr),	/* R8 */
> +				    0, 0, 0, 0,	                /* R9-R12 */
> +				    &dummy,                     /* R4 */
> +				    &dummy,                     /* R5 */
> +				    &dummy,	                /* R6 */
> +				    &dummy,	                /* R7 */
> +				    &dummy,	                /* R8 */
> +				    &dummy,	                /* R9 */
> +				    &dummy,	                /* R10 */
> +				    &dummy,	                /* R11 */
> +				    &dummy);	                /* R12 */

I asked SO to recount arguments and we've come to a conclusion that
there're in fact 19 args not 18 as the name suggests. 19 args is
I-N-S-A-N-E.

> +u64 ehea_h_register_rpage_eq(const u64 hcp_adapter_handle,
> +			     const u64 eq_handle,
> +			     const u8 pagesize,
> +			     const u8 queue_type,
> +			     const u64 log_pageaddr, const u64 count)
> +{
> +	u64 hret = H_ADAPTER_PARM;
> +
> +	if (count != 1)
> +		return H_PARAMETER;
> +
> +	hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> +				     eq_handle, log_pageaddr, count);
> +	return hret;

Just

	return ehea_h_register_rpage(...);

> +u64 ehea_h_register_rpage_cq(const u64 hcp_adapter_handle,
> +			     const u64 cq_handle,
> +			     const u8 pagesize,
> +			     const u8 queue_type,
> +			     const u64 log_pageaddr,
> +			     const u64 count, const struct h_epa epa)
> +{
> +	u64 hret = H_ADAPTER_PARM;
> +
> +	if (count != 1)
> +		return H_PARAMETER;
> +
> +	hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> +				     cq_handle, log_pageaddr, count);
> +	return hret;

Ditto.

> +u64 ehea_h_register_rpage_qp(const u64 hcp_adapter_handle,
> +			     const u64 qp_handle,
> +			     const u8 pagesize,
> +			     const u8 queue_type,
> +			     const u64 log_pageaddr,
> +			     const u64 count, struct h_epa epa)
> +{
> +	u64 hret = H_ADAPTER_PARM;
> +
> +	if (count != 1)
> +		return H_PARAMETER;
> +
> +	hret = ehea_h_register_rpage(hcp_adapter_handle, pagesize, queue_type,
> +				     qp_handle, log_pageaddr, count);
> +	return hret;
> +}

Ditto.

> +static inline int hcp_epas_ctor(struct h_epas *epas, u64 paddr_kernel,
> +				u64 paddr_user)
> +{
> +	epas->kernel.fw_handle = (u64) ioremap(paddr_kernel, PAGE_SIZE);
> +	epas->user.fw_handle = paddr_user;
> +	return 0;
> +}
> +
> +static inline int hcp_epas_dtor(struct h_epas *epas)
> +{
> +
> +	if (epas->kernel.fw_handle)
> +		iounmap((void *)epas->kernel.fw_handle);
> +	epas->user.fw_handle = epas->kernel.fw_handle = 0;
> +	return 0;
> +}

Always returns 0;

^ permalink raw reply

* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Anton Blanchard @ 2006-08-18 15:13 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, Thomas Klein, linux-ppc, Marcus Eder
In-Reply-To: <20060818150600.GG5201@martell.zuzino.mipt.ru>

 
Hi,

> I asked SO to recount arguments and we've come to a conclusion that
> there're in fact 19 args not 18 as the name suggests. 19 args is
> I-N-S-A-N-E.

It will be partially cleaned up by:

http://ozlabs.org/pipermail/linuxppc-dev/2006-July/024556.html

However it doesnt fix the fact someone has architected such a crazy
interface :(

Anton

^ permalink raw reply

* Re: [PATCH 02/13] IB/ehca: includes
From: Christoph Raisch @ 2006-08-18 15:35 UTC (permalink / raw)
  To: abergman
  Cc: linux-kernel, openib-general, linuxppc-dev, Hoang-Nam Nguyen,
	Marcus Eder
In-Reply-To: <200608180144.19149.arnd.bergmann@de.ibm.com>


abergman

> > +#define EDEB_P_GENERIC(level,idstring,format,args...) \
>
> These macros are responsible for 61% of the object code size of your
module.
> ...Please get rid of that crap entirely and replace
> it with dev_info/dev_dbg/dev_warn calls where appropriate!
>
>    Arnd <><

we'll change these EDEBs to a wrapper around dev_err, dev_dbg and dev_warn
as it's done in the mthca driver.
All EDEB_EN and EDEB_EX will be removed, that type of tracing can be done
if needed by kprobes.
There are a few cases where we won't get to a dev, for these few places
we'll use a simple wrapper around printk, as done in ipoib.

Hope that's the "official" way how to implement it in ib drivers.


Gruss / Regards . . . Christoph R

^ permalink raw reply

* Re: [PATCH 13/13] IB/ehca: makefiles/kconfig
From: Hoang-Nam Nguyen @ 2006-08-18 15:43 UTC (permalink / raw)
  To: abergman
  Cc: linux-kernel, openib-general, linuxppc-dev, Christoph Raisch,
	Marcus Eder
In-Reply-To: <200608180134.39050.arnd.bergmann@de.ibm.com>


abergman@de.ltcfwd.linux.ibm.com wrote on 18.08.2006 01:34:37:

> On Thursday 17 August 2006 22:11, Roland Dreier wrote:
> > +
> > +CFLAGS += -DEHCA_USE_HCALL -DEHCA_USE_HCALL_KERNEL
>
> This seems really pointless, since you're always defining these
> macros to the same value.
>
> Just drop the CFLAGS and remove the code that depends on them
> being different.
Yes, that's true. Those defines are unnecessary. We'll throw them out.
Thx
Hoang-Nam Nguyen

^ permalink raw reply

* Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
From: Thomas Klein @ 2006-08-18 15:41 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, linux-ppc, Marcus Eder
In-Reply-To: <20060818140506.GC5201@martell.zuzino.mipt.ru>

Hi Alexey,

first of all thanks a lot for the extensive review.


Alexey Dobriyan wrote:
>> +	u64 hret = H_HARDWARE;
> 
> Useless assignment here and everywhere.
> 

Initializing returncodes to errorstate is a cheap way to prevent
accidentally returning (uninitalized) success returncodes which
can lead to catastrophic misbehaviour.

>> +static void netdev_get_drvinfo(struct net_device *dev,
>> +			       struct ethtool_drvinfo *info)
>> +{
>> +	strncpy(info->driver, DRV_NAME, sizeof(info->driver) - 1);
>> +	strncpy(info->version, DRV_VERSION, sizeof(info->version) - 1);
> 
> Use strlcpy() to not forget -1 accidently.

I agree.

Kind regards
Thomas

^ permalink raw reply

* Re: [2.6.19 PATCH 2/7] ehea: pHYP interface
From: Christoph Raisch @ 2006-08-18 16:02 UTC (permalink / raw)
  To: Anton Blanchard
  Cc: Thomas Q Klein, Jan-Bernd Themann, netdev, linux-kernel,
	linux-ppc, Marcus Eder, tklein, Alexey Dobriyan
In-Reply-To: <20060818151340.GB27947@krispykreme>


>
> Hi,
>
> > I asked SO to recount arguments and we've come to a conclusion that
> > there're in fact 19 args not 18 as the name suggests. 19 args is
> > I-N-S-A-N-E.
>
> It will be partially cleaned up by:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2006-July/024556.html
>
> However it doesnt fix the fact someone has architected such a crazy
> interface :(
>
> Anton


well, just as background info, this is the wrapper around
a single assembly instruction which calls system firmware and takes
9 CPU registers for input and 9 CPU registers for output parameters.
This definition by platform architecture won't change in the near future,
but the good news is with Antons change the wrapper will look much nicer.

Gruss / Regards . . . Christoph R

^ permalink raw reply

* Re: [2.6.19 PATCH 3/7] ehea: queue management
From: Arnd Bergmann @ 2006-08-18 16:16 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Thomas Q Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, Marcus Eder, tklein, abergman, Arjan van de Ven
In-Reply-To: <OFC184057C.C58732A2-ONC12571CE.004E5EF6-C12571CE.004ED1D4@de.ibm.com>

On Friday 18 August 2006 16:24, Christoph Raisch wrote:
> And as always in performance tuning... one size fits all unfortunately is
> not the correct answer.

Ah, good. What is the maximum sensible value that you came up with?

> Therefore we'll leave that open to the user as most other new ethernet
> driver did as well.

Sure. The interesting question is whether you want to allow users
to set it to a value that is no longer sensible to do with
__get_free_pages() and requires vmalloc().

	Arnd <><

^ permalink raw reply

* Re: Please pull powerpc.git 'merge' branch
From: Greg KH @ 2006-08-18 16:20 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <17637.8568.40692.21510@cargo.ozlabs.ibm.com>

On Fri, Aug 18, 2006 at 12:10:00PM +1000, Paul Mackerras wrote:
> Greg,
> 
> Please do:
> 
> git pull \
> git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git merge
> 
> to get the following PowerPC updates for 2.6.18.

Pulled from, and pushed out.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 02/13] IB/ehca: includes
From: Arnd Bergmann @ 2006-08-18 16:21 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Christoph Raisch, Hoang-Nam Nguyen, linux-kernel, openib-general,
	Marcus Eder
In-Reply-To: <OFED0915E4.3CED6795-ONC12571CE.0053ECB8-C12571CE.0055546A@de.ibm.com>

On Friday 18 August 2006 17:35, Christoph Raisch wrote:
> we'll change these EDEBs to a wrapper around dev_err, dev_dbg and dev_warn
> as it's done in the mthca driver.
>
> ...
>
> Hope that's the "official" way how to implement it in ib drivers.

I guess it would be even better to just use the dev_* macros directly
instead of having your own wrapper. You can do that in both ehca and ehea.

	Arnd <><

^ permalink raw reply

* Re: InfiniBand merge plans for 2.6.19
From: Michael S. Tsirkin @ 2006-08-18 16:21 UTC (permalink / raw)
  To: Roland Dreier; +Cc: netdev, linux-kernel, openib-general, linuxppc-dev
In-Reply-To: <adawt9786ii.fsf@cisco.com>

Quoting r. Roland Dreier <rdreier@cisco.com>:
>     o  I also have the following minor changes queued in the
>        for-2.6.19 branch of infiniband.git:


Cold you oplease consider IB/mthca: recover from device errors
as well?

-- 
MST

^ permalink raw reply

* Re: InfiniBand merge plans for 2.6.19
From: Roland Dreier @ 2006-08-18 16:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, openib-general, linuxppc-dev
In-Reply-To: <20060818162135.GA20206@mellanox.co.il>

    Michael> Cold you oplease consider IB/mthca: recover from device
    Michael> errors as well?

Yes, I will.  There's still plenty of time before 2.6.19 opens up.

^ permalink raw reply

* Re: update: consolidated flat device tree code
From: Hollis Blanchard @ 2006-08-18 16:51 UTC (permalink / raw)
  To: Pantelis Antoniou, Matthew McClintock; +Cc: linuxppc-dev, linuxppc-embedded
In-Reply-To: <1155860626.27466.126.camel@basalt.austin.ibm.com>

On Thu, 2006-08-17 at 19:23 -0500, Hollis Blanchard wrote:
> - added an explicit copyright statement.

On this subject, it's just been asked that the Xen library containing
this code be relicensed as LGPL (instead of GPL).

Panto, you're the original author of that code. Since we are trying to
make a library, I think LGPL makes sense. Is relicensing OK with you?

(Matt, this would be relevant to your outstanding patches as well.)

-- 
Hollis Blanchard
IBM Linux Technology Center

^ permalink raw reply

* Re: update: consolidated flat device tree code
From: Pantelis Antoniou @ 2006-08-18 16:52 UTC (permalink / raw)
  To: Hollis Blanchard; +Cc: linuxppc-embedded, linuxppc-dev
In-Reply-To: <1155919908.23182.28.camel@basalt.austin.ibm.com>


On 18 =CE=91=CF=85=CE=B3 2006, at 7:51 =CE=9C=CE=9C, Hollis Blanchard =
wrote:

> On Thu, 2006-08-17 at 19:23 -0500, Hollis Blanchard wrote:
>> - added an explicit copyright statement.
>
> On this subject, it's just been asked that the Xen library containing
> this code be relicensed as LGPL (instead of GPL).
>
> Panto, you're the original author of that code. Since we are trying to
> make a library, I think LGPL makes sense. Is relicensing OK with you?
>
> (Matt, this would be relevant to your outstanding patches as well.)
>
> --=20
> Hollis Blanchard
> IBM Linux Technology Center
>

Yes, this is fine.

Please go ahead.

Pantelis

^ permalink raw reply

* Re: [PATCH] Directly reference i8259@4d0 nodes in mpc8641_hpcn.dts.
From: Jon Loeliger @ 2006-08-18 17:32 UTC (permalink / raw)
  To: linuxppc-dev@ozlabs.org
In-Reply-To: <1155849609.10054.175.camel@cashmere.sps.mot.com>

On Thu, 2006-08-17 at 16:20, Jon Loeliger wrote:
> Rather than using some hand-coded linux,phandle
> node references, use DTC's direct node refs ability
> and let it manage the phandle names instead.
> 
> Signed-off-by: Jon Loeliger <jdl@freescale.com>
> ---
> 
> On Thu, 2006-08-17 at 13:51, Hollis Blanchard wrote:
> > Doesn't the device tree compiler add linux,phandle properties as needed?
> > In this case that would be when the node is referenced by a
> > "<&/foo/bar/i8259@4d0>" property.
> > 
> > On Thu, 2006-08-17 at 12:24 -0500, Jon Loeliger wrote:
> > > Add 'linux,phandle' entry to i8259@4d0 node.
> > > 
> > > Signed-off-by: Zhang Wei <wei.zhang@freescale.com>
> > > Signed-off-by: Jon Loeliger <jdl@freescale.com>
> > > ---
> 
> Paul,
> 
> If you think this is better, please apply this patch
> instead of my previous patch with the subject line:
> 
>     Patch] Fix the mpc8641_hpcn.dts file.
> 
> Thanks,
> jdl

Paul,

I'll take the application of the original patch,

	Fix the mpc8641_hpcn.dts file.

as an indication that this, alternate version should
now be simply dropped!

Thanks,
jdl

^ permalink raw reply

* Re: [2.6.19 PATCH 4/7] ehea: ethtool interface
From: Stephen Hemminger @ 2006-08-18 17:45 UTC (permalink / raw)
  To: Thomas Klein
  Cc: Thomas Klein, Marcus, Jan-Bernd Themann, netdev, linux-kernel,
	Christoph Raisch, linux-ppc, Eder, Alexey Dobriyan
In-Reply-To: <44E5DFA6.7040707@de.ibm.com>

On Fri, 18 Aug 2006 17:41:26 +0200
Thomas Klein <osstklei@de.ibm.com> wrote:

> Hi Alexey,
> 
> first of all thanks a lot for the extensive review.
> 
> 
> Alexey Dobriyan wrote:
> >> +	u64 hret = H_HARDWARE;
> > 
> > Useless assignment here and everywhere.
> > 
> 
> Initializing returncodes to errorstate is a cheap way to prevent
> accidentally returning (uninitalized) success returncodes which
> can lead to catastrophic misbehaviour.

That is old thinking. Current compilers do live/dead analysis
and tell you about this at compile time which is better than relying
on default behavior at runtime.

^ permalink raw reply

* Re: [2.6.19 PATCH 5/7] ehea: main header files
From: Michael Neuling @ 2006-08-18 18:03 UTC (permalink / raw)
  To: Jan-Bernd Themann
  Cc: Thomas Klein, Jan-Bernd Themann, netdev, linux-kernel,
	Thomas Klein, linux-ppc, Christoph Raisch, Marcus Eder
In-Reply-To: <200608181334.57701.ossthema@de.ibm.com>

> +static inline void ehea_update_sqa(struct ehea_qp *qp, u16 nr_wqes)
> +{
> +	struct h_epa epa = qp->epas.kernel;
> +	epa_store_acc(epa, QPTEMM_OFFSET(qpx_sqa),
> +		      EHEA_BMASK_SET(QPX_SQA_VALUE, nr_wqes));
> +}
> +
> +static inline void ehea_update_rq3a(struct ehea_qp *qp, u16 nr_wqes)
> +{
> +	struct h_epa epa = qp->epas.kernel;
> +	epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq3a),
> +		      EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
> +}
> +
> +static inline void ehea_update_rq2a(struct ehea_qp *qp, u16 nr_wqes)
> +{
> +	struct h_epa epa = qp->epas.kernel;
> +	epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq2a),
> +		      EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
> +}
> +
> +static inline void ehea_update_rq1a(struct ehea_qp *qp, u16 nr_wqes)
> +{
> +	struct h_epa epa = qp->epas.kernel;
> +	epa_store_acc(epa, QPTEMM_OFFSET(qpx_rq1a),
> +		      EHEA_BMASK_SET(QPX_RQ1A_VALUE, nr_wqes));
> +}
> +
> +static inline void ehea_update_feca(struct ehea_cq *cq, u32 nr_cqes)
> +{
> +	struct h_epa epa = cq->epas.kernel;
> +	epa_store_acc(epa, CQTEMM_OFFSET(cqx_feca),
> +		      EHEA_BMASK_SET(CQX_FECADDER, nr_cqes));
> +}
> +
> +static inline void ehea_reset_cq_n1(struct ehea_cq *cq)
> +{
> +	struct h_epa epa = cq->epas.kernel;
> +	epa_store_cq(epa, cqx_n1,
> +		     EHEA_BMASK_SET(CQX_N1_GENERATE_COMP_EVENT, 1));
> +}
> +
> +static inline void ehea_reset_cq_ep(struct ehea_cq *my_cq)
> +{
> +	struct h_epa epa = my_cq->epas.kernel;
> +	epa_store_acc(epa, CQTEMM_OFFSET(cqx_ep),
> +		      EHEA_BMASK_SET(CQX_EP_EVENT_PENDING, 0));
> +}

These are almost identical... I'm sure most (if not all) could be merged
into a single function or #define.

Mikey

^ permalink raw reply

* Re: [PATCH] powerpc: emulate power5 popcntb instruction
From: Will Schmidt @ 2006-08-18 18:11 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <36A1A5C6-5269-4BDA-9F3E-A632B3A627DB@kernel.crashing.org>


In an attempt to make it easier for a power5 optimized app to run on a 
power4 or a 970 or random earlier machine, this provides emulation of
the popcntb instruction.    Rewritten to use a slicker algorithm as
suggested by Segher.   I left a 'tmp' variable in play, as it seemed
cleaner to use tmp than referring to regs->gpr[rs] and [ra] multiple
times within the magic algorithm.  

Also tested on power4 with both 32 and 64 userspace this time.
*blush*.  :-) 


Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>

---

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 2105767..a0e80dd 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -588,6 +588,8 @@ #define INST_LSWX		0x7c00042a
 #define INST_STSWI		0x7c0005aa
 #define INST_STSWX		0x7c00052a
 
+#define INST_POPCNTB		0x7c0000f4
+
 static int emulate_string_inst(struct pt_regs *regs, u32 instword)
 {
 	u8 rT = (instword >> 21) & 0x1f;
@@ -656,6 +658,23 @@ static int emulate_string_inst(struct pt
 	return 0;
 }
 
+static int emulate_popcntb_inst(struct pt_regs *regs, u32 instword)
+{
+	u32 ra,rs;
+	unsigned long tmp;
+
+	ra = (instword >> 16) & 0x1f;
+	rs = (instword >> 21) & 0x1f;
+
+	tmp = regs->gpr[rs];
+	tmp = tmp - ((tmp >> 1) & 0x5555555555555555);
+	tmp = (tmp & 0x3333333333333333) + ((tmp >> 2) & 0x3333333333333333);
+	tmp = (tmp + (tmp >> 4)) & 0x0f0f0f0f0f0f0f0f;
+	regs->gpr[ra] = tmp;
+
+	return 0;
+}
+
 static int emulate_instruction(struct pt_regs *regs)
 {
 	u32 instword;
@@ -693,6 +712,11 @@ static int emulate_instruction(struct pt
 	if ((instword & INST_STRING_GEN_MASK) == INST_STRING)
 		return emulate_string_inst(regs, instword);
 
+	/* Emulate the popcntb (Population Count Bytes) instruction. */
+	if ((instword & INST_POPCNTB) == INST_POPCNTB) {
+		return emulate_popcntb_inst(regs, instword);
+	}
+
 	return -EINVAL;
 }
 

^ permalink raw reply related

* Re: [RFC] MPC8260 SPI driver
From: Mark A. Greer @ 2006-08-18 18:34 UTC (permalink / raw)
  To: laurent.pinchart; +Cc: linuxppc-embedded

Laurent,

Should this driver also work on 8247/48/71/72?
If so, renaming 8260 -> 82xx or similar would be nice.

Also, please put patches inline (not as attachments).

Thanks,

Mark

^ permalink raw reply

* Re: [PATCH] Fix IRQ handling on MPC8540 ADS
From: Sergei Shtylyov @ 2006-08-18 18:37 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <Pine.LNX.4.61.0608172019390.420@ld0175-tx32.am.freescale.net>

Hello.

Andy Fleming wrote:
> * Fixed IRQ handling for the 85xx ADS boards so it uses the new
>   generic irq stuff
> * Fixed PCI IRQ mapping so it comes from the device tree

    NAK. The kernel doesn't build with this patch. I'm getting this:

   CC      arch/powerpc/platforms/85xx/mpc85xx_ads.o
arch/powerpc/platforms/85xx/mpc85xx_ads.c: In function `mpc85xx_ads_pic_init':
arch/powerpc/platforms/85xx/mpc85xx_ads.c:76: warning: implicit declaration of
function `of_put_node'

   and then finally:

   LD      .tmp_vmlinux1
arch/powerpc/platforms/built-in.o(.init.text+0x26c): In function 
`mpc85xx_ads_pic_init':
arch/powerpc/platforms/85xx/mpc85xx_ads.c:84: undefined reference to `of_put_node'
arch/powerpc/platforms/built-in.o(.init.text+0x404):arch/powerpc/platforms/85xx/mpc85xx_ads.c:76: 
undefined reference to `of_put_node'
make: *** [.tmp_vmlinux1] Error 1

    There's no such function in the kernel -- it actually aclled 
of_node_put(). I can post an updated patch after testing (if it succeeds) if 
you like...

> This patch *really* needs to go in before 2.6.18 is final, else 2.6.18 
> doesn't build for 85xx.

    Looks like it's a bit crude yet to be committed...

> Thanks,
> Andy

WBR, Sergei

^ permalink raw reply

* [PATCH] PURR should use correct cpu feature bit
From: Anton Blanchard @ 2006-08-18 18:48 UTC (permalink / raw)
  To: linuxppc-dev, paulus


Now we have a PURR cpu feature bit, we should use it in sysfs code.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index fec228c..a8172de 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -231,7 +231,7 @@ #endif
 	if (cur_cpu_spec->num_pmcs >= 8)
 		sysdev_create_file(s, &attr_pmc8);
 
-	if (cpu_has_feature(CPU_FTR_SMT))
+	if (cpu_has_feature(CPU_FTR_PURR))
 		sysdev_create_file(s, &attr_purr);
 }
 
@@ -273,7 +273,7 @@ #endif
 	if (cur_cpu_spec->num_pmcs >= 8)
 		sysdev_remove_file(s, &attr_pmc8);
 
-	if (cpu_has_feature(CPU_FTR_SMT))
+	if (cpu_has_feature(CPU_FTR_PURR))
 		sysdev_remove_file(s, &attr_purr);
 }
 #endif /* CONFIG_HOTPLUG_CPU */

^ permalink raw reply related

* Re: [PATCH] powerpc: emulate power5 popcntb instruction
From: Arnd Bergmann @ 2006-08-18 19:05 UTC (permalink / raw)
  To: linuxppc-dev, will_schmidt; +Cc: Paul Mackerras
In-Reply-To: <1155924687.9659.25.camel@farscape.rchland.ibm.com>

On Friday 18 August 2006 20:11, Will Schmidt wrote:
> +#define INST_POPCNTB           0x7c0000f4
> +

> +=A0=A0=A0=A0=A0=A0=A0/* Emulate the popcntb (Population Count Bytes) ins=
truction. */
> +=A0=A0=A0=A0=A0=A0=A0if ((instword & INST_POPCNTB) =3D=3D INST_POPCNTB) {
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return emulate_popcntb_inst=
(regs, instword);
> +=A0=A0=A0=A0=A0=A0=A0}
> +

Is that the right check? The other similar traps check against a
mask of 0x7c0007fe.

	Arnd <><

^ permalink raw reply

* Re: [PATCH] Add 85xx DTS files
From: Sergei Shtylyov @ 2006-08-18 19:14 UTC (permalink / raw)
  To: Andy Fleming; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.61.0608172024570.420@ld0175-tx32.am.freescale.net>

Hello.

Andy Fleming wrote:

> Added the DTS files for the 8540 ADS, and the 8555/41/48 CDS

    MPC8540ADS file doesn't seem completely correct.

> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
> new file mode 100644
> index 0000000..88b0ea7
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -0,0 +1,250 @@
[...]
> +	soc8540@e0000000 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		#interrupt-cells = <2>;
> +		device_type = "soc";
> +		ranges = <0 e0000000 00100000>;
> +		reg = <e0000000 00100000>;	// CCSRBAR 1M
> +		bus-frequency = <0>;
> +
> +		i2c@3000 {
> +			device_type = "i2c";
> +			compatible = "fsl-i2c";
> +			reg = <3000 100>;
> +			interrupts = <1b 2>;

    This means active high interrupt, doesn't it? So, 
Documentation/powerpc/booting=without-of.txt should be indeed wrong on the 
OpenPIC IRQ sense encoding... The kernel doesn't seem to put any attention to 
theis field currently though...

> +			interrupt-parent = <40000>;
> +			dfsrr;
> +		};
> +
> +		mdio@24520 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "mdio";
> +			compatible = "gianfar";
> +			reg = <24520 20>;
> +			linux,phandle = <24520>;
> +			ethernet-phy@0 {
> +				linux,phandle = <2452000>;
> +				interrupt-parent = <40000>;
> +				interrupts = <35 3>;

    Falling edge?

> +				reg = <0>;
> +				device_type = "ethernet-phy";
> +			};
> +			ethernet-phy@1 {
> +				linux,phandle = <2452001>;
> +				interrupt-parent = <40000>;
> +				interrupts = <35 3>;
> +				reg = <1>;
> +				device_type = "ethernet-phy";
> +			};

    The board has 2 more PHYs using IRQ54 (0x37).

> +		ethernet@26000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			device_type = "network";
> +			model = "FEC";
> +			compatible = "gianfar";
> +			reg = <26000 1000>;
> +			local-mac-address = [ 00 E0 0C 00 73 02 ];
> +			interrupts = <19 2>;
> +			interrupt-parent = <40000>;
> +			phy-handle = <2452001>;
> +		};

    This doesn't seem correct. FEC is served by PHY #3 (Davicom), not PHY #1 
(Marvell).

[...]
> +		pci@8000 {
> +			linux,phandle = <8000>;
> +			interrupt-map-mask = <f800 0 0 7>;
> +			interrupt-map = <
> +
> +				/* IDSEL 0x02 */
> +				1000 0 0 1 40000 31 1
> +				1000 0 0 2 40000 32 1
> +				1000 0 0 3 40000 33 1
> +				1000 0 0 4 40000 34 1
> +
> +				/* IDSEL 0x03 */
> +				1800 0 0 1 40000 34 1
> +				1800 0 0 2 40000 31 1
> +				1800 0 0 3 40000 32 1
> +				1800 0 0 4 40000 33 1
> +
> +				/* IDSEL 0x04 */
> +				2000 0 0 1 40000 33 1
> +				2000 0 0 2 40000 34 1
> +				2000 0 0 3 40000 31 1
> +				2000 0 0 4 40000 32 1
> +
> +				/* IDSEL 0x05 */
> +				2800 0 0 1 40000 32 1
> +				2800 0 0 2 40000 33 1
> +				2800 0 0 3 40000 34 1
> +				2800 0 0 4 40000 31 1
> +
> +				/* IDSEL 0x0c */
> +				6000 0 0 1 40000 31 1
> +				6000 0 0 2 40000 32 1
> +				6000 0 0 3 40000 33 1
> +				6000 0 0 4 40000 34 1
> +
> +				/* IDSEL 0x0d */
> +				6800 0 0 1 40000 34 1
> +				6800 0 0 2 40000 31 1
> +				6800 0 0 3 40000 32 1
> +				6800 0 0 4 40000 33 1
> +
> +				/* IDSEL 0x0e */
> +				7000 0 0 1 40000 33 1
> +				7000 0 0 2 40000 34 1
> +				7000 0 0 3 40000 31 1
> +				7000 0 0 4 40000 32 1
> +
> +				/* IDSEL 0x0f */
> +				7800 0 0 1 40000 32 1
> +				7800 0 0 2 40000 33 1
> +				7800 0 0 3 40000 34 1
> +				7800 0 0 4 40000 31 1
> +
> +				/* IDSEL 0x12 */
> +				9000 0 0 1 40000 31 1
> +				9000 0 0 2 40000 32 1
> +				9000 0 0 3 40000 33 1
> +				9000 0 0 4 40000 34 1
> +
> +				/* IDSEL 0x13 */
> +				9800 0 0 1 40000 34 1
> +				9800 0 0 2 40000 31 1
> +				9800 0 0 3 40000 32 1
> +				9800 0 0 4 40000 33 1
> +
> +				/* IDSEL 0x14 */
> +				a000 0 0 1 40000 33 1
> +				a000 0 0 2 40000 34 1
> +				a000 0 0 3 40000 31 1
> +				a000 0 0 4 40000 32 1
> +
> +				/* IDSEL 0x15 */
> +				a800 0 0 1 40000 32 1
> +				a800 0 0 2 40000 33 1
> +				a800 0 0 3 40000 34 1
> +				a800 0 0 4 40000 31 1>;
> +			interrupt-parent = <40000>;
> +			interrupts = <08 3>;

    Hm, why it's falling edge (if it is)? Isn't it an internal IRQ which are 
all active high?

> +			bus-range = <0 0>;
> +			ranges = <02000000 0 80000000 80000000 0 20000000
> +				  01000000 0 00000000 e2000000 0 00100000>;

   Well, U-Boot sets up 16 MB I/O window, not 1 MB. Or have this changed?

> +			clock-frequency = <3f940aa>;
> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;
> +			reg = <8000 1000>;
> +			compatible = "85xx";
> +			device_type = "pci";
> +		};

    Well, these props should probaby come the first in the node, for clarity...

WBR, Sergei

^ permalink raw reply

* Re: [PATCH 2/4]: powerpc/cell spidernet low watermark patch.
From: Linas Vepstas @ 2006-08-18 19:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Arnd Bergmann, netdev, James K Lewis, linux-kernel, linuxppc-dev,
	Jens Osterkamp
In-Reply-To: <1155771820.11312.116.camel@localhost.localdomain>

On Thu, Aug 17, 2006 at 01:43:40AM +0200, Benjamin Herrenschmidt wrote:
> 
> Sounds good (without actually looking at the code though :), that was a
> long required improvement to that driver. Also, we should probably look
> into using NAPI polling for tx completion queue as well, no ?

Just for a lark, I tried using NAPI polling, while disabling all TX
interrupts. Performance was a disaster: 8Mbits/sec, fom which I conclude
that the tcp ack packets do not flow back fast enough to allw reliance
on NAPI polling for transmit.

I was able to get as high as 960 Mbits/sec in unusal circumstances, 
at 100% cpu usage. Oprofile indicates that the next major improvement
would be to add scatter/gather, which I'll take a shot at next week,
if I don't get interrupted. However, I'm getting interrupted a lot these
days.

--linas

^ permalink raw reply

* Re: [PATCH] powerpc: emulate power5 popcntb instruction
From: Kumar Gala @ 2006-08-18 19:30 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <200608182105.45264.arnd@arndb.de>


On Aug 18, 2006, at 2:05 PM, Arnd Bergmann wrote:

> On Friday 18 August 2006 20:11, Will Schmidt wrote:
>> +#define INST_POPCNTB           0x7c0000f4
>> +
>
>> +       /* Emulate the popcntb (Population Count Bytes)  
>> instruction. */
>> +       if ((instword & INST_POPCNTB) == INST_POPCNTB) {
>> +               return emulate_popcntb_inst(regs, instword);
>> +       }
>> +
>
> Is that the right check? The other similar traps check against a
> mask of 0x7c0007fe.

I agree with Arnd here, its better to check with a larger mask to  
ensure that bits that should be '0' in the minor opcode are.

For example, if you had an instruction that was 0x7c0000f7 it would  
match.

- kumar

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox