Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933168AbbGVDvf (ORCPT ); Tue, 21 Jul 2015 23:51:35 -0400 Received: from mail-pd0-f174.google.com ([209.85.192.174]:32836 "EHLO mail-pd0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932093AbbGVDvd (ORCPT ); Tue, 21 Jul 2015 23:51:33 -0400 Message-ID: <1437536981.30906.9.camel@axtens.net> Subject: Re: [PATCH v5 3/7] powerpc/powernv: Nest PMU detection and device tree parser From: Daniel Axtens To: Madhavan Srinivasan Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Stephane Eranian , Paul Mackerras , Anton Blanchard , Sukadev Bhattiprolu , Anshuman Khandual Date: Wed, 22 Jul 2015 13:49:41 +1000 In-Reply-To: <1437045206-7491-4-git-send-email-maddy@linux.vnet.ibm.com> References: <1437045206-7491-1-git-send-email-maddy@linux.vnet.ibm.com> <1437045206-7491-4-git-send-email-maddy@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-kX+xYsq+0FG/YOIK7Wfj" X-Mailer: Evolution 3.12.11-1 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4312 Lines: 136 --=-kX+xYsq+0FG/YOIK7Wfj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, > +static struct perchip_nest_info p8_nest_perchip_info[P8_NEST_MAX_CHIPS]; > + > +static int nest_ima_dt_parser(void) > +{ > + const __be32 *gcid; > + const __be64 *chip_ima_reg; > + const __be64 *chip_ima_size; > + struct device_node *dev; > + struct perchip_nest_info *p8ni; > + int idx; > + > + /* > + * "nest-ima" folder contains two things, > + * a) per-chip reserved memory region for Nest PMU Counter data > + * b) Support Nest PMU units and their event files > + */ > + for_each_node_with_property(dev, "ibm,ima-chip") { > + gcid =3D of_get_property(dev, "ibm,chip-id", NULL); > + chip_ima_reg =3D of_get_property(dev, "reg", NULL); > + chip_ima_size =3D of_get_property(dev, "size", NULL); > + > + if ((!gcid) || (!chip_ima_reg) || (!chip_ima_size)) { > + pr_err("Nest_PMU: device %s missing property\n", > + dev->full_name); > + return -ENODEV; > + } > + > + /* chip id to save reserve memory region */ > + idx =3D (uint32_t)be32_to_cpup(gcid); So be32_to_cpup returns a __u32. You're casting to a uint32_t and then assigning to an int. - Do you need the intermediate cast? - Should idx be an unsigned type? > + > + /* > + * Using a local variable to make it compact and > + * easier to read > + */ We probably don't need this comment. But a better variable name would be helpful!=20 > + p8ni =3D &p8_nest_perchip_info[idx]; > + p8ni->pbase =3D be64_to_cpup(chip_ima_reg); > + p8ni->size =3D be64_to_cpup(chip_ima_size); > + p8ni->vbase =3D (uint64_t) phys_to_virt(p8ni->pbase); > + } > + > + return 0; > +} > + > +static int __init nest_pmu_init(void) > +{ > + int ret =3D -ENODEV; > + > + /* > + * Lets do this only if we are hypervisor > + */ > + if (!cur_cpu_spec->oprofile_cpu_type || > + !(strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8") =3D=3D 0)= || > + !cpu_has_feature(CPU_FTR_HVMODE)) > + return ret; > + I'm still really uncomfortable with doing this via oprofile_cpu_type. If the kernel is compiled without oprofile support, will that get populated? I'm also curious about why we need checking for power8 at all. We already know we're not going to run on hardware without a nest PMU because of the device tree check below.=20 What happens if there's a future generation of chip that supports nest PMUs? If it's really important to check versions in this function, maybe you could put a version property in the ibm,ima-chip node? > + /* > + * Nest PMU information is grouped under "nest-ima" node > + * of the top-level device-tree directory. Detect Nest PMU > + * by the "ibm,ima-chip" property. > + */ > + if (!of_find_node_with_property(NULL, "ibm,ima-chip")) > + return ret; > + > + /* > + * Parse device-tree for Nest PMU information > + */ > + ret =3D nest_ima_dt_parser(); > + if (ret) > + return ret; > + > + return 0; > +} > +device_initcall(nest_pmu_init); --=20 Regards, Daniel --=-kX+xYsq+0FG/YOIK7Wfj Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: GPGTools - https://gpgtools.org iQIcBAABCgAGBQJVrxLVAAoJEPC3R3P2I92F97YP/jNCHgTGxWUhd4s5GcuuS0ZU aEdirldcCxyGsV08gdJK3+Nt2mcOEoDWG3zwnkUjhXR9q7CoFbK2Sl/2PY5ZBWGq 31cwH4u85YP9sgkSzDxMD62wZhm2SlGBUO3PR6NTFS+Mk5zoi6jcZb9Bysp2hlR2 ndmXSWQ4JPbvQdHaP6Kvppp0MqR9nmbfoo4BxhsnrjWyuYh+RO943Yf1fD0GZYYp zBLIsRs81fHe5sF18fX+915wZ89Zhikmc+v2V1fArX3g/vvDq96GJTwldYm5fDZW zzMZ7p6RbBHjS57h4HN/9Wi0cXsq9rbMjgy7RImm2GeAQIkuZKhBCsq5oBUdGxE+ NbEKnxCGbpEeQ/+Mn6c66N70ptK8vHBePdPOaIgG1D+it737/7CRpBU+pyJmP6g8 l9x5RurBKNUBu+qO3RzNzxJtDJ7xcEdmu7OxaAiSB9Zo403cMT4l3sAlOrKDZzaC XQ2Ssq2bY9swMLpuvWr/Glzek6Kbuyx3w1lqnqa/lfZm9vAmkxXwce+TiwqLtIv1 W5DSWRdeoDk6ew9XRAMmfyu2M/NcSmwPHj2nAcqGvn1SH5UCriNpy3Pim+PBPJQT VAug1TBw8Cv9C4hOun2OJWk2DX16ok6hXFeBCkW+c+1QDGmHHw3B6bImBCLHFn0t U6rBfDQ5diwEouQt7tMS =zB/+ -----END PGP SIGNATURE----- --=-kX+xYsq+0FG/YOIK7Wfj-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/