From owner-freebsd-ppc@FreeBSD.ORG Wed Dec 11 22:40:33 2013 Return-Path: Delivered-To: freebsd-ppc@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7795D57C; Wed, 11 Dec 2013 22:40:33 +0000 (UTC) Received: from mail-bk0-x22e.google.com (mail-bk0-x22e.google.com [IPv6:2a00:1450:4008:c01::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id B283A104F; Wed, 11 Dec 2013 22:40:32 +0000 (UTC) Received: by mail-bk0-f46.google.com with SMTP id u15so585406bkz.33 for ; Wed, 11 Dec 2013 14:40:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=2p/WsCef1M9Su61+EbK1294WML8d7XuAx1WijfbBo3Y=; b=l3ong/R207bf0hZK1dAoPdwJQeeWIw1U4lMWbXxvhLG9vYZR9H3oIW9m2HV6i4vCcP 8e2hHg1WEIfg2AXu3EqVytktvoyN9FPMvZz6iUsNkjmGdXL5x7wJrdQEJkOMIGSqHuHu QyjeTVTobRQMgrd4Y+s4a6WMfRWM28pY3R9R5cXjlKNeaF/k6edACrhXWZjt/5nVQURy HX3LCVth1nIaCscoGfYCFg6MilNEwJRkpauKVpfV7dGZFbecEc3cm6RneKdX7DGk6nYr qQ+8r9g+GgRdg9Qos/31QyDUwifkWuYLxb9KxKVud8bw6Bc9TRtr8Wtfs5Dq7CFlH/nU nAHQ== MIME-Version: 1.0 X-Received: by 10.204.127.135 with SMTP id g7mr1580761bks.42.1386801630927; Wed, 11 Dec 2013 14:40:30 -0800 (PST) Sender: chmeeedalf@gmail.com Received: by 10.205.90.136 with HTTP; Wed, 11 Dec 2013 14:40:30 -0800 (PST) In-Reply-To: <201312111626.12035.jhb@freebsd.org> References: <20131204222113.39fb23dd@zhabar.gateway.2wire.net> <201312111626.12035.jhb@freebsd.org> Date: Wed, 11 Dec 2013 14:40:30 -0800 X-Google-Sender-Auth: djaeks2JCuZXT6kmaY_trFZp7qU Message-ID: Subject: Re: Request for testing an alternate branch From: Justin Hibbits To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: FreeBSD Current , FreeBSD PowerPC ML X-BeenThere: freebsd-ppc@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Porting FreeBSD to the PowerPC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Dec 2013 22:40:33 -0000 On Wed, Dec 11, 2013 at 1:26 PM, John Baldwin wrote: > On Thursday, December 05, 2013 1:21:13 am Justin Hibbits wrote: >> I've been working on the projects/pmac_pmu branch for some time now to >> add suspend/resume as well as CPU speed change for certain PowerPC >> machines, about a year since I created the branch, and now it's stable >> enough that I want to merge it into HEAD, hence this request. However, >> it does touch several drivers, turning them into "early drivers", such >> that they can be initialized, and suspended and resumed at a different >> time. Saying that, I do need testing from other architectures, to make >> sure I haven't broken anything. >> >> The technical details: >> >> To get proper ordering, I've extended the bus_generic_suspend() and >> bus_generic_resume() to do multiple passes. Devices which cannot be >> enabled or disabled at the current pass level would return an EAGAIN. >> This could possibly cause problems, since it's an addition to an >> existing API rather than a new API to run along side it, so it needs a >> great deal of testing. It works fine on PowerPC, but I don't have any >> i386/amd64 or sparc64 hardware to test it on, so would like others who >> do to test it. I don't think that it would impact x86 at all (testing >> is obviously required), because the nexus is not an EARLY_DRIVER_MODULE, >> so all devices would be handled at the same pass. But, I do know the >> sparc64 has an EARLY_DRIVER_MODULE() nexus, so that will likely be >> impacted. > > I have patches to change many x86 drivers to use EARLY_DRIVER_MODULE() > FWIW. > > Also, I'm still not a fan of the EAGAIN approach. I'd rather have a method > in bus_if.m to suspend or resume a single device and to track that a device > is suspended or resumed via a device_t flag or some such. (I think I had > suggested this previously as it would also allow us to have a tool to > suspend/resume individual drivers at runtime apart from a full suspend/resume > request). > > -- > John Baldwin Understood. You had mentioned something along those lines before. Is it safe/sane to partially merge a branch into HEAD? If so, I can merge only the changes necessary for PMU cpufreq, and work on the suspend/resume separately. I put them together because most of the low level code involved is the same between them. I do like your idea of a device_t flag, but I'm not sure what the best way to go with that would be. I do already use a device_t flag to determine if the device is suspended, but only bus_generic_* access that in this patch. Would a better way be: * root_suspend instead of suspending its children, instead traverses and suspends each descendent in reverse order. * While doing this, insert each device upon successful suspend into a list. * For root_resume(), traverse the list back, and suspend each device in the reverse order. With this, add a new method, called device_suspend_child() to suspend a specific child if it hasn't already been suspended. * This could require modifying the PCI driver to move the device child suspend/resume into those functions, instead of doing that logic in the device_suspend/device_resume itself. I guess, in short, I'm asking: Is it fine if I merge only the code necessary for this cpufreq? That would require making other changes to this before merging in, to isolate that code, but it's very doable. - Justin