On Wed, 2014-10-01 at 16:47 +1000, Michael Ellerman wrote: > On Tue, 2014-30-09 at 10:34:51 UTC, Michael Neuling wrote: > > From: Ian Munsie <imun...@au1.ibm.com> > > > > __spu_trap_data_seg() currently contains code to determine the VSID and ESID > > required for a particular EA and mm struct. > > > > This code is generically useful for other co-processors. This moves the > > code > > of the cell platform so it can be used by other powerpc code. It also adds > > 1TB > > segment handling which Cell didn't have. > > I'm not loving this. > > For starters the name "copro_data_segment()" doesn't contain any verbs, and it > doesn't tell me what it does.
Ok. > If we give it a name that says what it does, we get > copro_get_ea_esid_and_vsid(). > Or something equally ugly. Ok > And then in patch 10 you move the bulk of the logic into calculate_vsid(). That was intentional on my part. I want this patch to be clear that we're moving this code out of cell. Then I wanted the optimisations to be in a separate patch. It does mean we touch the code twice in this series, but I was hoping it would make it easier to review. Alas. :-) > So instead can we: > - add a small helper that does the esid calculation, eg. calculate_esid() ? > - factor out the vsid logic into a helper, calculate_vsid() ? > - rework the spu code to use those, dropping __spu_trap_data_seg() > - use the helpers in the cxl code OK, I think I can do that. I might change the name to something better in this patch, but I'll leave these cleanups to the later patch 10. Mikey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
